diff --git a/code/__DEFINES/dcs/attack_chain_signals.dm b/code/__DEFINES/dcs/attack_chain_signals.dm index f0d6f716b095..64cbef3243a7 100644 --- a/code/__DEFINES/dcs/attack_chain_signals.dm +++ b/code/__DEFINES/dcs/attack_chain_signals.dm @@ -1,12 +1,8 @@ // MARK: Item Interactions // Return values for non-attack interactions. -#define ITEM_INTERACT_SUCCESS (1<<0) //! Cancel the rest of the attack chain, indicating success. -#define ITEM_INTERACT_BLOCKING (1<<1) //! Cancel the rest of the attack chain, without indicating success. -#define ITEM_INTERACT_SKIP_TO_ATTACK (1<<2) //! Skip the rest of the interaction chain, going straight to the attack phase. - -/// Combination return value for any item interaction that cancels the rest of the attack chain. -#define ITEM_INTERACT_ANY_BLOCKER (ITEM_INTERACT_SUCCESS | ITEM_INTERACT_BLOCKING) +#define ITEM_INTERACT_COMPLETE 1 //! Cancel the rest of the attack chain, indicating success. +#define ITEM_INTERACT_SKIP_TO_AFTER_ATTACK 2 //! Skip pre-attack and attack/attack_by, going straight to after_attack. /// Sent when this atom is clicked on by a mob with an item. /// diff --git a/code/_onclick/item_attack.dm b/code/_onclick/item_attack.dm index 1427ba832999..ad0951adb77b 100644 --- a/code/_onclick/item_attack.dm +++ b/code/_onclick/item_attack.dm @@ -2,7 +2,8 @@ * This is the proc that handles the order of an item_attack. * * The order of procs called is: - * * [/atom/proc/base_item_interaction] on the target. If it returns ITEM_INTERACT_SUCCESS or ITEM_INTERACT_BLOCKING, the chain will be stopped. + * * [/atom/proc/base_item_interaction] on the target. If it returns ITEM_INTERACT_COMPLETE, the chain will be stopped. + * If it returns ITEM_INTERACT_SKIP_TO_AFTER_ATTACK, all attack chain steps except after-attack will be skipped. * * [/obj/item/proc/pre_attack] on `src`. If this returns FINISH_ATTACK, the chain will be stopped. * * [/atom/proc/attack_by] on the target. If it returns FINISH_ATTACK, the chain will be stopped. * * [/obj/item/proc/after_attack]. The return value does not matter. @@ -14,13 +15,14 @@ var/list/modifiers = params2list(params) var/item_interact_result = target.base_item_interaction(user, src, modifiers) - if(item_interact_result & ITEM_INTERACT_SUCCESS) - return - if(item_interact_result & ITEM_INTERACT_BLOCKING) - return + switch(item_interact_result) + if(ITEM_INTERACT_COMPLETE) + return + if(ITEM_INTERACT_SKIP_TO_AFTER_ATTACK) + __after_attack_core(user, target, params, proximity_flag) + return // Attack phase - if(pre_attack(target, user, params)) return @@ -35,14 +37,8 @@ // At this point it means the attack was "successful", or at least // handled, in some way. This can mean nothing happened, this can mean the // target took damage, etc. + __after_attack_core(user, target, params, proximity_flag) - // TODO: `target` here should probably be another `!QDELETED` check. - // Preserved for backwards compatibility, may be fixed post-migration. - if(target && !QDELETED(src)) - if(new_attack_chain) - after_attack(target, user, proximity_flag, params) - else - afterattack__legacy__attackchain(target, user, proximity_flag, params) /// Called when the item is in the active hand, and clicked; alternately, there /// is an 'activate held object' verb or you can hit pagedown. @@ -137,6 +133,17 @@ if(!target.new_attack_chain) return target.attacked_by__legacy__attackchain(src, user, /* def_zone */ null) +/obj/item/proc/__after_attack_core(mob/user, atom/target, params, proximity_flag = 1) + PRIVATE_PROC(TRUE) + + // TODO: `target` here should probably be another `!QDELETED` check. + // Preserved for backwards compatibility, may be fixed post-migration. + if(target && !QDELETED(src)) + if(new_attack_chain) + after_attack(target, user, proximity_flag, params) + else + afterattack__legacy__attackchain(target, user, proximity_flag, params) + /obj/item/proc/__attack_core(mob/living/target, mob/living/user) PRIVATE_PROC(TRUE) diff --git a/code/game/objects/items/weapons/agent_id.dm b/code/game/objects/items/weapons/agent_id.dm index 5eea728393b5..8868213c3410 100644 --- a/code/game/objects/items/weapons/agent_id.dm +++ b/code/game/objects/items/weapons/agent_id.dm @@ -59,7 +59,7 @@ if(isliving(user) && user?.mind?.special_role) to_chat(usr, "The card's microscanners activate as you pass it over [I], copying its access.") access |= I.access //Don't copy access if user isn't an antag -- to prevent metagaming - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE /obj/item/card/id/syndicate/ui_act(action, list/params, datum/tgui/ui, datum/ui_state/state) if(..()) @@ -296,4 +296,4 @@ if(user.mind.special_role) to_chat(user, "The card's microscanners activate as you pass it over [I], copying its access.") access |= I.access // Don't copy access if user isn't an antag -- to prevent metagaming - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE diff --git a/code/game/objects/items/weapons/cards_ids.dm b/code/game/objects/items/weapons/cards_ids.dm index 01e5fd797da7..38416775f880 100644 --- a/code/game/objects/items/weapons/cards_ids.dm +++ b/code/game/objects/items/weapons/cards_ids.dm @@ -45,7 +45,7 @@ /obj/item/card/emag/interact_with_atom(atom/target, mob/living/user, list/modifiers) if(target.emag_act(user)) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE /obj/item/card/emag/magic_key name = "magic key" @@ -55,13 +55,13 @@ /obj/item/card/emag/magic_key/interact_with_atom(atom/target, mob/living/user, list/modifiers) if(!isairlock(target)) - return ITEM_INTERACT_BLOCKING + return ITEM_INTERACT_COMPLETE var/obj/machinery/door/D = target D.locked = FALSE D.update_icon() D.emag_act(user) qdel(src) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE /obj/item/card/cmag desc = "It's a card coated in a slurry of electromagnetic bananium." @@ -82,7 +82,7 @@ /obj/item/card/cmag/interact_with_atom(atom/target, mob/living/user, list/modifiers) if(target.cmag_act(user)) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE /obj/item/card/id name = "identification card" @@ -264,12 +264,12 @@ item_state = decal.decal_item_state qdel(decal) qdel(used) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE else if(istype(used, /obj/item/barcodescanner)) var/obj/item/barcodescanner/B = used B.scanID(src, user) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE else if(istype(used, /obj/item/stamp)) if(!stamped) @@ -277,18 +277,18 @@ stamped = 1 to_chat(user, "You stamp the ID card!") playsound(user, 'sound/items/handling/standard_stamp.ogg', 50, vary = TRUE) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE to_chat(user, "This ID has already been stamped!") - return ITEM_INTERACT_BLOCKING + return ITEM_INTERACT_COMPLETE else if(istype(used, /obj/item/card/id/guest)) attach_guest_pass(used, user) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE else if(istype(used, /obj/item/storage/wallet)) used.attackby__legacy__attackchain(src, user) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE /obj/item/card/id/AltClick(mob/user) if(user.stat || HAS_TRAIT(user, TRAIT_HANDS_BLOCKED) || !Adjacent(user)) diff --git a/code/game/objects/structures/curtains.dm b/code/game/objects/structures/curtains.dm index a9032998a514..31262011e569 100644 --- a/code/game/objects/structures/curtains.dm +++ b/code/game/objects/structures/curtains.dm @@ -43,7 +43,7 @@ /obj/structure/curtain/item_interaction(mob/living/user, obj/item/used, list/modifiers) if(istype(used, /obj/item/toy/crayon)) color = tgui_input_color(user,"Please choose a color.", "Curtain Color") - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE /obj/structure/curtain/screwdriver_act(mob/user, obj/item/I) . = TRUE diff --git a/code/modules/mining/machine_vending.dm b/code/modules/mining/machine_vending.dm index d434bb35a4dd..d82d8351d673 100644 --- a/code/modules/mining/machine_vending.dm +++ b/code/modules/mining/machine_vending.dm @@ -510,7 +510,7 @@ points = 0 else to_chat(user, "There's no points left on [src].") - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE /obj/item/card/mining_point_card/examine(mob/user) . = ..() diff --git a/code/modules/vehicle/janivehicle.dm b/code/modules/vehicle/janivehicle.dm index 34ced584b805..54183d321078 100644 --- a/code/modules/vehicle/janivehicle.dm +++ b/code/modules/vehicle/janivehicle.dm @@ -96,28 +96,28 @@ if(istype(used, /obj/item/storage/bag/trash)) if(mybag) to_chat(user, fail_msg) - return ITEM_INTERACT_BLOCKING + return ITEM_INTERACT_COMPLETE if(!user.drop_item()) - return ITEM_INTERACT_BLOCKING + return ITEM_INTERACT_COMPLETE to_chat(user, "You hook [used] onto [src].") used.forceMove(src) mybag = used update_icon(UPDATE_OVERLAYS) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE if(istype(used, /obj/item/borg/upgrade/floorbuffer)) if(buffer_installed) to_chat(user, fail_msg) - return ITEM_INTERACT_BLOCKING + return ITEM_INTERACT_COMPLETE buffer_installed = TRUE qdel(used) to_chat(user,"You upgrade [src] with [used].") update_icon(UPDATE_OVERLAYS) - return ITEM_INTERACT_SUCCESS + return ITEM_INTERACT_COMPLETE if(mybag && user.a_intent == INTENT_HELP && !is_key(used)) mybag.attackby__legacy__attackchain(used, user) - return ITEM_INTERACT_ANY_BLOCKER + return ITEM_INTERACT_COMPLETE return ..() diff --git a/code/modules/vehicle/vehicle.dm b/code/modules/vehicle/vehicle.dm index 3cfdb02ac5d6..c1484bdaa251 100644 --- a/code/modules/vehicle/vehicle.dm +++ b/code/modules/vehicle/vehicle.dm @@ -78,10 +78,10 @@ inserted_key = used else to_chat(user, "[used] seems to be stuck to your hand!") - return ITEM_INTERACT_ANY_BLOCKER + return ITEM_INTERACT_COMPLETE if(istype(used, /obj/item/borg/upgrade/vtec) && install_vtec(used, user)) - return ITEM_INTERACT_ANY_BLOCKER + return ITEM_INTERACT_COMPLETE /obj/vehicle/AltClick(mob/user) if(inserted_key && user.Adjacent(user)) diff --git a/docs/references/attack_chain.md b/docs/references/attack_chain.md index 894810b607be..3b7885355745 100644 --- a/docs/references/attack_chain.md +++ b/docs/references/attack_chain.md @@ -32,10 +32,13 @@ The core of the attack chain commences: `COMSIG_INTERACT_USER`. If any listeners request it (usually by returning a non-null value), the attack chain may end here. 5. If the target implements `item_interaction()`, it is called here, and can - return one of the `ITEM_INTERACT_` flags to end the attack chain. + either return `ITEM_INTERACT_COMPLETE` to end the attack chain, or + `ITEM_INTERACT_SKIP_TO_AFTER_ATTACK` to skip all phases of the attack chain + except for after-attack. 6. If the item being used on the target implements `interact_with_atom()`, it is - called here, and can return one of the `ITEM_INTERACT_` flags to end the - attack chain. + called here, and can either return `ITEM_INTERACT_COMPLETE` to end the attack + chain, or `ITEM_INTERACT_SKIP_TO_AFTER_ATTACK` to skip all phases of the + attack chain except for after-attack. The above steps can generally be considered the "item interaction phase", when the action is not meant to cause in-game harm to the target. If the attack chain @@ -89,12 +92,16 @@ into the attack chain. ### ITEM_INTERACT flags -One may also ask why there are several `ITEM_INTERACT_` flags if returning any -of them always results in the end of the attack chain. This is for two reasons: +One may also ask why the `ITEM_INTERACT_SKIP_TO_AFTER_ATTACK` flag is necessary. +Pre-migration, a common pattern was for an object to skip certain items in its +`attackby`, and let those items handle the interaction in their `afterattack`. +Some examples of this include: -1. To make it clear at the call site what the intent of the code is, and -2. So that in the future, if we do wish to separate the behavior of these flags, - we do not need to refactor all of the call sites. +- Mountable frames being "ignored" in `/turf/attackby`, in order to let + `/obj/item/mounted/frame/afterattack` handle its specific behavior. +- Reagent containers being "ignored" in various machines' `attackby`, in order + to let the container's `afterattack` handle reagent transfer or other specific + behavior. ## Attack Chain Refactor @@ -257,33 +264,33 @@ at each junction whenever we have handled the item interaction. if(mybag) to_chat(user, fail_msg) - return -+ return ITEM_INTERACT_BLOCKING ++ return ITEM_INTERACT_COMPLETE if(!user.drop_item()) - return -+ return ITEM_INTERACT_BLOCKING ++ return ITEM_INTERACT_COMPLETE to_chat(user, "You hook [I] onto [src].") I.forceMove(src) mybag = I update_icon(UPDATE_OVERLAYS) - return -+ return ITEM_INTERACT_SUCCESS ++ return ITEM_INTERACT_COMPLETE + if(istype(I, /obj/item/borg/upgrade/floorbuffer)) if(buffer_installed) to_chat(user, fail_msg) - return -+ return ITEM_INTERACT_BLOCKING ++ return ITEM_INTERACT_COMPLETE buffer_installed = TRUE qdel(I) to_chat(user,"You upgrade [src] with [I].") update_icon(UPDATE_OVERLAYS) - return - if(istype(I, /obj/item/borg/upgrade/vtec) && floorbuffer) -+ return ITEM_INTERACT_SUCCESS ++ return ITEM_INTERACT_COMPLETE + + if(mybag && user.a_intent == INTENT_HELP && !is_key(I)) + mybag.attackby__legacy__attackchain(I, user) -+ return ITEM_INTERACT_ANY_BLOCKER ++ return ITEM_INTERACT_COMPLETE + + return ..() ``` diff --git a/docs/references/images/attack_chain_flowchart.drawio b/docs/references/images/attack_chain_flowchart.drawio index 385bc4e9ba4e..7efac2046a81 100644 --- a/docs/references/images/attack_chain_flowchart.drawio +++ b/docs/references/images/attack_chain_flowchart.drawio @@ -1,11 +1,11 @@ - + - + - - + + @@ -33,7 +33,7 @@ - + @@ -76,7 +76,7 @@ - + @@ -84,7 +84,7 @@ - + @@ -187,64 +187,64 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -252,70 +252,70 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -324,13 +324,13 @@ - + - + - + @@ -339,75 +339,75 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -416,60 +416,60 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -477,16 +477,16 @@ - + - + - + - + @@ -494,7 +494,7 @@ - + @@ -502,16 +502,16 @@ - + - + - + - + @@ -519,13 +519,13 @@ - + - + - + @@ -533,7 +533,7 @@ - + @@ -541,154 +541,157 @@ - + - + - + - + - + - + - + - + - + - - + + + + + + + - + - + - - - - - + + - + - + - + + - - - - - - - - - - + - + - + + + + + + + + + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -698,13 +701,13 @@ - + - + - + @@ -714,67 +717,67 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -786,7 +789,7 @@ - + @@ -798,110 +801,109 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - - - - + + - - + + - - + + - - + + - - + + - - - - - + + + + + + + - - + + diff --git a/docs/references/images/attack_chain_flowchart.png b/docs/references/images/attack_chain_flowchart.png index 1339bd8fef05..a22149036355 100644 Binary files a/docs/references/images/attack_chain_flowchart.png and b/docs/references/images/attack_chain_flowchart.png differ