From 043677c6f69d09ac67ef112e56e17b81611d5a61 Mon Sep 17 00:00:00 2001 From: Ghom <42542238+Ghommie@users.noreply.github.com> Date: Fri, 5 Dec 2025 00:40:16 +0100 Subject: [PATCH] Storing objects with slowdown in a backpack or belt (or any such storage) now properly updates your speed (#93967) ## About The Pull Request Another one of those things that I've noticed when playing around with fish tanks; The slowdown lingered even when the fish tank (which depends on the total weight of fish inside it) was no longer held and was only updated another item is equipped or held. This is because `attempt_insert` doesn't end up calling `DoUnEquip`, which along with `equip_to_slot`, is one of the cornerstones of the whole inventory system that we have had for over a decade. Luckily, this doesn't break things entirely because `item/doMove` seems to have a fallback, but it only covers held items and only does half of what `DoUnEquip` does, because it's its own copypaste code, disconnected from the standard unequip call stack. I've done some changes to make sure `DoUnEquip` is always called on `doMove` if we find that the item still has the IN_INVENTORY flag. I've also updated the code comment for it as well, to emphasize that the measure is a fallback and not an excuse to call forceMove or Move if we know that the object is held or equipped on a mob. If something doesn't work, it'll be likely caught by the CI (it's a core feature of the game after all) or stack traces. Also, despite equipment slowdown supporting all mob types, when equipping/unequipping items it's only applied to carbon mobs. This is not _strictly_ a contributing factor to the titled issue but it still limits a balance feature that ought to affect all mobs with hands and/or equipment slots. ## Why It's Good For The Game Fixing issues with inventory and storages. Hopefully improving and modernizing years old code a little. --- code/datums/storage/storage.dm | 12 +++--- code/game/objects/items.dm | 37 ++++++++----------- code/game/objects/items/shooting_range.dm | 2 +- code/modules/mob/inventory.dm | 15 ++++++-- .../mob/living/basic/drone/inventory.dm | 2 +- .../mob/living/carbon/human/inventory.dm | 3 -- code/modules/mob/living/carbon/inventory.dm | 9 +---- code/modules/mob/mob.dm | 7 ---- 8 files changed, 39 insertions(+), 48 deletions(-) diff --git a/code/datums/storage/storage.dm b/code/datums/storage/storage.dm index 4c36ed6394f..f60cfccac58 100644 --- a/code/datums/storage/storage.dm +++ b/code/datums/storage/storage.dm @@ -492,11 +492,15 @@ GLOBAL_LIST_EMPTY(cached_storage_typecaches) SEND_SIGNAL(parent, COMSIG_ATOM_STORED_ITEM, to_insert, user, force) SEND_SIGNAL(src, COMSIG_STORAGE_STORED_ITEM, to_insert, user, force) - to_insert.forceMove(real_location) - item_insertion_feedback(user, to_insert, override) - parent.update_appearance() + if(ismob(to_insert.loc)) + var/mob/item_carrier = to_insert.loc + item_carrier.transferItemToLoc(to_insert, parent, animated = FALSE) // This allows has_unequipped() to be properly called. + else + to_insert.forceMove(real_location) if(get(real_location, /mob) != user) to_insert.do_pickup_animation(real_location, user) + item_insertion_feedback(user, to_insert, override) + parent.update_appearance() return TRUE /// Since items inside storages ignore transparency for QOL reasons, we're tracking when things are dropped onto them instead of our UI elements @@ -741,8 +745,6 @@ GLOBAL_LIST_EMPTY(cached_storage_typecaches) return if(collection_mode == COLLECT_ONE) - if(thing.loc == user) - user.dropItemToGround(thing, silent = TRUE) //this is nessassary to update any inventory slot it is attached to attempt_insert(thing, user) return COMPONENT_CANCEL_ATTACK_CHAIN diff --git a/code/game/objects/items.dm b/code/game/objects/items.dm index 3c3d0b084d5..31d158374f1 100644 --- a/code/game/objects/items.dm +++ b/code/game/objects/items.dm @@ -716,7 +716,6 @@ if(item_flags & DROPDEL && !QDELETED(src)) qdel(src) - item_flags &= ~IN_INVENTORY UnregisterSignal(src, list(SIGNAL_ADDTRAIT(TRAIT_NO_WORN_ICON), SIGNAL_REMOVETRAIT(TRAIT_NO_WORN_ICON))) SEND_SIGNAL(src, COMSIG_ITEM_DROPPED, user) SEND_SIGNAL(user, COMSIG_MOB_DROPPED_ITEM, src) @@ -728,7 +727,6 @@ SHOULD_CALL_PARENT(TRUE) SEND_SIGNAL(src, COMSIG_ITEM_PICKUP, user) SEND_SIGNAL(user, COMSIG_LIVING_PICKED_UP_ITEM, src) - item_flags |= IN_INVENTORY /// called when "found" in pockets and storage items. Returns 1 if the search should end. /obj/item/proc/on_found(mob/finder) @@ -781,7 +779,6 @@ for(var/datum/action/action as anything in actions) give_item_action(action, user, slot) - item_flags |= IN_INVENTORY RegisterSignals(src, list(SIGNAL_ADDTRAIT(TRAIT_NO_WORN_ICON), SIGNAL_REMOVETRAIT(TRAIT_NO_WORN_ICON)), PROC_REF(update_slot_icon), override = TRUE) if(!initial && (slot_flags & slot) && (play_equip_sound())) @@ -1297,28 +1294,26 @@ /obj/item/proc/get_part_rating() return 0 +/** + * this proc override makes sure that even if DoUnEquip is not properly called through the appropriate channels, + * it'll still be called if we find that the item has the IN_INVENTORY flag. + * + * THIS IS BY NO MEAN AN EXCUSE TO KNOWINGLY AVOID CALLING THE RIGHT PROCS FOR INVENTORY MANAGEMENT, + * BUT A FALLBACK IN THE CASE WE MISTAKINGLY DON'T, TO MAKE SURE THINGS WORK AS INTENDED SINCE + * INVENTORY MANAGEMENT HAS A LOT MORE TO IT THAN JUST CALLING A PROC OR TWO MANUALLY. + */ /obj/item/doMove(atom/destination) - if (!ismob(loc)) + if (!(item_flags & IN_INVENTORY)) + return ..() + + if(!ismob(loc)) + stack_trace("[src] had the IN_INVENTORY flag but the location was not a mob!") + item_flags &= ~IN_INVENTORY return ..() var/mob/owner = loc - var/hand_index = owner.get_held_index_of_item(src) - if(!hand_index) - return ..() - - owner.held_items[hand_index] = null - owner.update_held_items() - if(owner.client) - owner.client.screen -= src - if(owner.observers?.len) - for(var/mob/dead/observe as anything in owner.observers) - if(observe.client) - observe.client.screen -= src - layer = initial(layer) - SET_PLANE_IMPLICIT(src, initial(plane)) - appearance_flags &= ~NO_CLIENT_COLOR - dropped(owner, FALSE) - return ..() + // This should remove the IN_INVENTORY flag. Otherwise we'll end up having a loop + owner.transferItemToLoc(src, destination, force = TRUE, silent = TRUE, animated = FALSE) /obj/item/proc/canStrip(mob/stripper, mob/owner) SHOULD_BE_PURE(TRUE) diff --git a/code/game/objects/items/shooting_range.dm b/code/game/objects/items/shooting_range.dm index 9dc2872588b..32b8c08747d 100644 --- a/code/game/objects/items/shooting_range.dm +++ b/code/game/objects/items/shooting_range.dm @@ -5,7 +5,7 @@ icon_state = "target_h" density = FALSE max_integrity = 1800 - item_flags = CAN_BE_HIT + obj_flags = CAN_BE_HIT /// Lazylist to keep track of bullet-hole overlays. var/list/bullethole_overlays diff --git a/code/modules/mob/inventory.dm b/code/modules/mob/inventory.dm index 070e735eddb..53f312ccdc6 100644 --- a/code/modules/mob/inventory.dm +++ b/code/modules/mob/inventory.dm @@ -181,7 +181,7 @@ I.do_pickup_animation(src) if(get_item_for_held_index(hand_index)) dropItemToGround(get_item_for_held_index(hand_index), force = TRUE) - I.forceMove(src) + I.forceMove(src) //this has to come before has_equipped() is called held_items[hand_index] = I SET_PLANE_EXPLICIT(I, ABOVE_HUD_PLANE, src) if(I.pulledby) @@ -440,6 +440,7 @@ item_dropping.layer = initial(item_dropping.layer) SET_PLANE_EXPLICIT(item_dropping, initial(item_dropping.plane), newloc) item_dropping.appearance_flags &= ~NO_CLIENT_COLOR + item_dropping.item_flags &= ~IN_INVENTORY //This has to come before MoveToNullspace/forceMove is called if(!no_move && !(item_dropping.item_flags & DROPDEL)) //item may be moved/qdel'd immedietely, don't bother moving it if (isnull(newloc)) item_dropping.moveToNullspace() @@ -458,7 +459,7 @@ * * Optional - include_flags, (see obj.flags.dm) describes which optional things to include or not (pockets, accessories, held items) */ -/mob/living/proc/get_equipped_items(include_flags = NONE) +/mob/proc/get_equipped_items(include_flags = NONE) var/list/items = list() for(var/obj/item/item_contents in contents) if(item_contents.item_flags & IN_INVENTORY) @@ -472,6 +473,10 @@ return items +///Get all items in our possession that should affect our movespeed +/mob/proc/get_equipped_speed_mod_items() + return get_equipped_items(INCLUDE_HELD|INCLUDE_ABSTRACT|INCLUDE_PROSTHETICS) + /** * Returns the items that were successfully unequipped. */ @@ -531,12 +536,16 @@ /// This proc is called after an item has been successfully handled and equipped to a slot. /mob/proc/has_equipped(obj/item/item, slot, initial = FALSE) SHOULD_CALL_PARENT(TRUE) - return item.on_equipped(src, slot, initial) + item.item_flags |= IN_INVENTORY + . = item.on_equipped(src, slot, initial) + if(.) + update_equipment_speed_mods() /// This proc is called after an item has been removed from a mob but before it has been officially deslotted. /mob/proc/has_unequipped(obj/item/item, silent = FALSE) SHOULD_CALL_PARENT(TRUE) item.dropped(src, silent) + update_equipment_speed_mods() return TRUE /** diff --git a/code/modules/mob/living/basic/drone/inventory.dm b/code/modules/mob/living/basic/drone/inventory.dm index c6c7aa5da95..acacdb0e265 100644 --- a/code/modules/mob/living/basic/drone/inventory.dm +++ b/code/modules/mob/living/basic/drone/inventory.dm @@ -59,7 +59,7 @@ equipping.pulledby.stop_pulling() equipping.screen_loc = null // will get moved if inventory is visible - equipping.forceMove(src) + equipping.forceMove(src) //This has to come before has_equipped is called. SET_PLANE_EXPLICIT(equipping, ABOVE_HUD_PLANE, src) switch(slot) diff --git a/code/modules/mob/living/carbon/human/inventory.dm b/code/modules/mob/living/carbon/human/inventory.dm index 0e08086cf6e..1346390b985 100644 --- a/code/modules/mob/living/carbon/human/inventory.dm +++ b/code/modules/mob/living/carbon/human/inventory.dm @@ -207,9 +207,6 @@ return not_handled //For future deeper overrides -/mob/living/carbon/human/get_equipped_speed_mod_items() - return ..() - list(l_store, r_store, s_store) - /mob/living/carbon/human/doUnEquip(obj/item/item_dropping, force, newloc, no_move, invdrop = TRUE, silent = FALSE) . = ..() //See mob.dm for an explanation on this and some rage about people copypasting instead of calling ..() like they should. if(!. || !item_dropping) diff --git a/code/modules/mob/living/carbon/inventory.dm b/code/modules/mob/living/carbon/inventory.dm index 01a39ebf7b2..218e8544d9f 100644 --- a/code/modules/mob/living/carbon/inventory.dm +++ b/code/modules/mob/living/carbon/inventory.dm @@ -118,7 +118,7 @@ for(var/mob/dead/observe as anything in observers) observe.client?.screen -= equipping - equipping.forceMove(src) + equipping.forceMove(src) //This has to come before has_equipped is called. SET_PLANE_EXPLICIT(equipping, ABOVE_HUD_PLANE, src) equipping.appearance_flags |= NO_CLIENT_COLOR var/not_handled = FALSE @@ -163,15 +163,11 @@ return not_handled -/mob/living/carbon/get_equipped_speed_mod_items() - return ..() + get_equipped_items(INCLUDE_ABSTRACT) - /mob/living/carbon/has_equipped(obj/item/item, slot, initial = FALSE) . = ..() if(!.) return - update_equipment_speed_mods() hud_used?.update_locked_slots() if(!(slot & item.slot_flags)) // Things below only update if slotted in (ie: not held) return @@ -180,11 +176,10 @@ add_item_coverage(item) /mob/living/carbon/has_unequipped(obj/item/item) - . = ..() // NB: ATP the item is still in the slot, but no longer has the IN_INVENTORY flag (so is not returned by get_equipped_items) + . = ..() if(!.) return - update_equipment_speed_mods() hud_used?.update_locked_slots() if(item.hair_mask) update_body() diff --git a/code/modules/mob/mob.dm b/code/modules/mob/mob.dm index 6ac734a5066..20790b40ea3 100644 --- a/code/modules/mob/mob.dm +++ b/code/modules/mob/mob.dm @@ -1562,13 +1562,6 @@ else remove_movespeed_modifier(/datum/movespeed_modifier/equipment_speedmod) -///Get all items in our possession that should affect our movespeed -/mob/proc/get_equipped_speed_mod_items() - . = list() - for(var/obj/item/thing in held_items) - if(thing.item_flags & SLOWS_WHILE_IN_HAND) - . += thing - /mob/proc/set_stat(new_stat) if(new_stat == stat) return