Cleanup ITEM_INTERACT_ flags. (#27640)

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.
 ///

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)
 

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

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, "<span class='notice'>You hook [used] onto [src].</span>")
 		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,"<span class='notice'>You upgrade [src] with [used].</span>")
 		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 ..()
 

code/modules/vehicle/vehicle.dm

@@ -78,10 +78,10 @@
 			inserted_key = used
 		else
 			to_chat(user, "<span class='warning'>[used] seems to be stuck to your hand!</span>")
-		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))

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, "<span class='notice'>You hook [I] onto [src].</span>")
  		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,"<span class='notice'>You upgrade [src] with [I].</span>")
  		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 ..()
 ```