Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shield generator and Long range scanner refactor + Bioreactor fixes #8514

Merged
merged 8 commits into from
Sep 23, 2024

Conversation

AltHit
Copy link
Contributor

@AltHit AltHit commented Aug 13, 2024

About The Pull Request

This PR refactors and fixes three entirely broken machines. Now conduits on long range scanner and shield gen aren't just dummy models but can be constructed separately, also their machines will works worse if they lack conduits and their work will improve if you put extra conduit!

Why It's Good For The Game

Bugs fixed here existed for ages and now after ~6 years you can actually unplug shield from it's room and plug it somewhere else. And that was just a one example of many

Testing

Many local test launches on VS Code testing different functions of those machines
EDIT: Tried cutting off separate cabels from conduits: power input dropped by 33% when one of 3 capacitors was cut. As expected.
Took shield generator for a walk and installed it in a third section hallway. Third sections hallway's APC did not approve of this as expected but after giving it a little more love from the substation I managed to get shields up right in this hallway
Using RnD printed curcuits and parts to build new shield generator and logng range scanners from scratch: They actually ran and worked fine with the exeption of draining power from the entire section to them. As expected of course.
Changed parts in shield generator to debug ones: Generator started to consume 1/100 of it's power showing that math works alright.
Changed parts in logng range scanner to debug variations: Scanner pulse lasted for several minutes and consumed almost no power. Formula for this one took me several days to figure out...
Built long range scanner using cheep artifical bluespace crystal and with only one conduit: ping of this scanner lasted for a whopping 5 seconds!

Changelog

🆑
add: Added ability to construct conduits for Shield generator and Long range scanner
add: Added ability to connect/disconnect conduits for Shield generator and Long range scanner with enough MEC skill
add: Conduit circuit board designs to RnD
tweak: Now conduits for shieldgen and scanner have parts inside of them and they can be upgraded to improve efficiency
tweak: Changed parts that are used in construction of Shield generator and Long range scanner
tweak: Bioreactor now deals burn damage instead of brute
fix: Corrected start-up and shut down animations for Long range scanner
fix: Fixed Bioreactor breaking after clicking it with a screwdriver
fix: Fixed Bioreactor working with broken glass walls
fix: Fixed Bioreactor glass being inversed transparent (it was see-through when it should have blocked view and vice versa)
fix: Fixed consoles not making sounds when they were supposed to make it
fix: Cahors wine now actually cures poisoning
refactor: Now Shield generator and Long range scanner have shared parent object
config: Changed long range scanner power consumption
/:cl:

AltHit and others added 5 commits July 30, 2024 04:08
Now multistructures don't break until rebuild when screwed
Bioreactor will now properly check for any breaches in the chamber and it will properly shut down if it spots any breaches
Swapped placeholder power consumption for the real one
Fixed bioreactor window opacity being inverted
Cahors now has a slight anti-toxin effect as it should
Bioreactor chamber ritual now updates breached status of bioreactor to restore it
Fixed ability to extend shieldgen's and sensor's capacitors while they are unanchored
Changed effects of Cahors wine
@Firefox13
Copy link
Contributor

изображение
This section is meant not be for "Yeah I tested it" but for "I test it X and Y way" so we can find if you missed something. You obviously probably can't remember everything, so just write what you can remember.

Copy link
Member

@SirRichardFrancis SirRichardFrancis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo conduits should be kept as a semi-cosmetic thing: improving efficiency somewhat if extra space is sacrificed for them, but not a big deal otherwise.
So they're essentially "free" to deploy, not a bunch of actually independently build machines.
Otherwise it would be really annoying to build a new shield generator, have to make four machines instead of one.
Could still build only a generator, but how inefficient would it be?

Arguments of proc don't need var/ in them, as well as constructions like mob/user as mob, just mob/user or even user as mob is enough.

Suggested by SirRichardFrancis
And yes, I tested affected procs.
@AltHit
Copy link
Contributor Author

AltHit commented Aug 28, 2024

I mean... You don't rebuild shield generators so often do you? And even if you do - I tested and the most basic setup with protection from asteroids and whole ship coverage can work from only one conduit. It won't be good but it will work and it will charge
If needed - I can add curcuits for those conduits in cargo as well

@github-actions github-actions bot added the Merge Conflict Merge Conflict label Sep 14, 2024
@SirRichardFrancis SirRichardFrancis added Fix Balance and removed Merge Conflict Merge Conflict labels Sep 21, 2024
Copy link
Member

@SirRichardFrancis SirRichardFrancis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and found no issues beside interactions with conduits being somewhat unintuitive.

@Humonitarian Humonitarian merged commit 954370b into discordia-space:master Sep 23, 2024
6 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants