[MIRROR] Timer subsystem update (#9320)

Co-authored-by: Selis <12716288+ItsSelis@users.noreply.github.com>
Co-authored-by: Kashargul <144968721+Kashargul@users.noreply.github.com>

code/__defines/_helpers.dm

@@ -14,4 +14,3 @@
 #define ICON_SIZE_X 32
 /// The Y/Height dimension of ICON_SIZE. This will more than likely be the smaller axis.
 #define ICON_SIZE_Y 32
-yy

code/__defines/_lists.dm

@@ -46,46 +46,4 @@
 
 #define reverseList(L) reverseRange(L.Copy())
 
-// CHOMPEdit Start
-/// Passed into BINARY_INSERT to compare keys
-#define COMPARE_KEY __BIN_LIST[__BIN_MID]
-/// Passed into BINARY_INSERT to compare values
-#define COMPARE_VALUE __BIN_LIST[__BIN_LIST[__BIN_MID]]
-
-/****
-	* Binary search sorted insert
-	* INPUT: Object to be inserted
-	* LIST: List to insert object into
-	* TYPECONT: The typepath of the contents of the list
-	* COMPARE: The object to compare against, usualy the same as INPUT
-	* COMPARISON: The variable on the objects to compare
-	* COMPTYPE: How should the values be compared? Either COMPARE_KEY or COMPARE_VALUE.
-	*/
-#define BINARY_INSERT(INPUT, LIST, TYPECONT, COMPARE, COMPARISON, COMPTYPE) \
-	do {\
-		var/list/__BIN_LIST = LIST;\
-		var/__BIN_CTTL = length(__BIN_LIST);\
-		if(!__BIN_CTTL) {\
-			__BIN_LIST += INPUT;\
-		} else {\
-			var/__BIN_LEFT = 1;\
-			var/__BIN_RIGHT = __BIN_CTTL;\
-			var/__BIN_MID = (__BIN_LEFT + __BIN_RIGHT) >> 1;\
-			var ##TYPECONT/__BIN_ITEM;\
-			while(__BIN_LEFT < __BIN_RIGHT) {\
-				__BIN_ITEM = COMPTYPE;\
-				if(__BIN_ITEM.##COMPARISON <= COMPARE.##COMPARISON) {\
-					__BIN_LEFT = __BIN_MID + 1;\
-				} else {\
-					__BIN_RIGHT = __BIN_MID;\
-				};\
-				__BIN_MID = (__BIN_LEFT + __BIN_RIGHT) >> 1;\
-			};\
-			__BIN_ITEM = COMPTYPE;\
-			__BIN_MID = __BIN_ITEM.##COMPARISON > COMPARE.##COMPARISON ? __BIN_MID : __BIN_MID + 1;\
-			__BIN_LIST.Insert(__BIN_MID, INPUT);\
-		};\
-	} while(FALSE)
-// CHOMPEdit End
-
 #define islist(L) istype(L, /list)

code/__defines/misc.dm

@@ -31,7 +31,6 @@
 #define MAX_CLIENT_FPS	200
 
 // Some arbitrary defines to be used by self-pruning global lists. (see master_controller)
-#define PROCESS_KILL 26 // Used to trigger removal from a processing list.
 #define MAX_GEAR_COST 15 // Used in chargen for accessory loadout limit.
 
 // For secHUDs and medHUDs and variants. The number is the location of the image on the list hud_list of humans.

code/__defines/subsystems.dm

@@ -19,7 +19,7 @@
  * Timing should be based on how timing progresses on clients, not the server.
  *
  * Tracking this is more expensive,
- * should only be used in conjuction with things that have to progress client side, such as
+ * should only be used in conjunction with things that have to progress client side, such as
  * animate() or sound()
  */
 #define TIMER_CLIENT_TIME (1<<2)
@@ -43,15 +43,36 @@
 ///Empty ID define
 #define TIMER_ID_NULL -1
 
-#define INITIALIZATION_INSSATOMS 0	//New should not call Initialize
+/// Used to trigger object removal from a processing list
-#define INITIALIZATION_INNEW_MAPLOAD 1	//New should call Initialize(TRUE)
+#define PROCESS_KILL 26
-#define INITIALIZATION_INNEW_REGULAR 2	//New should call Initialize(FALSE)
 
-#define INITIALIZE_HINT_NORMAL   0  //Nothing happens
+
-#define INITIALIZE_HINT_LATELOAD 1  //Call LateInitialize
+//! ## Initialization subsystem
-#define INITIALIZE_HINT_QDEL     2  //Call qdel on the atom
+
-//CHOMPEdit Begin
+///New should not call Initialize
-//type and all subtypes should always call Initialize in New()
+#define INITIALIZATION_INSSATOMS 0
+///New should call Initialize(TRUE)
+#define INITIALIZATION_INNEW_MAPLOAD 1
+///New should call Initialize(FALSE)
+#define INITIALIZATION_INNEW_REGULAR 2
+
+//! ### Initialization hints
+
+///Nothing happens
+#define INITIALIZE_HINT_NORMAL 0
+/**
+ * call LateInitialize at the end of all atom Initialization
+ *
+ * The item will be added to the late_loaders list, this is iterated over after
+ * initialization of subsystems is complete and calls LateInitalize on the atom
+ * see [this file for the LateIntialize proc](atom.html#proc/LateInitialize)
+ */
+#define INITIALIZE_HINT_LATELOAD 1
+
+///Call qdel on the atom after initialization
+#define INITIALIZE_HINT_QDEL 2
+
+///type and all subtypes should always immediately call Initialize in New()
 #define INITIALIZE_IMMEDIATE(X) ##X/New(loc, ...){\
 	..();\
 	if(!(flags & ATOM_INITIALIZED)) {\
@@ -75,24 +96,23 @@ var/global/list/runlevel_flags = list(RUNLEVEL_LOBBY, RUNLEVEL_SETUP, RUNLEVEL_G
 
 //! ### SS initialization hints
 /**
- * Negative values incidate a failure or warning of some kind, positive are good.
+ * Negative values indicate a failure or warning of some kind, positive are good.
- * 0 and 1 are unused so that TRUE and FALSE are guarenteed to be invalid values.
+ * 0 and 1 are unused so that TRUE and FALSE are guaranteed to be invalid values.
  */
 
 /// Subsystem failed to initialize entirely. Print a warning, log, and disable firing.
 #define SS_INIT_FAILURE -2
 
-/// The default return value which must be overriden. Will succeed with a warning.
+/// The default return value which must be overridden. Will succeed with a warning.
 #define SS_INIT_NONE -1
 
-/// Subsystem initialized sucessfully.
+/// Subsystem initialized successfully.
 #define SS_INIT_SUCCESS 2
 
-/// Successful, but don't print anything. Useful if subsystem was disabled.
+/// If your system doesn't need to be initialized (by being disabled or something)
 #define SS_INIT_NO_NEED 3
 
 //! ### SS initialization load orders
-
 // Subsystem init_order, from highest priority to lowest priority
 // Subsystems shutdown in the reverse of the order they initialize in
 // The numbers just define the ordering, they are meaningless otherwise.
@@ -139,8 +159,6 @@ var/global/list/runlevel_flags = list(RUNLEVEL_LOBBY, RUNLEVEL_SETUP, RUNLEVEL_G
 #define INIT_ORDER_STATPANELS		-98
 #define INIT_ORDER_CHAT				-100 //Should be last to ensure chat remains smooth during init.
 
-
-
 // Subsystem fire priority, from lowest to highest priority
 // If the subsystem isn't listed here it's either DEFAULT or PROCESS (if it's a processing subsystem child)
 #define FIRE_PRIORITY_PLAYERTIPS	5

code/_helpers/_lists.dm

@@ -921,6 +921,46 @@ var/global/list/json_cache = list()
 ///Remove an untyped item to a list, taking care to handle list items by wrapping them in a list to remove the footgun
 #define UNTYPED_LIST_REMOVE(list, item) (list -= LIST_VALUE_WRAP_LISTS(item))
 
+/// Passed into BINARY_INSERT to compare keys
+#define COMPARE_KEY __BIN_LIST[__BIN_MID]
+/// Passed into BINARY_INSERT to compare values
+#define COMPARE_VALUE __BIN_LIST[__BIN_LIST[__BIN_MID]]
+
+/****
+	* Binary search sorted insert
+	* INPUT: Object to be inserted
+	* LIST: List to insert object into
+	* TYPECONT: The typepath of the contents of the list
+	* COMPARE: The object to compare against, usualy the same as INPUT
+	* COMPARISON: The variable on the objects to compare
+	* COMPTYPE: How should the values be compared? Either COMPARE_KEY or COMPARE_VALUE.
+	*/
+#define BINARY_INSERT(INPUT, LIST, TYPECONT, COMPARE, COMPARISON, COMPTYPE) \
+	do {\
+		var/list/__BIN_LIST = LIST;\
+		var/__BIN_CTTL = length(__BIN_LIST);\
+		if(!__BIN_CTTL) {\
+			__BIN_LIST += INPUT;\
+		} else {\
+			var/__BIN_LEFT = 1;\
+			var/__BIN_RIGHT = __BIN_CTTL;\
+			var/__BIN_MID = (__BIN_LEFT + __BIN_RIGHT) >> 1;\
+			var ##TYPECONT/__BIN_ITEM;\
+			while(__BIN_LEFT < __BIN_RIGHT) {\
+				__BIN_ITEM = COMPTYPE;\
+				if(__BIN_ITEM.##COMPARISON <= COMPARE.##COMPARISON) {\
+					__BIN_LEFT = __BIN_MID + 1;\
+				} else {\
+					__BIN_RIGHT = __BIN_MID;\
+				};\
+				__BIN_MID = (__BIN_LEFT + __BIN_RIGHT) >> 1;\
+			};\
+			__BIN_ITEM = COMPTYPE;\
+			__BIN_MID = __BIN_ITEM.##COMPARISON > COMPARE.##COMPARISON ? __BIN_MID : __BIN_MID + 1;\
+			__BIN_LIST.Insert(__BIN_MID, INPUT);\
+		};\
+	} while(FALSE)
+
 //CHOMPAdd start
 /proc/pick_weight(list/list_to_pick)
 	var/total = 0

code/controllers/configuration/entries/general.dm

@@ -60,6 +60,9 @@
 /// logs graffiti
 /datum/config_entry/flag/log_graffiti
 
+/// logs all timers in buckets on automatic bucket reset (Useful for timer debugging)
+/datum/config_entry/flag/log_timers_on_bucket_reset
+
 // FIXME: Unused
 ///datum/config_entry/string/nudge_script_path // where the nudge.py script is located
 //	default = "nudge.py"

code/controllers/subsystems/timer.dm

@@ -1,4 +1,5 @@
-#define BUCKET_LEN (round(10*(60/world.tick_lag), 1)) //how many ticks should we keep in the bucket. (1 minutes worth)
+/// Controls how many buckets should be kept, each representing a tick. (1 minutes worth)
+#define BUCKET_LEN (world.fps*1*60)
 /// Helper for getting the correct bucket for a given timer
 #define BUCKET_POS(timer) (((ROUND_UP((timer.timeToRun - timer.timer_subsystem.head_offset) / world.tick_lag)+1) % BUCKET_LEN) || BUCKET_LEN)
 /// Gets the maximum time at which timers will be invoked from buckets, used for deferring to secondary queue
@@ -55,7 +56,7 @@ SUBSYSTEM_DEF(timer)
 	bucket_resolution = world.tick_lag
 
 /datum/controller/subsystem/timer/stat_entry(msg)
-	msg = "B:[bucket_count] P:[length(second_queue)] H:[length(hashes)] C:[length(clienttime_timers)] S:[length(timer_id_dict)] RST:[bucket_reset_count]" // CHOMPEdit
+	msg = "B:[bucket_count] P:[length(second_queue)] H:[length(hashes)] C:[length(clienttime_timers)] S:[length(timer_id_dict)] RST:[bucket_reset_count]"
 	return ..()
 
 /datum/controller/subsystem/timer/proc/dump_timer_buckets(full = TRUE)
@@ -101,25 +102,7 @@ SUBSYSTEM_DEF(timer)
 		WARNING(msg)
 		if(bucket_auto_reset)
 			bucket_resolution = 0
-
+		dump_timer_buckets(CONFIG_GET(flag/log_timers_on_bucket_reset))
-		log_world("Timer bucket reset. world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
-		for (var/i in 1 to length(bucket_list))
-			var/datum/timedevent/bucket_head = bucket_list[i]
-			if (!bucket_head)
-				continue
-
-			log_world("Active timers at index [i]:")
-
-			var/datum/timedevent/bucket_node = bucket_head
-			var/anti_loop_check = 1000
-			do
-				log_world(get_timer_debug_string(bucket_node))
-				bucket_node = bucket_node.next
-				anti_loop_check--
-			while(bucket_node && bucket_node != bucket_head && anti_loop_check)
-		log_world("Active timers in the second_queue queue:")
-		for(var/I in second_queue)
-			log_world(get_timer_debug_string(I))
 
 	// Process client-time timers
 	if (next_clienttime_timer_index)
@@ -541,7 +524,7 @@ SUBSYSTEM_DEF(timer)
 		2 = timeToRun,
 		3 = wait,
 		4 = flags,
-		5 = callBack, /* Safe to hold this directly becasue it's never del'd */
+		5 = callBack, /* Safe to hold this directly because it's never del'd */
 		6 = "[callBack.object]",
 		7 = text_ref(callBack.object),
 		8 = getcallingtype(),
@@ -556,7 +539,7 @@ SUBSYSTEM_DEF(timer)
 		2 = timeToRun,
 		3 = wait,
 		4 = flags,
-		5 = callBack, /* Safe to hold this directly becasue it's never del'd */
+		5 = callBack, /* Safe to hold this directly because it's never del'd */
 		6 = "[callBack.object]",
 		7 = getcallingtype(),
 		8 = callBack.delegate,
@@ -662,7 +645,7 @@ SUBSYSTEM_DEF(timer)
 				hash_timer.hash = null // but keep it from accidentally deleting us
 			else
 				if (flags & TIMER_OVERRIDE)
-					hash_timer.hash = null // no need having it delete it's hash if we are going to replace it
+					hash_timer.hash = null // no need having it delete its hash if we are going to replace it
 					qdel(hash_timer)
 				else
 					if (hash_timer.flags & TIMER_STOPPABLE)

code/datums/datum.dm

@@ -22,7 +22,7 @@
 	var/list/open_tguis // CHOMPEdit: FIXME: open_uis
 
 	/// Active timers with this datum as the target
-	var/list/_active_timers // CHOMPEdit
+	var/list/_active_timers
 	/// Status traits attached to this datum. associative list of the form: list(trait name (string) = list(source1, source2, source3,...))
 	var/list/_status_traits
 
@@ -71,9 +71,22 @@
 	// Create and destroy is weird and I wanna cover my bases
 	var/harddel_deets_dumped = FALSE
 
-// Default implementation of clean-up code.
+/**
-// This should be overridden to remove all references pointing to the object being destroyed.
+ * Default implementation of clean-up code.
-// Return the appropriate QDEL_HINT; in most cases this is QDEL_HINT_QUEUE.
+ *
+ * This should be overridden to remove all references pointing to the object being destroyed, if
+ * you do override it, make sure to call the parent and return its return value by default
+ *
+ * Return an appropriate [QDEL_HINT][QDEL_HINT_QUEUE] to modify handling of your deletion;
+ * in most cases this is [QDEL_HINT_QUEUE].
+ *
+ * The base case is responsible for doing the following
+ * * Erasing timers pointing to this datum
+ * * Erasing compenents on this datum
+ * * Notifying datums listening to signals from this datum that we are going away
+ *
+ * Returns [QDEL_HINT_QUEUE]
+ */
 /datum/proc/Destroy(force=FALSE)
 	SHOULD_CALL_PARENT(TRUE)
 	// SHOULD_NOT_SLEEP(TRUE) FIXME: Causing some big issues still
@@ -81,12 +94,13 @@
 	weak_reference = null //ensure prompt GCing of weakref.
 
 	//clear timers
-	var/list/timers = _active_timers // CHOMPEdit
+	if(_active_timers)
-	_active_timers = null // CHOMPEdit
+		var/list/timers = _active_timers
-	for(var/datum/timedevent/timer as anything in timers)
+		_active_timers = null
-		if (timer.spent)
+		for(var/datum/timedevent/timer as anything in timers)
-			continue
+			if (timer.spent && !(timer.flags & TIMER_DELETE_ME))
-		qdel(timer)
+				continue
+			qdel(timer)
 
 	#ifdef REFERENCE_TRACKING
 	#ifdef REFERENCE_TRACKING_DEBUG
@@ -110,7 +124,6 @@
 	_clear_signal_refs()
 	//END: ECS SHIT
 
-	tag = null
 	SStgui.close_uis(src)
 
 	#ifdef REFERENCE_TRACKING

config/example/logging.txt

@@ -44,6 +44,9 @@ LOG_PDA
 ## log graffiti drawings
 LOG_GRAFFITI
 
+## Log all timers on timer auto reset
+# LOG_TIMERS_ON_BUCKET_RESET
+
 ## log world.log messages
 # LOG_WORLD_OUTPUT