Files
Bubberstation/code/modules/atmospherics/gasmixtures
Pickle-CodingandGitHub 4465ac6e17 The mole counts in the min checks in reactions.dm will now be multiplied by the inverse of the consumption multiplier. (#61557)
The moles in the min checks in reactions.dm will be multiplied by the inverse of the consumption multiplier. This will allow reactions to fully react when it has available gasses. It also makes some of the code easier to read.

Fixes (#61380)

From the issue report above:

Alright. So there's this pattern in reaction code

Looks like this

tgstation/code/modules/atmospherics/gasmixtures/reactions.dm

Lines 597 to 604 in 00154ae
 var/nob_formed = min((cached_gases[/datum/gas/nitrogen][MOLES] + cached_gases[/datum/gas/tritium][MOLES]) * 0.01, cached_gases[/datum/gas/tritium][MOLES] * 0.1, cached_gases[/datum/gas/nitrogen][MOLES] * 0.2) 
 var/energy_produced = nob_formed * (NOBLIUM_FORMATION_ENERGY / (max(cached_gases[/datum/gas/bz][MOLES], 1))) 
 if ((cached_gases[/datum/gas/tritium][MOLES] - 5 * nob_formed < 0) || (cached_gases[/datum/gas/nitrogen][MOLES] - 10 * nob_formed < 0)) 
 	return NO_REACTION 
  
 cached_gases[/datum/gas/tritium][MOLES] -= 5 * nob_formed 
 cached_gases[/datum/gas/nitrogen][MOLES] -= 10 * nob_formed 
 cached_gases[/datum/gas/hypernoblium][MOLES] += nob_formed 

We take the minimum of a few values, theoretically because we want the reaction to run with the lowest amount feasible.
So if there's 20 plasma, 10 o2, and 2 n2, and the reaction takes 4 parts plasma, 2 part o2, and 1 part n2, we'll only end up using 8 plasma, 4 o2, and 2 n2. Since we can't react without the n2 and all.

The if check is there to serve as a backup and prevent negative outputs, theoretically because they wreck havoc, though honestly they don't really, so long as the right bitflag is returned the whole mix is garbage collected anyway. Alright sanity check though. that's fine.

You notice how here, because he removes 5x nob formed from tritium, he includes that in the if check? If you scroll out to the right you'll notice that he multiplies the inputs in the min by the inverse of their scalar. At least that's what we want trying to do, mixed the two up.

You get the picture. The min serves to get the lowest possible amount to remove so the reaction can go through, and the if check serves as a sanity check wrapping around it.

The issue is people have been misusing it, for a good while. (Including in this instance)
They most commonly forget to include the inverse scaling in the min(), which leads to these weird fucked phantom gas minimums that aren't listed anywhere, but still stop the reaction.

See:

tgstation/code/modules/atmospherics/gasmixtures/reactions.dm

Lines 690 to 697 in 00154ae
 var/heat_efficency = min(temperature * 0.3, cached_gases[/datum/gas/freon][MOLES], cached_gases[/datum/gas/bz][MOLES]) 
 var/energy_used = heat_efficency * 9000 
 ASSERT_GAS(/datum/gas/healium, air) 
 if ((cached_gases[/datum/gas/freon][MOLES] - heat_efficency * 2.75 < 0 ) || (cached_gases[/datum/gas/bz][MOLES] - heat_efficency * 0.25 < 0)) //Shouldn't produce gas from nothing. 
 	return NO_REACTION 
 cached_gases[/datum/gas/freon][MOLES] -= heat_efficency * 2.75 
 cached_gases[/datum/gas/bz][MOLES] -= heat_efficency * 0.25 
 cached_gases[/datum/gas/healium][MOLES] += heat_efficency * 3 

They need to be updated to not do this, and use min() properly. It leads to dumb graphs like this https://www.desmos.com/calculator/xufgz8piqw (Healium formation graphed out. p is freon, b is bz. if either are reduced below 0, the reaction stops. If you notice this leads to really strange scaling deadspots, and a lot of frustrating behavior)

Thanks to @GuillaumePrata for bringing this to my attention, love you man.
2021-10-02 14:59:29 -07:00
..