From 67ba6cb2f33378a76f8a9dd66b204bccb8b5e771 Mon Sep 17 00:00:00 2001 From: SyncIt21 <110812394+SyncIt21@users.noreply.github.com> Date: Mon, 17 Feb 2025 05:15:13 +0530 Subject: [PATCH] Final reagent enhancements (#89289) ## About The Pull Request **1. Code Improvements** - `get_reagent_log_string()` now won't round reagent volumes cause they are already rounded by `update_total()` so its slightly faster - `trans_to()` now logs reagents faster as it does it in place without the overhead of passing the list to `get_external_reagent_log_string()` which parses our list into another list which is just unessassary - `trans_to()` is now faster performance wise as it no longer relies on `remove_reagent()` to remove the reagent - Merged `multiply_single_reagent()` into a single proc `multiply()` which does the job for both single & multiple reagents to reduce code. it now no longer uses proc `remove_reagent()` making it faster when reducing reagents - `copy_to()` no longer rounds its return value thus improving performance & won't return false negatives for values lesser than 0.01 **2. Fixes** - Fixes `multiply_single_reagent()` multiplying reagent subtypes as well instead of just the target type as it uses `locate()` - Fixes `multiply_reagents()` multiplying reagents twice. Notice how it would call `_multiply_reagent()` & then again do the exact same thing as that proc in the next line https://github.com/tgstation/tgstation/blob/dfe9f50ad74507dabf8b9a534f385cad170c53c1/code/modules/reagents/chemistry/holder/holder.dm#L605-L609 - `convert_reagent()` actually uses the weighted ph. I was wrong in what I assumed `override_base_ph` did Just lesser & faster code overall ## Changelog :cl: fix: multiplying reagents in cases like fishing & chem splash will yield accurate results. New amounts differ from present values fix: converting reagents actually yields correct ph again code: improved performance of reagent logging code: copying reagents won't return false negatives for values lesser than 0.01 /:cl: --- code/modules/fishing/fish/_fish.dm | 7 +- code/modules/fishing/fish/fish_traits.dm | 2 +- .../plumbing/plumbers/_plumb_reagents.dm | 24 +--- code/modules/reagents/chem_splash.dm | 4 +- .../reagents/chemistry/holder/holder.dm | 113 +++++++----------- .../reagents/chemistry/holder/properties.dm | 2 +- .../chemistry/machinery/chem_recipe_debug.dm | 2 +- 7 files changed, 58 insertions(+), 96 deletions(-) diff --git a/code/modules/fishing/fish/_fish.dm b/code/modules/fishing/fish/_fish.dm index 9b32dde5328..664ec6808f2 100644 --- a/code/modules/fishing/fish/_fish.dm +++ b/code/modules/fishing/fish/_fish.dm @@ -338,7 +338,7 @@ adjust_reagents_capacity((protein_volume - old_blood_volume) * volume_mult) ///Add the extra nutriment if(protein) - reagents.multiply_single_reagent(/datum/reagent/consumable/nutriment/protein, 2) + reagents.multiply(2, /datum/reagent/consumable/nutriment/protein) var/datum/component/edible/edible = GetComponent(/datum/component/edible) edible.foodtypes &= ~(RAW|GORE) @@ -457,8 +457,7 @@ if(!result_reagent) created.reagents.add_reagent(reagent.type, transfer_vol, reagents.copy_data(reagent), reagents.chem_temp, reagent.purity, reagent.ph, no_react = TRUE) continue - var/multiplier = transfer_vol / result_reagent.volume - created.reagents.multiply_single_reagent(reagent.type, multiplier) + created.reagents.multiply(transfer_vol / result_reagent.volume, reagent.type) return ..() /obj/item/fish/update_icon_state() @@ -609,7 +608,7 @@ var/amount_to_gen = bites_left / initial_bites_left * multiplier generate_fish_reagents(amount_to_gen) else - reagents.multiply_reagents(new_weight_ratio) + reagents.multiply(new_weight_ratio) adjust_reagents_capacity(volume_diff) weight = new_weight diff --git a/code/modules/fishing/fish/fish_traits.dm b/code/modules/fishing/fish/fish_traits.dm index f6ca105f1e2..d3a978344b8 100644 --- a/code/modules/fishing/fish/fish_traits.dm +++ b/code/modules/fishing/fish/fish_traits.dm @@ -669,7 +669,7 @@ GLOBAL_LIST_INIT(spontaneous_fish_traits, populate_spontaneous_fish_traits()) if(cooked_time >= FISH_SAFE_COOKING_DURATION) fish.reagents.del_reagent(/datum/reagent/consumable/liquidelectricity) else - fish.reagents.multiply_single_reagent(/datum/reagent/consumable/liquidelectricity, 0.66) + fish.reagents.multiply(0.66, /datum/reagent/consumable/liquidelectricity) /datum/fish_trait/electrogenesis/add_reagents(obj/item/fish/fish, list/reagents) . = ..() diff --git a/code/modules/plumbing/plumbers/_plumb_reagents.dm b/code/modules/plumbing/plumbers/_plumb_reagents.dm index 92097e2e1e9..a19b5e492cd 100644 --- a/code/modules/plumbing/plumbers/_plumb_reagents.dm +++ b/code/modules/plumbing/plumbers/_plumb_reagents.dm @@ -55,7 +55,6 @@ transfer_reactions(target_holder) var/list/cached_reagents = reagent_list - var/list/reagents_to_remove = list() var/transfer_amount var/transfered_amount var/total_transfered_amount = 0 @@ -93,23 +92,18 @@ transfered_amount = target_holder.add_reagent(reagent.type, transfer_amount, copy_data(reagent), chem_temp, reagent.purity, reagent.ph, no_react = TRUE, ignore_splitting = reagent.chemical_flags & REAGENT_DONOTSPLIT) //we only handle reaction after every reagent has been transferred. if(!transfered_amount) continue - reagents_to_remove += list(list("R" = reagent, "T" = transfer_amount)) total_transfered_amount += transfered_amount if(round_robin) to_transfer -= transfered_amount + reagent.volume -= transfered_amount if(!isnull(target_id)) break - - //remove chemicals that were added above - for(var/list/data as anything in reagents_to_remove) - var/datum/reagent/reagent = data["R"] - transfer_amount = data["T"] - remove_reagent(reagent.type, transfer_amount) + update_total() //handle reactions target_holder.handle_reactions() - src.handle_reactions() + handle_reactions() return total_transfered_amount @@ -199,7 +193,6 @@ //Set up new reagents to inherit the old ongoing reactions transfer_reactions(target_holder) - var/list/reagents_to_remove = list() var/working_volume var/catalyst_volume var/transfer_amount @@ -247,22 +240,17 @@ transfered_amount = target_holder.add_reagent(reagent.type, transfer_amount, copy_data(reagent), chem_temp, reagent.purity, reagent.ph, no_react = TRUE, ignore_splitting = reagent.chemical_flags & REAGENT_DONOTSPLIT) //we only handle reaction after every reagent has been transferred. if(!transfered_amount) continue - reagents_to_remove += list(list("R" = reagent, "T" = transfer_amount)) total_transfered_amount += transfered_amount if(round_robin) to_transfer -= transfered_amount + reagent.volume -= transfered_amount if(!isnull(target_id)) break - - //remove chemicals that were added above - for(var/list/data as anything in reagents_to_remove) - var/datum/reagent/reagent = data["R"] - transfer_amount = data["T"] - remove_reagent(reagent.type, transfer_amount) + update_total() //handle reactions target_holder.handle_reactions() - src.handle_reactions() + handle_reactions() return total_transfered_amount diff --git a/code/modules/reagents/chem_splash.dm b/code/modules/reagents/chem_splash.dm index af3c11ce210..3c3cab81d27 100644 --- a/code/modules/reagents/chem_splash.dm +++ b/code/modules/reagents/chem_splash.dm @@ -43,11 +43,11 @@ tmp_holder = FALSE original_max_volume = holder.maximum_volume if(threatscale < 1) - holder.multiply_reagents(threatscale) + holder.multiply(threatscale) holder.maximum_volume = maximum_reagents * threatscale else holder.maximum_volume = maximum_reagents * threatscale - holder.multiply_reagents(threatscale) + holder.multiply(threatscale) for(var/datum/reagents/reactant as anything in reactants) reactant.trans_to(holder, reactant.total_volume, threatscale, no_react = TRUE) diff --git a/code/modules/reagents/chemistry/holder/holder.dm b/code/modules/reagents/chemistry/holder/holder.dm index 3815494afa0..d0a09c113bc 100644 --- a/code/modules/reagents/chemistry/holder/holder.dm +++ b/code/modules/reagents/chemistry/holder/holder.dm @@ -1,6 +1,3 @@ -#define REAGENT_TRANSFER_AMOUNT "amount" -#define REAGENT_PURITY "purity" - ///////////////////////////////Main reagents code///////////////////////////////////////////// /// Holder for a bunch of [/datum/reagent] @@ -377,7 +374,7 @@ //add the new target reagent with the averaged values from the source reagents if(weighted_volume > 0) update_total() - add_reagent(target_reagent_typepath, weighted_volume * multiplier, reagtemp = chem_temp, added_purity = (weighted_purity / weighted_volume), override_base_ph = TRUE, added_ph = (weighted_ph / weighted_volume)) + add_reagent(target_reagent_typepath, weighted_volume * multiplier, reagtemp = chem_temp, added_purity = (weighted_purity / weighted_volume), added_ph = (weighted_ph / weighted_volume)) /// Removes all reagents /datum/reagents/proc/clear_reagents() @@ -496,7 +493,7 @@ continue if(methods) r_to_send += reagent - reagents_to_remove += list(list("R" = reagent, "T" = transfer_amount)) + reagents_to_remove[reagent] = transfer_amount total_transfered_amount += transfered_amount if(!isnull(target_id)) @@ -507,26 +504,29 @@ target_holder.expose(isorgan(target_atom) ? target : target_atom, methods, part, show_message, r_to_send) //remove chemicals that were added above - for(var/list/data as anything in reagents_to_remove) - var/datum/reagent/reagent = data["R"] - transfer_amount = data["T"] + for(var/datum/reagent/reagent as anything in reagents_to_remove) + transfer_amount = reagents_to_remove[reagent] if(methods) reagent.on_transfer(target_atom, methods, transfer_amount) - remove_reagent(reagent.type, transfer_amount) - transfer_log[reagent.type] = list(REAGENT_TRANSFER_AMOUNT = transfer_amount, REAGENT_PURITY = reagent.purity) + reagent.volume -= transfer_amount + update_total() + transfer_log += "[reagent.type] ([transfer_amount]u, [reagent.purity] purity)" //combat log if(transferred_by && target_atom) + //logging mob holder var/atom/log_target = target_atom if(isorgan(target_atom)) var/obj/item/organ/organ_item = target_atom log_target = organ_item.owner ? organ_item.owner : organ_item log_target.add_hiddenprint(transferred_by) //log prints so admins can figure out who touched it last. - log_combat(transferred_by, log_target, "transferred reagents to", my_atom, "which had [get_external_reagent_log_string(transfer_log)]") + + //logging reagents + log_combat(transferred_by, log_target, "transferred reagents to", my_atom, "which had [english_list(transfer_log)]") if(!no_react) target_holder.handle_reactions() - src.handle_reactions() + handle_reactions() return total_transfered_amount @@ -585,54 +585,50 @@ if(!no_react) // pass over previous ongoing reactions before handle_reactions is called transfer_reactions(target_holder) - - target_holder.update_total() target_holder.handle_reactions() - return round(total_transfered_amount, CHEMICAL_VOLUME_ROUNDING) + return total_transfered_amount /** - * Multiplies the reagents inside this holder by a specific amount + * Multiplies reagents inside this holder by a specific amount * Arguments - * * multiplier - the amount to multiply each reagent by + * + * * multiplier - the amount to multiply each reagent, its a percentile value where < 1 will reduce the volume and + * * > 1 will increase the volume. Final multiplier applied to the reagent volume is (1 - multiplier) + * * datum/reagent/target_id - multiply only this reagent in this holder leaving others untouched */ -/datum/reagents/proc/multiply_reagents(multiplier = 1) +/datum/reagents/proc/multiply(multiplier = 1, datum/reagent/target_id) + if(!total_volume) + return + + multiplier = round(min(multiplier, maximum_volume / total_volume), CHEMICAL_QUANTISATION_LEVEL) + if(multiplier < 0 || multiplier == 1) + return + + if(!isnull(target_id) && !ispath(target_id)) + stack_trace("Bad reagent path [target_id] passed to multiply") + return + + var/change = (multiplier - 1) //Get the % change + var/reagent_change var/list/cached_reagents = reagent_list - if(!total_volume || multiplier == 1) - return - var/change = (multiplier - 1) //Get the % change for(var/datum/reagent/reagent as anything in cached_reagents) - _multiply_reagent(reagent, change) + if(!isnull(target_id) && reagent.type != target_id) + continue + + reagent_change = reagent.volume * change if(change > 0) - add_reagent(reagent.type, reagent.volume * change, added_purity = reagent.purity, ignore_splitting = reagent.chemical_flags & REAGENT_DONOTSPLIT) + add_reagent(reagent.type, reagent_change, added_purity = reagent.purity, added_ph = reagent.ph, no_react = TRUE, ignore_splitting = reagent.chemical_flags & REAGENT_DONOTSPLIT) else - remove_reagent(reagent.type, abs(reagent.volume * change)) //absolute value to prevent a double negative situation (removing -50% would be adding 50%) + reagent.volume += reagent_change - update_total() + if(!isnull(target_id)) + break + + if(change < 0) + update_total() handle_reactions() -/** - * Multiplies a single inside this holder by a specific amount - * Arguments - * * reagent_path - The path of the reagent we want to multiply the volume of. - * * multiplier - the amount to multiply each reagent by - */ -/datum/reagents/proc/multiply_single_reagent(reagent_path, multiplier = 1) - var/datum/reagent/reagent = locate(reagent_path) in reagent_list - if(!reagent || multiplier == 1) - return - var/change = (multiplier - 1) //Get the % change - _multiply_reagent(reagent, change) - update_total() - handle_reactions() - -///Proc containing the operations called by both multiply_reagents() and multiply_single_reagent() -/datum/reagents/proc/_multiply_reagent(datum/reagent/reagent, change) - if(change > 0) - add_reagent(reagent.type, reagent.volume * change, added_purity = reagent.purity, ignore_splitting = reagent.chemical_flags & REAGENT_DONOTSPLIT) - else - remove_reagent(reagent.type, abs(reagent.volume * change)) //absolute value to prevent a double negative situation (removing -50% would be adding 50%) - /// Updates [/datum/reagents/var/total_volume] /datum/reagents/proc/update_total() var/list/cached_reagents = reagent_list @@ -829,24 +825,6 @@ //===============================Logging========================================== -/** - * Outputs a log-friendly list of reagents based on an external reagent list. - * - * Arguments: - * * external_list - Assoc list of (reagent_type) = list(REAGENT_TRANSFER_AMOUNT = amounts, REAGENT_PURITY = purity) - */ -/datum/reagents/proc/get_external_reagent_log_string(external_list) - if(!length(external_list)) - return "no reagents" - - var/list/data = list() - - for(var/reagent_type in external_list) - var/list/qualities = external_list[reagent_type] - data += "[reagent_type] ([round(qualities[REAGENT_TRANSFER_AMOUNT], CHEMICAL_QUANTISATION_LEVEL)]u, [qualities[REAGENT_PURITY]] purity)" - - return english_list(data) - /// Outputs a log-friendly list of reagents based on the internal reagent_list. /datum/reagents/proc/get_reagent_log_string() if(!length(reagent_list)) @@ -855,9 +833,6 @@ var/list/data = list() for(var/datum/reagent/reagent as anything in reagent_list) - data += "[reagent.type] ([round(reagent.volume, CHEMICAL_QUANTISATION_LEVEL)]u, [reagent.purity] purity)" + data += "[reagent.type] [reagent.volume]u, [reagent.purity] purity)" return english_list(data) - -#undef REAGENT_TRANSFER_AMOUNT -#undef REAGENT_PURITY diff --git a/code/modules/reagents/chemistry/holder/properties.dm b/code/modules/reagents/chemistry/holder/properties.dm index 1ea16253e89..f74b5893bb5 100644 --- a/code/modules/reagents/chemistry/holder/properties.dm +++ b/code/modules/reagents/chemistry/holder/properties.dm @@ -32,7 +32,7 @@ //short cut to break when we have found our one exact type if(type_check == REAGENT_STRICT_TYPE) - return total_amount + break return total_amount diff --git a/code/modules/reagents/chemistry/machinery/chem_recipe_debug.dm b/code/modules/reagents/chemistry/machinery/chem_recipe_debug.dm index 3dfb3c6e4f5..c2591c8dd30 100644 --- a/code/modules/reagents/chemistry/machinery/chem_recipe_debug.dm +++ b/code/modules/reagents/chemistry/machinery/chem_recipe_debug.dm @@ -172,7 +172,7 @@ var/target_temperature = decode_target_temperature() if(!isnull(target_temperature)) - target_reagents.adjust_thermal_energy((target_temperature - target_reagents.chem_temp) * 0.45 * seconds_per_tick * target_reagents.heat_capacity()) + target_reagents.adjust_thermal_energy((target_temperature - target_reagents.chem_temp) * 0.4 * seconds_per_tick * target_reagents.heat_capacity()) if(use_forced_purity) target_reagents.set_all_reagents_purity(forced_purity)