Attack chain, initial setup. (pull *immediately* for *any* TM issues) (…

…ParadiseSS13#26834)

* refactor: Attack chain, initial setup.

* migrate curtain to make dreamchecker happy

* update thurible

* don't call attacked_by separately for legacy attack chain

* remove duplicate proc

* condense similar code, put allowances for legacy code in new procs

* update docs, include diagram source

* add comment on how to update diagram

* fix admonition

* mindflayer updates

* remove commented out code

* clarify all steps

* after_attack should be overridable

* whoops

* retrofit recent changes

* duh, can't restrict this yet because of tool_acts

* i hate ore bags with the fire of a thousand suns

* return correct value for object attack logic

* Various cleanups.

We don't want to attempt to pull stuff out of `/obj/item/attackby`,
because those pieces are part of the related objects' migrations, not
`/obj/item` itself. Attempting to do this causes knockon effects where
things expected to call e.g. `/obj/item/storage/attackby` in the call
chain were not ferried over to the new item interaction code, because
the related objects hadn't actually been migrated over yet.

I've used refactoring /obj/vehicle as the example for migrating
`attackby` methods instead.

* simplify some argument names

* fuck it

* make it do the thing

* Rename CI module call

* Prove that CI works

* improve test output

* aaand fix it again

* fix curtain tool interactions

* fix compile error

* fix compile error

* Better docs, introduce migration plan tool.

.github/workflows/ci.yml

@@ -54,6 +54,7 @@ jobs:
           python tools/ci/no_duplicate_definitions.py
           python -m tools.ci.check_icon_conflicts
           python -m tools.ci.check_icon_dupenames
+          python -m tools.ci.check_legacy_attack_chain
           python -m tools.maplint.source --github
 
       - name: Run DreamChecker
@@ -95,7 +96,14 @@ jobs:
     strategy:
       fail-fast: false # Let all map tests run to completion
       matrix:
-        maptype: ['/datum/map/boxstation', '/datum/map/deltastation', '/datum/map/metastation', '/datum/map/cerestation', '/datum/map/emeraldstation']
+        maptype:
+          [
+            '/datum/map/boxstation',
+            '/datum/map/deltastation',
+            '/datum/map/metastation',
+            '/datum/map/cerestation',
+            '/datum/map/emeraldstation',
+          ]
         byondtype: ['STABLE']
     services:
       mariadb:

.github/workflows/devdocs.yml

@@ -15,7 +15,7 @@ jobs:
 
       - name: Build docs
         run: |
-          python -m pip install mkdocs==1.6.0 mkdocs-material[imaging]==9.5.31 mkdocs-github-admonitions-plugin==0.0.2
+          python -m pip install mkdocs==1.6.0 mkdocs-material[imaging]==9.5.31 mkdocs-github-admonitions-plugin==0.0.2 mkdocs-glightbox==0.4.0
           python -m mkdocs build
 
       - name: Deploy docs

code/__DEFINES/dcs/atom_signals.dm

@@ -8,10 +8,6 @@
 
 // from SSatoms InitAtom - Only if the  atom was not deleted or failed initialization
 #define COMSIG_ATOM_AFTER_SUCCESSFUL_INITIALIZE "atom_init_success"
-///from base of atom/attackby(): (/obj/item, /mob/living, params)
-#define COMSIG_PARENT_ATTACKBY "atom_attackby"
-///Return this in response if you don't want afterattack to be called
-	#define COMPONENT_NO_AFTERATTACK (1<<0)
 ///from base of atom/attack_hulk(): (/mob/living/carbon/human)
 #define COMSIG_ATOM_HULK_ATTACK "hulk_attack"
 ///from base of atom/examine(): (examining_user, examine_list)
@@ -99,13 +95,6 @@
 	#define ATOM_PREHIT_SUCCESS (1<<0)
 	#define ATOM_PREHIT_FAILURE (1<<1)
 
-// Attack signals. These should share the returned flags, to standardize the attack chain.
-// The chain currently works like:
-// tool_act -> pre_attack -> target.attackby (item.attack) -> afterattack
-// You can use these signal responses to cancel the attack chain at a certain point from most attack signal types.
-	/// This response cancels the attack chain entirely. If sent early, it might cause some later effects to be skipped.
-	#define COMPONENT_CANCEL_ATTACK_CHAIN (1<<0)
-
 /// Called from atom/Initialize() of target: (atom/target)
 #define COMSIG_ATOM_INITIALIZED_ON "atom_initialized_on"
 

code/__DEFINES/dcs/attack_chain_signals.dm

@@ -0,0 +1,67 @@
+// 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)
+
+/// Sent when this atom is clicked on by a mob with an item.
+///
+/// [/atom/proc/item_interaction] -> mob/living/user, obj/item/tool, list/modifiers
+#define COMSIG_INTERACT_TARGET		"interact_target"
+
+/// Sent to a mob clicking on an atom with an item.
+///
+/// [/atom/proc/item_interaction] -> atom/target, obj/item/tool, list/modifiers
+#define COMSIG_INTERACT_USER		"interact_user"
+
+/// Sent to an item clicking on an atom.
+///
+/// [/atom/proc/item_interaction] -> mob/living/user, atom/target, list/modifiers
+#define COMSIG_INTERACTING			"interacting"
+
+#define COMSIG_INTERACT_RANGED		"interact_ranged"		//! [/atom/proc/ranged_item_interaction]
+#define COMSIG_INTERACTING_RANGED	"interacting_ranged"	//! [/atom/proc/ranged_item_interaction]
+
+#define COMSIG_ACTIVATE_SELF		"activate_self"			//! [/obj/item/proc/activate_self] -> mob/user
+
+// Attack signals. These should share the returned flags, to standardize the attack chain.
+// The chain currently works like:
+// tool_act -> pre_attack -> target.attackby (item.attack) -> afterattack
+// You can use these signal responses to cancel the attack chain at a certain point from most attack signal types.
+
+// MARK: Attack Chain
+
+// Signal interceptors for short-circuiting parts of the attack chain.
+
+#define COMPONENT_CANCEL_ATTACK_CHAIN	(1<<0)	//! Cancel the attack chain entirely.
+#define COMPONENT_SKIP_ATTACK			(1<<1)	//! Skip this attack step, continuing for the next one to happen.
+#define COMPONENT_SKIP_AFTERATTACK		(1<<2)	//! Skip after_attacks (while allowing for e.g. attack_by).
+
+#define COMSIG_PRE_ATTACK			"pre_attack"			//! [/obj/item/proc/pre_attack] -> atom/target, mob/user, params
+
+#define COMSIG_ATTACK				"attack"				//! [/obj/item/proc/attack] -> mob/living/target, mob/living/user
+#define COMSIG_ATTACK_OBJ			"attack_obj"			//! [/obj/item/proc/attack_obj] -> obj/attacked, mob/user
+#define COMSIG_ATTACK_OBJ_LIVING	"attack_obj_living"		//! [/obj/item/proc/attack_obj] -> obj/attacked
+#define COMSIG_ATTACK_BY			"attack_by"				//! [/atom/proc/attackby] -> obj/item/weapon, mob/living/user, params
+
+#define COMSIG_AFTER_ATTACK			"item_after_attack"		//! [/obj/item/proc/afterattack] -> atom/target, mob/user, params
+#define COMSIG_AFTER_ATTACKED_BY	"after_attacked_by"		//! [/obj/item/proc/afterattack] -> obj/item/weapon, mob/user, proximity_flag, params
+
+// Return values for directing the control of the attack chain. Distinct from
+// signal interceptors because they're not meant to be combined, and to mesh better with
+// historical use of return values in attack chain procs.
+
+#define CONTINUE_ATTACK		0	//! Continue the attack chain, i.e. allow other signals to respond.
+#define FINISH_ATTACK		1	//! Do not continue the attack chain.
+
+// Legacy-only, do not use in new code
+
+#define COMSIG_TOOL_ATTACK "tool_attack"	//! [/obj/item/proc/tool_attack_chain] -> atom/tool, mob/user
+#define COMPONENT_NO_ATTACK (1<<0)
+#define COMPONENT_NO_INTERACT (1<<0)
+#define COMPONENT_NO_ATTACK_OBJ (1<<0)
+#define COMPONENT_CANCEL_TOOLACT (1<<0)

code/__DEFINES/dcs/item_signals.dm

@@ -6,21 +6,7 @@
 
 // /obj/item
 
-///from base of obj/item/attack(): (/mob/living/target, /mob/living/user)
-#define COMSIG_ITEM_ATTACK "item_attack"
-///from base of obj/item/attack_self(): (/mob)
-#define COMSIG_ITEM_ATTACK_SELF "item_attack_self"
-	#define COMPONENT_NO_INTERACT (1<<0)
-///from base of obj/item/attack_obj(): (/obj, /mob)
-#define COMSIG_ITEM_ATTACK_OBJ "item_attack_obj"
-	#define COMPONENT_NO_ATTACK_OBJ (1<<0)
-///from base of obj/item/pre_attack(): (atom/target, mob/user, params)
-#define COMSIG_ITEM_PRE_ATTACK "item_pre_attack"
-	#define COMPONENT_NO_ATTACK (1<<0)
-///from base of obj/item/pre_attack(): (atom/target, mob/user, params)
-#define COMSIG_ITEM_BEING_ATTACKED "item_being_attacked"
-///from base of obj/item/afterattack(): (atom/target, mob/user, params)
-#define COMSIG_ITEM_AFTERATTACK "item_afterattack"
+
 ///called on [/obj/item] before unequip from base of [/mob/proc/unEquip]: (force, atom/newloc, no_move, invdrop, silent)
 #define COMSIG_ITEM_PRE_UNEQUIP "item_pre_unequip"
 	///only the pre unequip can be cancelled
@@ -41,9 +27,6 @@
 	#define COMPONENT_BLOCK_SHARPEN_BLOCKED (1<<1)
 	#define COMPONENT_BLOCK_SHARPEN_ALREADY (1<<2)
 	#define COMPONENT_BLOCK_SHARPEN_MAXED (1<<3)
-///from base of [/obj/item/proc/tool_attack_chain]: (atom/tool, mob/user)
-#define COMSIG_TOOL_ATTACK "tool_attack"
-	#define COMPONENT_CANCEL_TOOLACT (1<<0)
 /// Called by [/obj/item/assembly/proc/pulse]
 #define COMSIG_ASSEMBLY_PULSED "item_assembly_pulsed"
 ///from [/mob/living/carbon/human/proc/Move]: ()
@@ -129,7 +112,7 @@
 
 // other items
 
-/// from base of /obj/item/slimepotion/speed/afterattack(): (obj/target, /obj/src, mob/user)
+/// from base of /obj/item/slimepotion/speed/afterattack__legacy__attackchain(): (obj/target, /obj/src, mob/user)
 #define COMSIG_SPEED_POTION_APPLIED "speed_potion"
 	#define SPEED_POTION_STOP (1<<0)
 

code/__DEFINES/dcs/mob_signals.dm

@@ -36,7 +36,7 @@
 	#define COMPONENT_ITEM_NO_ATTACK (1<<0)
 ///from base of /mob/living/proc/apply_damage(): (damage, damagetype, def_zone)
 #define COMSIG_MOB_APPLY_DAMAGE	"mob_apply_damage"
-///from base of obj/item/afterattack(): (atom/target, mob/user, proximity_flag, click_parameters)
+///from base of obj/item/afterattack__legacy__attackchain(): (atom/target, mob/user, proximity_flag, click_parameters)
 #define COMSIG_MOB_ITEM_AFTERATTACK "mob_item_afterattack"
 ///from base of mob/RangedAttack(): (atom/A, params)
 #define COMSIG_MOB_ATTACK_RANGED "mob_attack_ranged"

code/_onclick/click.dm

@@ -44,7 +44,7 @@
 	The most common are:
 	* mob/UnarmedAttack(atom,adjacent) - used here only when adjacent, with no item in hand; in the case of humans, checks gloves
 	* atom/attackby(item,user) - used only when adjacent
-	* item/afterattack(atom,user,adjacent,params) - used both ranged and adjacent
+	* item/afterattack__legacy__attackchain(atom,user,adjacent,params) - used both ranged and adjacent
 	* mob/RangedAttack(atom,params) - used only ranged, only used for tk and laser eyes but could be changed
 */
 /mob/proc/ClickOn(atom/A, params)
@@ -123,7 +123,10 @@
 	var/obj/item/W = get_active_hand()
 
 	if(W == A)
-		W.attack_self(src)
+		if(W.new_attack_chain)
+			W.activate_self(src)
+		else
+			W.attack_self__legacy__attackchain(src)
 		if(hand)
 			update_inv_l_hand()
 		else
@@ -150,10 +153,12 @@
 			if(ismob(A))
 				changeNext_move(CLICK_CD_MELEE)
 			UnarmedAttack(A, 1)
-
 	else
 		if(W)
-			W.afterattack(A, src, 0, params) // 0: not Adjacent
+			if(W.new_attack_chain)
+				A.base_ranged_item_interaction(src, W, params)
+			else
+				W.afterattack__legacy__attackchain(A, src, 0, params) // 0: not Adjacent
 		else
 			RangedAttack(A, params)
 

code/_onclick/click_override.dm

@@ -25,7 +25,7 @@
 	icon_state = "book"
 	var/datum/middle_click_override/clickBehavior = new /datum/middle_click_override/badmin_clicker
 
-/obj/item/badmin_book/attack_self(mob/living/user as mob)
+/obj/item/badmin_book/attack_self__legacy__attackchain(mob/living/user as mob)
 	if(user.middleClickOverride)
 		to_chat(user, "<span class='warning'>You try to draw power from [src], but you cannot hold the power at this time!</span>")
 		return

code/_onclick/cyborg.dm

@@ -83,7 +83,11 @@
 		return
 
 	if(W == A)
-		W.attack_self(src)
+		if(W.new_attack_chain)
+			W.activate_self(src)
+		else
+			W.attack_self__legacy__attackchain(src)
+
 		return
 
 	// cyborgs are prohibited from using storage items so we can I think safely remove (A.loc in contents)
@@ -98,7 +102,7 @@
 	if(can_reach(A, W))
 		W.melee_attack_chain(src, A, params)
 		return
-	W.afterattack(A, src, 0, params)
+	W.afterattack__legacy__attackchain(A, src, 0, params)
 	return
 
 /mob/living/silicon/robot/MiddleShiftControlClickOn(atom/A)

code/_onclick/hud/screen_objects.dm

@@ -76,7 +76,7 @@
 /atom/movable/screen/grab/attack_hand()
 	return
 
-/atom/movable/screen/grab/attackby()
+/atom/movable/screen/grab/attackby__legacy__attackchain()
 	return
 /atom/movable/screen/act_intent
 	name = "intent"
@@ -170,7 +170,7 @@
 	if(master)
 		var/obj/item/I = usr.get_active_hand()
 		if(I)
-			master.attackby(I, usr, params)
+			master.attackby__legacy__attackchain(I, usr, params)
 	return TRUE
 
 /atom/movable/screen/storage/proc/is_item_accessible(obj/item/I, mob/user)
@@ -226,7 +226,7 @@
 			S.orient2hud(user)
 			S.show_to(user)
 	else // If it's not in the storage, try putting it inside
-		S.attackby(I, user)
+		S.attackby__legacy__attackchain(I, user)
 	return TRUE
 
 /atom/movable/screen/zone_sel