mirror of
https://github.com/Bubberstation/Bubberstation.git
synced 2026-08-21 20:20:33 +01:00
## About The Pull Request Refactors the way we listen for reagent changes. The changes made can be listed as points **1. Removes `COMSIG_REAGENTS_PRE_ADD_REAGENT`** Used to stop new reagents from being added to the holder, its only application is with the BRPED to stop inserting reagents into beakers/cells stored inside it. Rather than using this signal a cleaner solution is to simply remove the component part's reagent holders' flags which allow us to insert reagents into it(i.e. `REFILABLE`, `INJECTIBLE`, `DRAINABLE`) and restore them back when that part is removed thus achieving the same results. Thus `add_reagent()` is now slightly faster because it no longer uses this signal **2. Removes every other signal used by the reagent holder** Removes pretty much every other signal used by `holder.dm` which are `COMSIG_REAGENTS_[NEW_REAGENT, ADD_REAGENT, DEL_REAGENT, REM_REAGENT, CLEAR_REAGENTS]` While yes, it is true that all these signals are unique & serve a specific purpose the problem is no object in code respects their uniqueness & instead clumps them up all together & hooks them onto one proc to listen for "reagent changes". You see this code pattern repeated in so many places https://github.com/tgstation/tgstation/blob/9277364ef6449262e2c693ff6817925e074c47ce/code/modules/power/power_store.dm#L105 Not only does this look ugly but it also has a memory overhead (4 to 5 signal slots all performing the same action which is a lot compared to the solution i implemented below). Bonus is that "none" of the parameters passed to this proc are used so they go to waste as well. So after removing a ton of code we need something that can still make the code function which brings us to point 3 **3. Adds a new signal `COMSIG_REAGENTS_HOLDER_UPDATED` to rule them all** So if all objects in game are listening for "reagent changes"[adding/removing, reagents] then we need to look at the proc that is always called during these changes & that is none other than `update_total()` so we let that send out a signal and cause all objects to hook onto this 1 signal instead of 4 to 5 signals as explained in point 2 ## Why It's Good For The Game This section isn't necessary but i want us to better appreciate both the code & performance benifits of this PR. 1. First of all its waaaay less code and signals to worry about. Just look at the number of lines of code removed compared to added. Nothing more to say 2. Overhead of `RegisterSignal` compared to `RegisterSignals` is less for obvious reasons 3. `remove_all` is significantly faster as it no longer calls `remove_reagent()`[which in turn calls `update_total()` & `handle_reactions()` per call & uses a for loop so its a nested for loop of doom] for every reagent it removes, instead it does the work by itself & calls the above 2 procs just once 4. Usually when a reagent is deleted it calls `COMSIG_REAGENTS_REM_REAGENT` & `COMSIG_REAGENTS_DEL_REAGENT`. So if you have a holder with like 3 reagents upon transferring/deleting them you get a total of 6 signal calls!!. Now it's just 3(when using `trans_to`) and just 1 when using `remove_all/clear_reagents`. Need i say more no ## Changelog 🆑 fix: hydrophonics circuit component actually sets output level when reagents are changed in the tray refactor: refactors how code listens for reagent changes. Report bugs on github /🆑
88 lines
3.7 KiB
Plaintext
88 lines
3.7 KiB
Plaintext
/datum/component/plumbing/reaction_chamber
|
|
demand_connects = NORTH
|
|
supply_connects = SOUTH
|
|
|
|
/datum/component/plumbing/reaction_chamber/Initialize(start=TRUE, _ducting_layer, _turn_connects=TRUE, datum/reagents/custom_receiver)
|
|
. = ..()
|
|
if(!istype(parent, /obj/machinery/plumbing/reaction_chamber))
|
|
return COMPONENT_INCOMPATIBLE
|
|
|
|
/datum/component/plumbing/reaction_chamber/can_give(amount, reagent, datum/ductnet/net)
|
|
. = FALSE
|
|
|
|
var/obj/machinery/plumbing/reaction_chamber/reaction_chamber = parent
|
|
|
|
//cannot give when we outselves are requesting or reacting the reagents
|
|
if(amount <= 0 || !reagents.total_volume || !reaction_chamber.emptying || reagents.is_reacting)
|
|
return
|
|
|
|
//check to see if we can give catalysts only if they are in excess
|
|
var/list/datum/reagent/catalysts = reaction_chamber.catalysts
|
|
for(var/datum/reagent/chemical as anything in reagents.reagent_list)
|
|
if(reagent && chemical.type != reagent)
|
|
continue
|
|
|
|
//we have the exact amounts so no excess to spare
|
|
if(chemical.volume <= (catalysts[chemical.type] || 0))
|
|
if(reagent)
|
|
break
|
|
else
|
|
continue
|
|
|
|
//atleast 1 reagent to give so take whatever
|
|
return TRUE
|
|
|
|
/datum/component/plumbing/reaction_chamber/send_request(dir)
|
|
var/obj/machinery/plumbing/reaction_chamber/chamber = parent
|
|
|
|
if(chamber.emptying)
|
|
return
|
|
|
|
//take in reagents
|
|
var/present_amount
|
|
var/diff
|
|
var/list/datum/reagent/required_reagents = chamber.catalysts | chamber.required_reagents
|
|
for(var/datum/reagent/required_reagent as anything in required_reagents)
|
|
//find how much amount is already present if at all and get the reagent reference
|
|
present_amount = 0
|
|
for(var/datum/reagent/present_reagent as anything in reagents.reagent_list)
|
|
if(required_reagent == present_reagent.type)
|
|
present_amount = present_reagent.volume
|
|
break
|
|
|
|
//compute how much more is needed
|
|
diff = min(required_reagents[required_reagent] - present_amount, MACHINE_REAGENT_TRANSFER)
|
|
if(diff >= CHEMICAL_QUANTISATION_LEVEL) // the closest we can ask for so values like 0.9999 become 1
|
|
process_request(diff, required_reagent, dir)
|
|
if(!chamber.catalysts[required_reagent]) //only block if not a catalyst as they can come in whenever they are available
|
|
return
|
|
|
|
reagents.flags &= ~NO_REACT
|
|
reagents.handle_reactions()
|
|
|
|
chamber.emptying = TRUE //If we move this up, it'll instantly get turned off since any reaction always sets the reagent_total to zero. Other option is make the reaction update
|
|
//everything for every chemical removed, wich isn't a good option either.
|
|
chamber.on_reagent_change(reagents) //We need to check it now, because some reactions leave nothing left.
|
|
if(chamber.emptying) //if we are still emptying then keep checking for reagents until we are emptied out
|
|
chamber.RegisterSignal(reagents, COMSIG_REAGENTS_HOLDER_UPDATED, TYPE_PROC_REF(/obj/machinery/plumbing/reaction_chamber, on_reagent_change))
|
|
|
|
///Special connect that we currently use for reaction chambers. Being used so we can keep certain inputs separate, like into a special internal acid container
|
|
/datum/component/plumbing/acidic_input
|
|
demand_connects = WEST
|
|
demand_color = COLOR_YELLOW
|
|
ducting_layer = SECOND_DUCT_LAYER
|
|
|
|
/datum/component/plumbing/acidic_input/send_request(dir)
|
|
process_request(reagent = /datum/reagent/reaction_agent/acidic_buffer, dir = dir)
|
|
|
|
///Special connect that we currently use for reaction chambers. Being used so we can keep certain inputs separate, like into a special internal base container
|
|
/datum/component/plumbing/alkaline_input
|
|
demand_connects = EAST
|
|
demand_color = COLOR_VIBRANT_LIME
|
|
ducting_layer = FOURTH_DUCT_LAYER
|
|
|
|
/datum/component/plumbing/alkaline_input/send_request(dir)
|
|
process_request(reagent = /datum/reagent/reaction_agent/basic_buffer, dir = dir)
|
|
|
|
|