From 9fe443de7191f74449e3967aa038efdf2a9725d4 Mon Sep 17 00:00:00 2001 From: NovaBot <154629622+NovaBot13@users.noreply.github.com> Date: Mon, 19 Feb 2024 13:07:39 -0500 Subject: [PATCH] [MIRROR] Reaction & holder tweaks (#1055) * Reaction & holder tweaks (#81545) ## About The Pull Request - Fixes #81537. For competing reactions where reaction B consumes the results formed from reaction A `total_step_added` yields values higher than the actual volume of products formed causing the reaction to end early leading to less volume of product formed as explained in the comment - Removes `update_total()` from instant reactions as it already does that for us making it a little faster - merges `remove_all_direct()` with `remove_all()` to reduce code as their functionalities are very similar ## Changelog :cl: fix: multiple reactions occuring in the same holder like mixing drinks will yield correct reagent result volumes code: Removes `update_total()` from instant reactions improving performance slightly code: merges `remove_all_direct()` with `remove_all()` to reduce code size /:cl: * Reaction & holder tweaks --------- Co-authored-by: SyncIt21 <110812394+SyncIt21@users.noreply.github.com> --- .../wizard/equipment/teleport_rod.dm | 4 +- .../modules/reagents/chemistry/equilibrium.dm | 8 +++- .../reagents/chemistry/holder/holder.dm | 40 +++++++------------ .../reagents/chemistry/holder/reactions.dm | 2 - 4 files changed, 22 insertions(+), 32 deletions(-) diff --git a/code/modules/antagonists/wizard/equipment/teleport_rod.dm b/code/modules/antagonists/wizard/equipment/teleport_rod.dm index 3c41cae5257..79df35ca25a 100644 --- a/code/modules/antagonists/wizard/equipment/teleport_rod.dm +++ b/code/modules/antagonists/wizard/equipment/teleport_rod.dm @@ -107,8 +107,8 @@ // (Primarily a way to prevent cheese with damage healing chem mixes, // but also serves as a counter-counter to stuff like mute toxin.) var/obj/item/organ/user_stomach = user.get_organ_slot(ORGAN_SLOT_STOMACH) - user.reagents?.remove_all_direct(0.33) - user_stomach?.reagents?.remove_all_direct(0.33) + user.reagents?.remove_all(0.33, relative = TRUE) + user_stomach?.reagents?.remove_all(0.33, relative = TRUE) if(user.has_status_effect(/datum/status_effect/teleport_flux/perma)) return diff --git a/code/modules/reagents/chemistry/equilibrium.dm b/code/modules/reagents/chemistry/equilibrium.dm index c3ccc000207..4ab88b3caa6 100644 --- a/code/modules/reagents/chemistry/equilibrium.dm +++ b/code/modules/reagents/chemistry/equilibrium.dm @@ -390,11 +390,15 @@ reaction_quality = purity //post reaction checks - if(!(check_fail_states(total_step_added))) + if(!check_fail_states(total_step_added)) to_delete = TRUE return //If the volume of reagents created(total_step_added) >= volume of reagents still to be created(step_target_vol) then end //i.e. we have created all the reagents needed for this reaction - if(total_step_added >= step_target_vol) + //This is only accurate when a single reaction is present and we don't have multiple reactions where + //reaction B consumes the products formed from reaction A(which can happen in add_reagent() as it also triggers handle_reactions() which can consume the reagent just added) + //because total_step_added will be higher than the actual volume that was created leading to the reaction ending early + //and yielding less products than intended + if(total_step_added >= step_target_vol && length(holder.reaction_list) == 1) to_delete = TRUE diff --git a/code/modules/reagents/chemistry/holder/holder.dm b/code/modules/reagents/chemistry/holder/holder.dm index e6a861b851d..d32cd92361b 100644 --- a/code/modules/reagents/chemistry/holder/holder.dm +++ b/code/modules/reagents/chemistry/holder/holder.dm @@ -304,54 +304,42 @@ return total_removed //this should be amount unless the loop is prematurely broken, in which case it'll be lower. It shouldn't ever go OVER amount. /** - * Removes all reagents by an amount equal to - * [amount specified] / total volume present in this holder + * Removes all reagents either proportionally(amount is the direct volume to remove) + * when proportional the total volume of all reagents removed will equal to amount + * or relatively(amount is a percentile between 0->1) when relative amount is the % + * of each reagent to be removed + * * Arguments * - * * amount - the volume of each reagent + * * amount - the amount to remove + * * relative - if TRUE amount is treated as an percentage between 0->1. If FALSE amount is the direct volume to remove */ - -/datum/reagents/proc/remove_all(amount = 1) +/datum/reagents/proc/remove_all(amount = 1, relative = FALSE) if(!total_volume) return FALSE if(!IS_FINITE(amount)) stack_trace("non finite amount passed to remove all reagents [amount]") return FALSE + if(relative && (amount < 0 || amount > 1)) + stack_trace("illegal percentage value passed to remove all reagents [amount]") + return FALSE amount = round(amount, CHEMICAL_QUANTISATION_LEVEL) if(amount <= 0) return FALSE var/list/cached_reagents = reagent_list - var/part = amount / total_volume var/total_removed_amount = 0 - + var/part = amount + if(!relative) + part /= total_volume for(var/datum/reagent/reagent as anything in cached_reagents) total_removed_amount += remove_reagent(reagent.type, reagent.volume * part) handle_reactions() return round(total_removed_amount, CHEMICAL_VOLUME_ROUNDING) -/** - * Like remove_all but removes a percentage of every reagent directly rather than by volume - * - * Arguments - * * percentage - the percentage of each reagent to remove - */ -/datum/reagents/proc/remove_all_direct(percentage = 0.5) - if(!total_volume) - return 0 - if(percentage <= 0) - stack_trace("non positive percentage passed to remove all reagents direct [percentage]") - return 0 - var/total_removed_amount = 0 - for(var/datum/reagent/reagent as anything in reagent_list) - total_removed_amount += remove_reagent(reagent.type, reagent.volume * percentage) - - handle_reactions() - return round(total_removed_amount, CHEMICAL_VOLUME_ROUNDING) - /** * Removes an specific reagent from this holder * Arguments diff --git a/code/modules/reagents/chemistry/holder/reactions.dm b/code/modules/reagents/chemistry/holder/reactions.dm index 8005e45427b..5d0e5480e5a 100644 --- a/code/modules/reagents/chemistry/holder/reactions.dm +++ b/code/modules/reagents/chemistry/holder/reactions.dm @@ -97,8 +97,6 @@ if((selected_reaction.reaction_flags & REACTION_INSTANT) || (flags & REAGENT_HOLDER_INSTANT_REACT)) //If we have instant reactions, we process them here instant_react(selected_reaction) .++ - update_total() - continue else var/exists = FALSE for(var/datum/equilibrium/E_exist as anything in reaction_list)