-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Gas Miner refactor #29657
Gas Miner refactor #29657
Conversation
Let me know if I should add the part about |
placement: | ||
mode: SnapgridCenter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all unneeded because it's the default behavior for GasMinerBase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for separating this into a separate PR. I've left a few review comments. I concur that Broken
-> Idle
is a breaking change, and ideally you would point that out in the breaking changes section. I plan to test locally tomorrow and merge so if you wouldn't mind addressing the nitpicks that would be great.
Content.Server/Atmos/Piping/Other/Components/GasMinerComponent.cs
Outdated
Show resolved
Hide resolved
Content.Server/Atmos/Piping/Other/Components/GasMinerComponent.cs
Outdated
Show resolved
Hide resolved
Content.Server/Atmos/Piping/Other/EntitySystems/GasMinerSystem.cs
Outdated
Show resolved
Hide resolved
Content.Server/Atmos/Piping/Other/EntitySystems/GasMinerSystem.cs
Outdated
Show resolved
Hide resolved
Content.Server/Atmos/Piping/Other/EntitySystems/GasMinerSystem.cs
Outdated
Show resolved
Hide resolved
Content.Server/Atmos/Piping/Other/EntitySystems/GasMinerSystem.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Partmedia <[email protected]>
Content.Server/Atmos/Piping/Other/EntitySystems/GasMinerSystem.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Separate the environment check from CapSpawnAmount into GetValidEnvironment to make the code a little cleaner, and also makes these two checks independent. CapSpawnAmount and GetValidEnvironment now both have zero side-effects Broken renamed Idle to reflect its use. Broken in my mind implies that there's some method for fixing. --------- Co-authored-by: Partmedia <[email protected]>
About the PR
Lightly refactors
GasMinerSystem
to prepare for a content PR making them examinable.Why / Balance
Technical details
CapSpawnAmount
intoGetValidEnvironment
to make the code a little cleaner, and also makes these two checks independent.CapSpawnAmount
andGetValidEnvironment
now both have zero side-effectsBroken
renamedIdle
to reflect its use.Broken
in my mind implies that there's some method for fixing.enabled
read-write though VVIdle
read-only though VVMedia
Breaking changes
GasMinerComponent.Broken
renamedGasMinerComponent.Idle
to reflect its use.Changelog