-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Canisters [ Continuation of clement-or #2544 ] #2628
Conversation
Update forks
…tation-14 into more-canisters
…tation-14 into more-canisters
…tation-14 into more-canisters
…station-14 into more-canisters-ext
…t interdependencies for no reason whatsoever)
Seems fine, I approve |
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.
This was supposed to be visible on the (1/2) commit, but I failed
#2628 (comment)
Attempted to leave an additional note on the commits, failed hilariously |
Content.Server/GameObjects/Components/Atmos/GasCanisterComponent.cs
Outdated
Show resolved
Hide resolved
Content.Server/GameObjects/Components/Atmos/GasCanisterComponent.cs
Outdated
Show resolved
Hide resolved
Content.Server/GameObjects/Components/Atmos/GasCanisterComponent.cs
Outdated
Show resolved
Hide resolved
Aside from these few nitpicks, this seems fine to merge. It still seems to be missing a feature to fill gas tank using canisters, but that can be added in a future PR if you want. Up to you. |
Content.Client/GameObjects/Components/Atmos/GasCanisterWindow.cs
Outdated
Show resolved
Hide resolved
Content.Client/GameObjects/Components/Atmos/GasCanisterWindow.cs
Outdated
Show resolved
Hide resolved
Content.Client/GameObjects/Components/Atmos/GasCanisterWindow.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Pieter-Jan Briers <pieterjan.briers@gmail.com>
Content.Server/GameObjects/Components/Atmos/GasCanisterComponent.cs
Outdated
Show resolved
Hide resolved
Might wanna fix this for windows. |
Look at the test that failed |
The failing test is ReconnectTest, unrelated to this PR, correct? |
Yes |
…rated (hopefully)
Content.Client/GameObjects/Components/Atmos/GasCanisterVisualizer.cs
Outdated
Show resolved
Hide resolved
Content.Client/GameObjects/Components/Atmos/GasCanisterVisualizer.cs
Outdated
Show resolved
Hide resolved
Content.Client/GameObjects/Components/Atmos/GasCanisterWindow.cs
Outdated
Show resolved
Hide resolved
Content.Client/GameObjects/Components/Atmos/GasCanisterWindow.cs
Outdated
Show resolved
Hide resolved
Content.Client/GameObjects/Components/Atmos/GasCanisterWindow.cs
Outdated
Show resolved
Hide resolved
Content.Client/GameObjects/Components/Atmos/GasCanisterWindow.cs
Outdated
Show resolved
Hide resolved
Content.Server/GameObjects/Components/Atmos/GasCanisterComponent.cs
Outdated
Show resolved
Hide resolved
…t [Updates for TileAtmosphere.Invalidate]
Thanks for all the big work, it was my first big PR and I didn't have a chance to complete it due to health issues |
I'm attempting to maintain this PR.
Note that if clement-or (author of most of the commits here in #2544 ) wants to instead continue it, they can.
The added commits so far are just to handle the "obvious" issues that appeared during my testing of the PR.
Under the circumstances, I'd consider this not-draft, as pipenet port connections were implemented by clement-or contrary to the description of their PR.
Transfer of air between canisters works but seems to act "non-obviously" in regards to the mechanics of transfer to/from the PipeNet.
Something that doesn't seem to be present at all is transfer to/from tank items, but this could be delayed for a future PR - having a way to use PipeNet at all is a start in getting it going.