Use archived gas mixture in gas exchange comparison

[drakewill-CRL]

About the PR

CompareExchange() now allows using archives gases instead of current ones.

Why / Balance

Fixes #32053

Technical details

A flag was added to CompareExchange to let it pick the current air or the archived air value for comparisons. The 2 calls to it were updated to match the new function signature. The function's 2 main arguments were also changed to the parent of both the current and archived air values.

Media

No visible changes

Requirements

• I have read and am following the Pull Request and Changelog Guidelines.
• I have added media to this PR or it does not require an ingame showcase.

Breaking changes

• In TileAtmosphere, TemperatureArchived and MolesArchived have been rolled into AirArchived. Use AirArchived.Temperature and AirArchived.Moles to access these fields, respectively.

Changelog

🆑

• fix: Fix error in gas exchange processing order.

[ScarKy0]

it was DIRECTIONAL??

[mirrorcult]

comment on the function should change since it takes tileatmosphere instead of gasmixture now

[lzk228]

your cl is broken, should have 🆑 symbol

[Partmedia]

Thanks for the investigation and fix. From a code perspective, I think it would be cleaner, easier to understand, and more testable if you left the old CompareExchange as-is and wrapped a new function around it. It could even overload CompareExchange but with TileAtmosphere arguments (though I think overloading makes it harder for humans to understand).

That would permit you to remove the bool useArchived = false optional parameter, since the only two callers have been modified to set that to true.

[Partmedia]

Do you happen to have a minimal map that demonstrates the issue and how the changes fix it?

[Ilya246]

Do you happen to have a minimal map that demonstrates the issue and how the changes fix it?

i'm pretty sure this bug causes burns in closed room to just completely break and have one tile hitting tempcap and an adjacent tile being at like 100C, this is not exaggerated, this probably wasn't reported yet because i am lazy and because nobody else does closed burns of the required kind

[deltanedas]

i reported it ages ago

[Partmedia]

i reported it ages ago

Mind giving me a minimum reproducible map file for me to test this fix?

[deltanedas]

it doesnt need a map just fill a 3x3 or larger chamber with burn mix, ignite the center and watch the center burn and sides stay at 20C until the reaction ends via fuel or tempcap (needs diffusing more fuel from adjacent tiles)

[Partmedia]

I tested this and this patch does not appear to fix @deltanedas 's problem. I would be curious what problem this patch actually fixes and if there's a minimal map that demonstrates this patch fixing that issue.

[Partmedia]

I'd like to add that this patch nevertheless seems correct and we should probably merge it with the suggested changes. I just want to make sure we all understand what is actually being fixed and have a way to validate that.

[Partmedia]

The reason I had suggested this change was to prevent unnecessary code duplication between the two methods. Now I see why this is: TileAtmosphere has a GasMixture but MolesArchived does not. I wonder if there's a way to clean this up.

[Partmedia]

It seems to me that the most reasonable way to handle this would be to replace TileAtmosphere.TemperatureArchived and TileAtmosphere.MolesArchived with TileAtmosphere.AirArchived which would be a GasMixture like TileAtmosphere.Air.

[Partmedia]

Would you mind taking a look at the updated diff and double checking that this still fixes the linked bug?

[drakewill-CRL]

Yes, you're passing in and handling the archived data, so that diff should resolve the issue.

[Partmedia]

Thanks!

[LemonInTheDark]

it was DIRECTIONAL??

Assume we have a turf surrounded by 4 mixes that differ from it by idk 10%
Because we compare and then share with each in a loop, using non archived moles means each successive compare will see a smaller and smaller delta. At some point this hits 0, when in reality the numbers we would SHARE against (the archive) are still at that 10% number.
This leads to a failed share for 1 tick, often then becoming possible next iteration due to other gas moving.

On /tg/ this biased gas flow to the north east, because that's the order we fill out atmos_adjacent_turfs as a list.
Note: this was only a problem on the initial turf interaction, once a turf becomes active it will always share with its neighbors, compare exists just as a way to gate small deltas spreading out forever.

[Ilya246]

what delta described can also happen the other way, chamber center stays 800C but sides go tempcap