-
Notifications
You must be signed in to change notification settings - Fork 412
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
Oxygen Generator Improvements #15497
base: master
Are you sure you want to change the base?
Conversation
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.
Missing text for oxygengenerator upgrade?
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.
Overall looks pretty good, spotted only some minor code quality issues. Also would have to change the content definitions a bit, if we'd include the changes in the vanilla game.
Only skimmed through the xml changes, didn't go the UI code very thoroughly, as it's easier to spot the issues by testing, which I didn't do.
No objections on these changes overall.
Barotrauma/BarotraumaClient/ClientSource/Items/Components/Machines/OxygenGenerator.cs
Outdated
Show resolved
Hide resolved
Barotrauma/BarotraumaClient/ClientSource/Items/Components/Machines/OxygenGenerator.cs
Outdated
Show resolved
Hide resolved
Barotrauma/BarotraumaClient/ClientSource/Items/Components/Machines/OxygenGenerator.cs
Outdated
Show resolved
Hide resolved
Barotrauma/BarotraumaServer/ServerSource/Items/Components/Machines/OxygenGenerator.cs
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,5 @@ | |||
<Text language="English"> |
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.
If we intend to change the vanilla content, these lines should be defined in EnglishVanillaNew.xml, which is where we take the new lines to be translated. When the translations are done, we'll move the new lines in EnglishVanilla.xml and the other language files. For a mod, I think these would be fine.
@@ -0,0 +1,5 @@ | |||
<ContentPackage name="oxygen-changes" modversion="1.0.0" gameversion="1.0.8.0"> |
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.
When doing changes in the vanilla content, Vanilla.xml should be used instead. This would be fine for mods.
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.
Hey there, I'm not sure where I should place these files after addressing this, or how I should handle differences between changes/additions and mixed types.
Please let me know!
Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/OxygenGenerator.cs
Outdated
Show resolved
Hide resolved
Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/OxygenGenerator.cs
Outdated
Show resolved
Hide resolved
Without opening the UI of the oxygen generator, would this be visible in any way? At least, to me it would make sense if vents stopped turning when oxygen generation is at 0 (and slowing down at lower values, ideally), but also the generator itself no longer moving and/or showing a red light or orange light based on the percentage of oxygen generation. |
- Serialize CurrFlow. - Only animate while producing oxygen.
Vents already stop turning when no oxygen is being produced, and both the vents and the generator make no sound when not producing anything. |
- Remove decrease in power consumption; the improved generation lets players decrease the generation for less consumption anyway. - Decrease additional generation by 5%, for the same reason.
Turns out this applies to the original value (x - (40% + 40%)) instead of sequentially (x - 20% - 20%), leading to the new value being wrong. This reverts commit 1708511.
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.
Looks good to me from the technical perspective. Would have to decide what to do with this. As commented earlier, some changes are still required in the content definitions, if we'd include this in the vanilla game. But we can take care of that. For testing purposes, it's fine to have this implemented as a mod for now. GJ!
One concern I have about this is whether it's clear enough what's happening if someone for example turns the oxygen generation rate to 10%. How would a player know it's possible to adjust it, where is it communicated? How can a player tell the oxygen generation has been adjusted down, would it seem like a bug if they start suffocating for seemingly no reason? I think one thing that would alleviate the problem a bit is if the oxygen generation rate was visible on the generator and the vents somehow. Maybe the animation speed (and the volume of the sounds too?) could be relative to the oxygen generation rate? |
This PR makes changes to oxygen generators that I believe will enhance the interactions available with them.
The first change adds the ability for players to find and change the oxygen generation rate either manually, or through wiring.
The second change adds a UI to oxygen generators that allows players to:
The third change adds a new upgrade for oxygen generators that increases the oxygen generation rate.
This upgrade has a base cost of 2000 mk and 5 oxygenite shards, with tier restrictions of:
Each level of this upgrade increases oxygen generation by 20%.
The intended effect of these changes is to add more depth to the mechanics of oxygen generators, with the end goal of getting players to engage with them beyond it just being a repairable device that fills oxygen tanks.
For example, traitors can set the generation amount to a lower value to silently starve the crew of oxygen.
Another example is players can create more complex "duty cycle" circuts to save fuel by dynamically changing the generation amount through wiring, therefore reducing the power consumption.