Limit max oxygen tolerance#6796
Conversation
|
Would this limit the slider on the tolerance panel to 8%, or am I misreading the explanation. |
|
It would limit the movable slider, but not the max display value, this one stays at 100% max |
|
It seems that since auto-evo is still run, the begging conditions when player's cell tolerances are calculated are changed (the one read in |
|
Shouldn't it be slightly increased instead? Because couldn't auto-evo theoretically cause the world to also get more oxygen thus requiring more protection. I'd like to much more avoid the situation where the player's initial tolerance is not good enough for their environment. |
| } | ||
| else | ||
| { | ||
| // auto-evo is run and oxygen levels might decrease a bit so make sure the tolerance is high enough |
There was a problem hiding this comment.
This comment still says the opposite (decrease instead of increase), but other than that the code changes now look good.
hhyyrylainen
left a comment
hhyyrylainen
left a comment
There was a problem hiding this comment.
To me it does not look good that the upper bar is not updated to match with the range of the lower bar, I think both sections should have the same max range as otherwise it looks silly like this:
I still wasn't able to get rid of this slight offset appearing in some values. I have no idea what might be causing it, my guess is that it is some rounding problem? The issue is never bigger than 1 pixel wide |
…oxygen-tolerance-limiting
|
Okay, this PR is ready. Might still need some rebalancing tho |
hhyyrylainen
left a comment
hhyyrylainen
left a comment
There was a problem hiding this comment.
I looked at the code and it is pretty much good now.
I noticed one slight problem with old tolerance values, if you drag the slider down and back up you can end up missing 0.1% MP, which is not the worlds most serious thing so I think we can merge this if only older saves can sometimes show this slight bug:
Playing one more generation and then going to the editor has the bug go away, so I assume some of the Math.Ceil calls / rounding cause the problem.
(I can provide the save file that allows triggering this if needed)
So yeah, I think this is good enough to merge now, but if that slight bug would be easy enough to fix we can wait some time for the fix.
4 participants
Brief Description of What This PR Does
This PR:
Draft for now as I need to playtest it more
Progress Checklist
Note: before starting this checklist the PR should be marked as non-draft.
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
styleguide.
Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.