From c27a90da71a2a7d6c9fa9646c17fabc40325bd43 Mon Sep 17 00:00:00 2001 From: SyncIt21 <110812394+SyncIt21@users.noreply.github.com> Date: Tue, 29 Aug 2023 22:16:45 +0530 Subject: [PATCH] [NO GBP]Refactors how sheets are ejected from mat container (#77945) ## About The Pull Request As #77936 pointed out an infinite while loop occurs in the recycler. This was caused due to my recent mat changes as follows. When the recycler eats up an item. If it found any material inside, it would attempt to eject any sheets formed from it over here https://github.com/tgstation/tgstation/blob/ed71735fb3d60e834abf5771525c98809a0bb6f5/code/game/machinery/recycler.dm#L199 Inside this proc i first create the sheets in `null space` why is explained in the comment https://github.com/tgstation/tgstation/blob/ed71735fb3d60e834abf5771525c98809a0bb6f5/code/datums/components/material/material_container.dm#L599-L600 After doing logging & stuff i then would move the sheets back into its target turf so it could get merged again https://github.com/tgstation/tgstation/blob/ed71735fb3d60e834abf5771525c98809a0bb6f5/code/datums/components/material/material_container.dm#L607-L608 Unfortunately this would trigger `COMSIG_ATOM_ENTERED` signal which the recycler is hooked onto. The recycler would then eat the newly ejected sheets again, which would cause more sheets to be ejected, which would again trigger the signal `COMSIG_ATOM_ENTERED` and you have your infinite loop. I did this because i had no clue how to stop the stack from merging when it got spawned and this was a poor solution, so upon examining the code i found there is a var to specifically stops stacks from merging https://github.com/tgstation/tgstation/blob/ed71735fb3d60e834abf5771525c98809a0bb6f5/code/game/objects/items/stacks/stack.dm#L81 Yup there is specific param called `merge=TRUE` which allows you to explicitly stop stacks from merging. I now use this var and manually merge stacks afterwards without calling `forceMove()` so it doesn't trigger the recycler anymore which caused the infinite loops. ## Why It's good for the game Writing checks to see if an object came from null space is weird. It makes the code difficult to reason with as such situations never happen. Now it makes sense for an atom to be moved "into" null space for e.g. when it gets deleted but for an atom to "return back from it" never happens and you shouldn't be bothered with figuring out how that happened. ## Changelog :cl: refactor: sheets are merged in a better way when ejected from machines & material related stuff /:cl: --- .../components/material/material_container.dm | 18 ++++++++++++++---- code/game/machinery/recycler.dm | 5 ----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/code/datums/components/material/material_container.dm b/code/datums/components/material/material_container.dm index 7f07c6cb91d..6b23fcce6f1 100644 --- a/code/datums/components/material/material_container.dm +++ b/code/datums/components/material/material_container.dm @@ -596,16 +596,26 @@ //eject sheets based on available amount after each iteration var/count = 0 while(sheet_amt > 0) - //create sheets in null space so it doesn't merge & delete itself - var/obj/item/stack/sheet/new_sheets = new material.sheet_type(null, min(sheet_amt, MAX_STACK_SIZE), FALSE, list((material) = SHEET_MATERIAL_AMOUNT)) + //don't merge yet. we need to do stuff with it first + var/obj/item/stack/sheet/new_sheets = new material.sheet_type(target, min(sheet_amt, MAX_STACK_SIZE), FALSE) count += new_sheets.amount //use material & deduct work needed use_amount_mat(new_sheets.amount * SHEET_MATERIAL_AMOUNT, material) sheet_amt -= new_sheets.amount //send signal SEND_SIGNAL(src, COMSIG_MATCONTAINER_SHEETS_RETRIEVED, new_sheets, context) - //now move to target so it gets merged - new_sheets.forceMove(target) + //no point merging anything into an already full stack + if(new_sheets.amount == new_sheets.max_amount) + continue + //now we can merge since we are done with it + for(var/obj/item/stack/item_stack in target) + if(item_stack == new_sheets || item_stack.type != material.sheet_type) //don't merge with self or different type + continue + //speed merge + var/merge_amount = min(item_stack.amount, new_sheets.max_amount - new_sheets.get_amount()) + item_stack.use(merge_amount) + new_sheets.add(merge_amount) + break return count /** diff --git a/code/game/machinery/recycler.dm b/code/game/machinery/recycler.dm index ddf55388d0b..cc6ead40627 100644 --- a/code/game/machinery/recycler.dm +++ b/code/game/machinery/recycler.dm @@ -116,11 +116,6 @@ /obj/machinery/recycler/proc/on_entered(datum/source, atom/movable/enterer, old_loc) SIGNAL_HANDLER - // This is explicitly so we avoid processing items that are entering from nullspace, - // to avoid infinite loops. - if(!old_loc) - return - INVOKE_ASYNC(src, PROC_REF(eat), enterer) /obj/machinery/recycler/proc/eat(atom/movable/morsel, sound=TRUE)