From 8b0577510d7524539328276d6c60f9c5fb5ed15e Mon Sep 17 00:00:00 2001
From: Kapu1178 <75460809+Kapu1178@users.noreply.github.com>
Date: Sun, 24 Mar 2024 20:06:51 -0400
Subject: [PATCH] Canonizes Movement (#892)

* movement info proof of concept

* oop

* oop2

* make compile

* nullspace movement

* see if this works

* lol whoops

* undef unit tests

* fuc

* fixes

* fix

* fix

* comments

* fixes
---
 code/__DEFINES/movement_info.dm               | 20 ++++++++
 code/game/atoms_movable.dm                    | 19 +++++--
 code/modules/unit_tests/_unit_tests.dm        |  1 +
 .../unit_tests/movement_order_sanity.dm       | 49 +++++++++++++++++++
 daedalus.dme                                  |  1 +
 5 files changed, 85 insertions(+), 5 deletions(-)
 create mode 100644 code/__DEFINES/movement_info.dm
 create mode 100644 code/modules/unit_tests/movement_order_sanity.dm

diff --git a/code/__DEFINES/movement_info.dm b/code/__DEFINES/movement_info.dm
new file mode 100644
index 000000000000..dc17f095c617
--- /dev/null
+++ b/code/__DEFINES/movement_info.dm
@@ -0,0 +1,20 @@
+#define ACTIVE_MOVEMENT_OLDLOC 1
+#define ACTIVE_MOVEMENT_DIRECTION 2
+#define ACTIVE_MOVEMENT_FORCED 3
+#define ACTIVE_MOVEMENT_OLDLOCS 4
+
+#define SET_ACTIVE_MOVEMENT(_old_loc, _direction, _forced, _oldlocs) \
+	active_movement = list( \
+		_old_loc, \
+		_direction, \
+		_forced, \
+		_oldlocs, \
+	)
+
+/// Finish any active movements
+#define RESOLVE_ACTIVE_MOVEMENT \
+	if(active_movement) { \
+		var/__move_args = active_movement; \
+		active_movement = null; \
+		Moved(arglist(__move_args)); \
+	}
diff --git a/code/game/atoms_movable.dm b/code/game/atoms_movable.dm
index 85556d8a2623..ab2c9fb5b33d 100644
--- a/code/game/atoms_movable.dm
+++ b/code/game/atoms_movable.dm
@@ -5,6 +5,8 @@
 
 	/// The last direction we moved in.
 	var/tmp/last_move = null
+	var/tmp/list/active_movement
+
 	///Are we moving with inertia? Mostly used as an optimization
 	var/tmp/inertia_moving = FALSE
 	///The last time we pushed off something
@@ -310,6 +312,8 @@
 	if(QDELING(src))
 		CRASH("Illegal abstract_move() on [type]!")
 
+	RESOLVE_ACTIVE_MOVEMENT
+
 	var/atom/old_loc = loc
 	var/direction = get_dir(old_loc, new_loc)
 	loc = new_loc
@@ -325,6 +329,9 @@
 	if(!newloc || newloc == loc)
 		return
 
+	// A mid-movement... movement... occured, resolve that first.
+	RESOLVE_ACTIVE_MOVEMENT
+
 	if(!direction)
 		direction = get_dir(src, newloc)
 
@@ -369,6 +376,7 @@
 	var/area/oldarea = get_area(oldloc)
 	var/area/newarea = get_area(newloc)
 
+	SET_ACTIVE_MOVEMENT(oldloc, direction, FALSE, old_locs)
 	loc = newloc
 
 	. = TRUE
@@ -386,13 +394,11 @@
 			entered_loc.Entered(src, oldloc, old_locs)
 	else
 		newloc.Entered(src, oldloc, old_locs)
+
 	if(oldarea != newarea)
 		newarea.Entered(src, oldarea)
 
-	if(loc != newloc) // Something moved us out of where we just moved to, Abort!!!
-		return
-
-	Moved(oldloc, direction, FALSE, old_locs)
+	RESOLVE_ACTIVE_MOVEMENT
 
 ////////////////////////////////////////
 
@@ -819,9 +825,12 @@
 
 /atom/movable/proc/doMove(atom/destination)
 	. = FALSE
+	RESOLVE_ACTIVE_MOVEMENT
+
 	var/atom/oldloc = loc
 	var/is_multi_tile = bound_width > world.icon_size || bound_height > world.icon_size
 
+	SET_ACTIVE_MOVEMENT(oldloc, NONE, TRUE, null)
 	if(destination)
 		var/same_loc = oldloc == destination
 		var/area/old_area = get_area(oldloc)
@@ -879,7 +888,7 @@
 			if(old_area)
 				old_area.Exited(src, NONE)
 
-	Moved(oldloc, NONE, TRUE)
+	RESOLVE_ACTIVE_MOVEMENT
 
 /**
  * Called when a movable changes z-levels.
diff --git a/code/modules/unit_tests/_unit_tests.dm b/code/modules/unit_tests/_unit_tests.dm
index 33176ef0d66e..00ebd84104b3 100644
--- a/code/modules/unit_tests/_unit_tests.dm
+++ b/code/modules/unit_tests/_unit_tests.dm
@@ -124,6 +124,7 @@
 #include "mob_spawn.dm"
 #include "modsuit.dm"
 #include "modular_map_loader.dm"
+#include "movement_order_sanity.dm"
 #include "novaflower_burn.dm"
 #include "objectives.dm"
 #include "outfit_sanity.dm"
diff --git a/code/modules/unit_tests/movement_order_sanity.dm b/code/modules/unit_tests/movement_order_sanity.dm
new file mode 100644
index 000000000000..a9f677b24980
--- /dev/null
+++ b/code/modules/unit_tests/movement_order_sanity.dm
@@ -0,0 +1,49 @@
+/datum/unit_test/movement_order_sanity/Run()
+	var/obj/movement_tester/test_obj = allocate(__IMPLIED_TYPE__, run_loc_floor_bottom_left)
+	var/list/movement_cache = test_obj.movement_order
+
+	var/obj/movement_interceptor/interceptor = allocate(__IMPLIED_TYPE__)
+	interceptor.forceMove(locate(run_loc_floor_bottom_left.x + 1, run_loc_floor_bottom_left.y, run_loc_floor_bottom_left.z))
+
+	var/did_move = step(test_obj, EAST)
+
+	TEST_ASSERT(did_move, "Object did not move at all.")
+	TEST_ASSERT(QDELETED(test_obj), "Object was not qdeleted.")
+	TEST_ASSERT(length(movement_cache) == 4, "Movement order length was not the expected value of 4, got: [length(movement_cache)].\nMovement Log\n[jointext(movement_cache, "\n")]")
+
+	// Due to when the logging takes place, it will always be Move Move > Moved Moved instead of the reality of
+	// Move > Moved > Move > Moved
+	TEST_ASSERT(findtext(movement_cache[1], "Moving from"),"Movement step 1 was not a Move attempt.\nMovement Log\n[jointext(movement_cache, "\n")]")
+	TEST_ASSERT(findtext(movement_cache[2], "Moving from"),"Movement step 2 was not a Move attempt.\nMovement Log\n[jointext(movement_cache, "\n")]")
+	TEST_ASSERT(findtext(movement_cache[3], "Moved from"),"Movement step 3 was not a Moved() call.\nMovement Log\n[jointext(movement_cache, "\n")]")
+	TEST_ASSERT(findtext(movement_cache[4], "Moved from"),"Movement step 4 was not a Moved() call.\nMovement Log\n[jointext(movement_cache, "\n")]")
+
+/obj/movement_tester
+	name = "movement debugger"
+	var/list/movement_order = list()
+
+/obj/movement_tester/Move(atom/newloc, direct, glide_size_override, z_movement_flags)
+	movement_order += "Moving from ([loc.x], [loc.y]) to [newloc ? "([newloc.x], [newloc.y])" : "NULL"]"
+	return ..()
+
+/obj/movement_tester/doMove(atom/destination)
+	movement_order += "Abstractly Moving from ([loc.x], [loc.y]) to [destination ? "([destination.x], [destination.y])" : "NULL"]"
+	return ..()
+
+/obj/movement_tester/Moved(atom/old_loc, movement_dir, forced, list/old_locs, momentum_change)
+	movement_order += "Moved from ([old_loc.x], [old_loc.y]) to [loc ? "([loc.x], [loc.y])" : "NULL"]"
+	return ..()
+
+/obj/movement_interceptor
+	name = "movement interceptor"
+
+/obj/movement_interceptor/Initialize(mapload)
+	. = ..()
+	AddElement(/datum/element/connect_loc, list(COMSIG_ATOM_ENTERED = PROC_REF(on_crossed)))
+
+/obj/movement_interceptor/proc/on_crossed(datum/source, atom/movable/arrived)
+	SIGNAL_HANDLER
+	if(src == arrived)
+		return
+
+	qdel(arrived)
diff --git a/daedalus.dme b/daedalus.dme
index 20a1ed09c8ea..2050c8304ac3 100644
--- a/daedalus.dme
+++ b/daedalus.dme
@@ -143,6 +143,7 @@
 #include "code\__DEFINES\monkeys.dm"
 #include "code\__DEFINES\move_force.dm"
 #include "code\__DEFINES\movement.dm"
+#include "code\__DEFINES\movement_info.dm"
 #include "code\__DEFINES\movespeed_modification.dm"
 #include "code\__DEFINES\mutant_colors.dm"
 #include "code\__DEFINES\nitrile.dm"