Merge branch 'master' into prepare_stuff_for_second_week

code/controllers/master/master.dm

@@ -489,7 +489,7 @@ var/CURRENT_TICKLIMIT = TICK_LIMIT_RUNNING
 			queue_node.times_fired++
 
 			if (queue_node_flags & SS_TICKER)
-				queue_node.next_fire = world.time + (world.tick_lag * (queue_node.wait + (queue_node.tick_overrun/100)))
+				queue_node.next_fire = world.time + (world.tick_lag * queue_node.wait)
 			else if (queue_node_flags & SS_POST_FIRE_TIMING)
 				queue_node.next_fire = world.time + queue_node.wait + (world.tick_lag * (queue_node.tick_overrun/100))
 			else if (queue_node_flags & SS_KEEP_TIMING)

code/controllers/master/subsystem.dm

@@ -104,7 +104,7 @@
 		else
 			if (SS_flags & SS_BACKGROUND)
 				continue
-			if (SS_flags & SS_TICKER)
+			if (SS_flags & (SS_TICKER|SS_BACKGROUND) == SS_TICKER)
 				break
 			if (queue_node_priority < SS_priority)
 				break

code/controllers/subsystems/processing/shuttle.dm

@@ -1,9 +1,8 @@
-var/datum/controller/subsystem/processing/shuttle/SSshuttle
+var/datum/controller/subsystem/shuttle/SSshuttle
 
-/datum/controller/subsystem/processing/shuttle
+/datum/controller/subsystem/shuttle
 	name = "Shuttle"
 	wait = 2 SECONDS
-	flags = 0
 	priority = SS_PRIORITY_SHUTTLE
 	init_order = SS_INIT_MISC                    //Should be initialized after all maploading is over and atoms are initialized, to ensure that landmarks have been initialized.
 
@@ -27,10 +26,10 @@ var/datum/controller/subsystem/processing/shuttle/SSshuttle
 
 	var/tmp/list/working_shuttles
 
-/datum/controller/subsystem/processing/shuttle/New()
+/datum/controller/subsystem/shuttle/New()
 	NEW_SS_GLOBAL(SSshuttle)
 
-/datum/controller/subsystem/processing/shuttle/Initialize()
+/datum/controller/subsystem/shuttle/Initialize()
 	last_landmark_registration_time = world.time
 	for(var/shuttle_type in subtypesof(/datum/shuttle)) // This accounts for most shuttles, though away maps can queue up more.
 		var/datum/shuttle/shuttle = shuttle_type
@@ -42,26 +41,26 @@ var/datum/controller/subsystem/processing/shuttle/SSshuttle
 	clear_init_queue()
 	. = ..()
 
-/datum/controller/subsystem/processing/shuttle/fire(resumed = FALSE)
+/datum/controller/subsystem/shuttle/fire(resumed = FALSE)
 	if (!resumed)
 		working_shuttles = process_shuttles.Copy()
 
 	while(working_shuttles.len)
 		var/datum/shuttle/shuttle = working_shuttles[working_shuttles.len]
 		working_shuttles.len--
-		if(shuttle.process_state && (shuttle.process(wait, times_fired, src) == PROCESS_KILL))
+		if(shuttle.process_state && (shuttle.process() == PROCESS_KILL))
 			process_shuttles -= shuttle
 
 		if(TICK_CHECK)
 			return
 
-/datum/controller/subsystem/processing/shuttle/proc/clear_init_queue()
+/datum/controller/subsystem/shuttle/proc/clear_init_queue()
 	if(block_queue)
 		return
 	initialize_shuttles()
 	initialize_sectors()
 
-/datum/controller/subsystem/processing/shuttle/proc/initialize_shuttles()
+/datum/controller/subsystem/shuttle/proc/initialize_shuttles()
 	var/list/shuttles_made = list()
 	for(var/shuttle_type in shuttles_to_initialize)
 		var/shuttle = initialize_shuttle(shuttle_type)
@@ -70,12 +69,12 @@ var/datum/controller/subsystem/processing/shuttle/SSshuttle
 	hook_up_motherships(shuttles_made)
 	shuttles_to_initialize = null
 
-/datum/controller/subsystem/processing/shuttle/proc/initialize_sectors()
+/datum/controller/subsystem/shuttle/proc/initialize_sectors()
 	for(var/sector in sectors_to_initialize)
 		initialize_sector(sector)
 	sectors_to_initialize = null
 
-/datum/controller/subsystem/processing/shuttle/proc/register_landmark(shuttle_landmark_tag, obj/effect/shuttle_landmark/shuttle_landmark)
+/datum/controller/subsystem/shuttle/proc/register_landmark(shuttle_landmark_tag, obj/effect/shuttle_landmark/shuttle_landmark)
 	if (registered_shuttle_landmarks[shuttle_landmark_tag])
 		CRASH("Attempted to register shuttle landmark with tag [shuttle_landmark_tag], but it is already registered!")
 	if (istype(shuttle_landmark))
@@ -93,12 +92,12 @@ var/datum/controller/subsystem/processing/shuttle/SSshuttle
 			else
 				landmarks_awaiting_sector += shuttle_landmark
 
-/datum/controller/subsystem/processing/shuttle/proc/get_landmark(var/shuttle_landmark_tag)
+/datum/controller/subsystem/shuttle/proc/get_landmark(var/shuttle_landmark_tag)
 	return registered_shuttle_landmarks[shuttle_landmark_tag]
 
 //Checks if the given sector's landmarks have initialized; if so, registers them with the sector, if not, marks them for assignment after they come in.
 //Also adds automatic landmarks that were waiting on their sector to spawn.
-/datum/controller/subsystem/processing/shuttle/proc/initialize_sector(obj/effect/overmap/visitable/given_sector)
+/datum/controller/subsystem/shuttle/proc/initialize_sector(obj/effect/overmap/visitable/given_sector)
 	given_sector.populate_sector_objects() // This is a late init operation that sets up the sector's map_z and does non-overmap-related init tasks.
 
 	for(var/landmark_tag in given_sector.initial_generic_waypoints)
@@ -119,7 +118,7 @@ var/datum/controller/subsystem/processing/shuttle/SSshuttle
 
 	initialized_sectors |= given_sector
 
-/datum/controller/subsystem/processing/shuttle/proc/try_add_landmark_tag(landmark_tag, obj/effect/overmap/visitable/given_sector)
+/datum/controller/subsystem/shuttle/proc/try_add_landmark_tag(landmark_tag, obj/effect/overmap/visitable/given_sector)
 	var/obj/effect/shuttle_landmark/landmark = get_landmark(landmark_tag)
 	if(!landmark)
 		return FALSE
@@ -132,14 +131,14 @@ var/datum/controller/subsystem/processing/shuttle/SSshuttle
 			given_sector.add_landmark(landmark, shuttle_name)
 			. = TRUE
 
-/datum/controller/subsystem/processing/shuttle/proc/initialize_shuttle(var/shuttle_type)
+/datum/controller/subsystem/shuttle/proc/initialize_shuttle(var/shuttle_type)
 	var/datum/shuttle/shuttle = shuttle_type
 	if(initial(shuttle.category) != shuttle_type)
 		shuttle = new shuttle()
 		shuttle_areas |= shuttle.shuttle_area
 		return shuttle
 
-/datum/controller/subsystem/processing/shuttle/proc/hook_up_motherships(shuttles_list)
+/datum/controller/subsystem/shuttle/proc/hook_up_motherships(shuttles_list)
 	for(var/datum/shuttle/S in shuttles_list)
 		if(S.mothershuttle && !S.motherdock)
 			var/datum/shuttle/mothership = shuttles[S.mothershuttle]
@@ -149,7 +148,7 @@ var/datum/controller/subsystem/processing/shuttle/SSshuttle
 			else
 				error("Shuttle [S] was unable to find mothership [mothership]!")
 
-/datum/controller/subsystem/processing/shuttle/proc/toggle_overmap(new_setting)
+/datum/controller/subsystem/shuttle/proc/toggle_overmap(new_setting)
 	if(overmap_halted == new_setting)
 		return
 	overmap_halted = !overmap_halted
@@ -157,5 +156,5 @@ var/datum/controller/subsystem/processing/shuttle/SSshuttle
 		var/obj/effect/overmap/visitable/ship/ship_effect = ship
 		overmap_halted ? ship_effect.halt() : ship_effect.unhalt()
 
-/datum/controller/subsystem/processing/shuttle/stat_entry()
+/datum/controller/subsystem/shuttle/stat_entry()
 	..("Shuttles:[shuttles.len], Ships:[ships.len], L:[registered_shuttle_landmarks.len][overmap_halted ? ", HALT" : ""]")

code/controllers/subsystems/timer.dm

@@ -1,8 +1,21 @@
-#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((timer.timeToRun - SStimer.head_offset) / world.tick_lag)+1) % BUCKET_LEN)||BUCKET_LEN)
-#define TIMER_ID_MAX (2**24) //max float with integer precision
+/// Gets the maximum time at which timers will be invoked from buckets, used for deferring to secondary queue
 #define TIMER_MAX (world.time + TICKS2DS(min(BUCKET_LEN-(SStimer.practical_offset-DS2TICKS(world.time - SStimer.head_offset))-1, BUCKET_LEN-1)))
+/// Max float with integer precision
+#define TIMER_ID_MAX (2**24)
 
+/**
+  * # Timer Subsystem
+  *
+  * Handles creation, callbacks, and destruction of timed events.
+  *
+  * It is important to understand the buckets used in the timer subsystem are just a series of circular doubly-linked
+  * lists. The object at a given index in bucket_list is a /datum/timedevent, the head of a circular list, which has prev
+  * and next references for the respective elements in that bucket's circular list.
+  */
 var/datum/controller/subsystem/timer/SStimer
 
 /datum/controller/subsystem/timer
@@ -10,72 +23,96 @@ var/datum/controller/subsystem/timer/SStimer
 	wait = 1 //SS_TICKER subsystem, so wait is in ticks
 	priority = SS_PRIORITY_TIMER
 
-	flags = SS_FIRE_IN_LOBBY|SS_TICKER|SS_NO_INIT
+	flags = SS_TICKER|SS_NO_INIT|SS_FIRE_IN_LOBBY
 
-	var/list/datum/timedevent/second_queue = list() //awe, yes, you've had first queue, but what about second queue?
+	/// Queue used for storing timers that do not fit into the current buckets
+	var/list/datum/timedevent/second_queue = list()
+	/// A hashlist dictionary used for storing unique timers
 	var/list/hashes = list()
-
-	var/head_offset = 0 //world.time of the first entry in the the bucket.
-	var/practical_offset = 0 //index of the first non-empty item in the bucket.
-	var/bucket_resolution = 0 //world.tick_lag the bucket was designed for
-	var/bucket_count = 0 //how many timers are in the buckets
-
-	var/list/bucket_list = list() //list of buckets, each bucket holds every timer that has to run that byond tick.
-
-	var/list/timer_id_dict = list() //list of all active timers assoicated to their timer id (for easy lookup)
-
-	var/list/clienttime_timers = list() //special snowflake timers that run on fancy pansy "client time"
-
+	/// world.time of the first entry in the bucket list, effectively the 'start time' of the current buckets
+	var/head_offset = 0
+	/// Index of the wrap around pivot for buckets. buckets before this are later running buckets wrapped around from the end of the bucket list.
+	var/practical_offset = 1
+	/// world.tick_lag the bucket was designed for
+	var/bucket_resolution = 0
+	/// How many timers are in the buckets
+	var/bucket_count = 0
+	/// List of buckets, each bucket holds every timer that has to run that byond tick
+	var/list/bucket_list = list()
+	/// List of all active timers associated to their timer ID (for easy lookup)
+	var/list/timer_id_dict = list()
+	/// Special timers that run in real-time, not BYOND time; these are more expensive to run and maintain
+	var/list/clienttime_timers = list()
+	/// Contains the last time that a timer's callback was invoked, or the last tick the SS fired if no timers are being processed
 	var/last_invoke_tick = 0
+	/// Keeps track of the next index to work on for client timers
+	var/next_clienttime_timer_index = 0
+	/// Contains the last time that a warning was issued for not invoking callbacks
 	var/static/last_invoke_warning = 0
+	/// Boolean operator controlling if the timer SS will automatically reset buckets if it fails to invoke callbacks for an extended period of time
 	var/static/bucket_auto_reset = TRUE
+	/// How many times bucket was reset
+	var/bucket_reset_count = 0
 
 /datum/controller/subsystem/timer/New()
 	NEW_SS_GLOBAL(SStimer)
 	bucket_list.len = BUCKET_LEN
+	head_offset = world.time
+	bucket_resolution = world.tick_lag
 
 /datum/controller/subsystem/timer/stat_entry(msg)
-	..("B:[bucket_count] P:[length(second_queue)] H:[length(hashes)] C:[length(clienttime_timers)]")
+	..("B:[bucket_count] P:[length(second_queue)] H:[length(hashes)] C:[length(clienttime_timers)] S:[length(timer_id_dict)] RST:[bucket_reset_count]")
 
-/datum/controller/subsystem/timer/fire(resumed = FALSE)
-	var/lit = last_invoke_tick
-	var/last_check = world.time - TICKS2DS(BUCKET_LEN*1.5)
-	var/list/bucket_list = src.bucket_list
-
-	if(!bucket_count)
-		last_invoke_tick = world.time
-
-	if(lit && lit < last_check && head_offset < last_check && last_invoke_warning < last_check)
-		last_invoke_warning = world.time
-		var/msg = "No regular timers processed in the last [BUCKET_LEN*1.5] ticks[bucket_auto_reset ? ", resetting buckets" : ""]!"
-		message_admins(msg)
-		WARNING(msg)
-		if(bucket_auto_reset)
-			bucket_resolution = 0
-
-		log_ss(name, "Timer bucket reset. world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
+/datum/controller/subsystem/timer/proc/dump_timer_buckets(full = TRUE)
+	var/list/to_log = list("Timer bucket reset. world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
+	if (full)
 		for (var/i in 1 to length(bucket_list))
 			var/datum/timedevent/bucket_head = bucket_list[i]
 			if (!bucket_head)
 				continue
 
-			log_ss(name, "Active timers at index [i]:")
-
+			to_log += "Active timers at index [i]:"
 			var/datum/timedevent/bucket_node = bucket_head
-			var/anti_loop_check = 1000
+			var/anti_loop_check = 1
 			do
-				log_ss(name, get_timer_debug_string(bucket_node))
+				to_log += 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_ss(name, "Active timers in the second_queue queue:")
+
+		to_log += "Active timers in the second_queue queue:"
 		for(var/I in second_queue)
-			log_ss(name, get_timer_debug_string(I))
+			to_log += get_timer_debug_string(I)
 
-	var/next_clienttime_timer_index = 0
-	var/len = length(clienttime_timers)
+	// Dump all the logged data to the world log
+	log_ss("timers", to_log.Join("\n"))
 
-	for (next_clienttime_timer_index in 1 to len)
+/datum/controller/subsystem/timer/fire(resumed = FALSE)
+	// Store local references to datum vars as it is faster to access them
+	var/lit = last_invoke_tick
+	var/list/bucket_list = src.bucket_list
+	var/last_check = world.time - TICKS2DS(BUCKET_LEN * 1.5)
+
+	// If there are no timers being tracked, then consider now to be the last invoked time
+	if(!bucket_count)
+		last_invoke_tick = world.time
+
+	// Check that we have invoked a callback in the last 1.5 minutes of BYOND time,
+	// and throw a warning and reset buckets if this is true
+	if(lit && lit < last_check && head_offset < last_check && last_invoke_warning < last_check)
+		last_invoke_warning = world.time
+		var/msg = "No regular timers processed in the last [BUCKET_LEN * 1.5] ticks[bucket_auto_reset ? ", resetting buckets" : ""]!"
+		message_admins(msg)
+		WARNING(msg)
+		if(bucket_auto_reset)
+			bucket_resolution = 0
+		dump_timer_buckets()
+
+	// Process client-time timers
+	if (next_clienttime_timer_index)
+		clienttime_timers.Cut(1, next_clienttime_timer_index+1)
+		next_clienttime_timer_index = 0
+	for (next_clienttime_timer_index in 1 to length(clienttime_timers))
 		if (MC_TICK_CHECK)
 			next_clienttime_timer_index--
 			break
@@ -86,8 +123,8 @@ var/datum/controller/subsystem/timer/SStimer
 
 		var/datum/callback/callBack = ctime_timer.callBack
 		if (!callBack)
-			clienttime_timers.Cut(next_clienttime_timer_index,next_clienttime_timer_index+1)
-			CRASH("Invalid timer: [get_timer_debug_string(ctime_timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset], REALTIMEOFDAY: [REALTIMEOFDAY]")
+			CRASH("Invalid timer: [get_timer_debug_string(ctime_timer)] world.time: [world.time], \
+				head_offset: [head_offset], practical_offset: [practical_offset], REALTIMEOFDAY: [REALTIMEOFDAY]")
 
 		ctime_timer.spent = REALTIMEOFDAY
 		callBack.InvokeAsync()
@@ -95,148 +132,115 @@ var/datum/controller/subsystem/timer/SStimer
 		if(ctime_timer.flags & TIMER_LOOP)
 			ctime_timer.spent = 0
 			ctime_timer.timeToRun = REALTIMEOFDAY + ctime_timer.wait
-			BINARY_INSERT(ctime_timer, clienttime_timers, datum/timedevent, timeToRun)
+			BINARY_INSERT(ctime_timer, clienttime_timers, /datum/timedevent, ctime_timer, timeToRun, COMPARE_KEY)
 		else
 			qdel(ctime_timer)
 
-
+	// Remove invoked client-time timers
 	if (next_clienttime_timer_index)
 		clienttime_timers.Cut(1, next_clienttime_timer_index+1)
+		next_clienttime_timer_index = 0
 
-	if (MC_TICK_CHECK)
-		return
-
-	var/static/list/spent = list()
-	var/static/datum/timedevent/timer
+	// Check for when we need to loop the buckets, this occurs when
+	// the head_offset is approaching BUCKET_LEN ticks in the past
 	if (practical_offset > BUCKET_LEN)
 		head_offset += TICKS2DS(BUCKET_LEN)
 		practical_offset = 1
 		resumed = FALSE
 
+	// Check for when we have to reset buckets, typically from auto-reset
 	if ((length(bucket_list) != BUCKET_LEN) || (world.tick_lag != bucket_resolution))
 		reset_buckets()
 		bucket_list = src.bucket_list
 		resumed = FALSE
 
-
-	if (!resumed)
-		timer = null
-
-	while (practical_offset <= BUCKET_LEN && head_offset + ((practical_offset-1)*world.tick_lag) <= world.time)
-		var/datum/timedevent/head = bucket_list[practical_offset]
-		if (!timer || !head || timer == head)
-			head = bucket_list[practical_offset]
-			timer = head
-		while (timer)
+	// Iterate through each bucket starting from the practical offset
+	while (practical_offset <= BUCKET_LEN && head_offset + ((practical_offset - 1) * world.tick_lag) <= world.time)
+		var/datum/timedevent/timer
+		while ((timer = bucket_list[practical_offset]))
 			var/datum/callback/callBack = timer.callBack
 			if (!callBack)
-				bucket_resolution = null //force bucket recreation
-				CRASH("Invalid timer: [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
+				crash_with("Invalid timer: [get_timer_debug_string(timer)] world.time: [world.time], \
+					head_offset: [head_offset], practical_offset: [practical_offset], bucket_joined: [timer.bucket_joined]")
+				if (!timer.spent)
+					bucket_resolution = null // force bucket recreation
+					return
 
+			timer.bucketEject() //pop the timer off of the bucket list.
+
+			// Invoke callback if possible
 			if (!timer.spent)
-				spent += timer
 				timer.spent = world.time
 				callBack.InvokeAsync()
 				last_invoke_tick = world.time
 
-			if (MC_TICK_CHECK)
-				return
-
-			timer = timer.next
-			if (timer == head)
-				break
-
-
-		bucket_list[practical_offset++] = null
-
-		//we freed up a bucket, lets see if anything in second_queue needs to be shifted to that bucket.
-		var/i = 0
-		var/L = length(second_queue)
-		for (i in 1 to L)
-			timer = second_queue[i]
-			if (timer.timeToRun >= TIMER_MAX)
-				i--
-				break
-
-			if (timer.timeToRun < head_offset)
-				bucket_resolution = null //force bucket recreation
-				crash_with("[i] Invalid timer state: Timer in long run queue with a time to run less then head_offset. [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
-
-				if (timer.callBack && !timer.spent)
-					timer.callBack.InvokeAsync()
-					spent += timer
-					bucket_count++
-				else if(!QDELETED(timer))
-					qdel(timer)
-				continue
-
-			if (timer.timeToRun < head_offset + TICKS2DS(practical_offset-1))
-				bucket_resolution = null //force bucket recreation
-				crash_with("[i] Invalid timer state: Timer in long run queue that would require a backtrack to transfer to short run queue. [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
-				if (timer.callBack && !timer.spent)
-					timer.callBack.InvokeAsync()
-					spent += timer
-					bucket_count++
-				else if(!QDELETED(timer))
-					qdel(timer)
-				continue
-
-			bucket_count++
-			var/bucket_pos = max(1, BUCKET_POS(timer))
-
-			var/datum/timedevent/bucket_head = bucket_list[bucket_pos]
-			if (!bucket_head)
-				bucket_list[bucket_pos] = timer
-				timer.next = null
-				timer.prev = null
-				continue
-
-			if (!bucket_head.prev)
-				bucket_head.prev = bucket_head
-			timer.next = bucket_head
-			timer.prev = bucket_head.prev
-			timer.next.prev = timer
-			timer.prev.next = timer
-		if (i)
-			second_queue.Cut(1, i+1)
-
-		timer = null
-
-	bucket_count -= length(spent)
-
-	for (var/i in spent)
-		var/datum/timedevent/qtimer = i
-		if(QDELETED(qtimer))
-			bucket_count++
-			continue
-		if(!(qtimer.flags & TIMER_LOOP))
-			qdel(qtimer)
-		else
-			bucket_count++
-			qtimer.spent = 0
-			qtimer.bucketEject()
-			if(qtimer.flags & TIMER_CLIENT_TIME)
-				qtimer.timeToRun = REALTIMEOFDAY + qtimer.wait
+			if (timer.flags & TIMER_LOOP) // Prepare looping timers to re-enter the queue
+				timer.spent = 0
+				timer.timeToRun = world.time + timer.wait
+				timer.bucketJoin()
 			else
-				qtimer.timeToRun = world.time + qtimer.wait
-			qtimer.bucketJoin()
+				qdel(timer)
 
-	spent.len = 0
+			if (MC_TICK_CHECK)
+				break
 
+		if (!bucket_list[practical_offset])
+			// Empty the bucket, check if anything in the secondary queue should be shifted to this bucket
+			bucket_list[practical_offset] = null // Just in case
+			practical_offset++
+			var/i = 0
+			for (i in 1 to length(second_queue))
+				timer = second_queue[i]
+				if (timer.timeToRun >= TIMER_MAX)
+					i--
+					break
+
+				// Check for timers that are scheduled to run in the past
+				if (timer.timeToRun < head_offset)
+					bucket_resolution = null // force bucket recreation
+					crash_with("[i] Invalid timer state: Timer in long run queue with a time to run less then head_offset. \
+						[get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
+					break
+
+				// Check for timers that are not capable of being scheduled to run without rebuilding buckets
+				if (timer.timeToRun < head_offset + TICKS2DS(practical_offset - 1))
+					bucket_resolution = null // force bucket recreation
+					crash_with("[i] Invalid timer state: Timer in long run queue that would require a backtrack to transfer to \
+						short run queue. [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
+					break
+
+				timer.bucketJoin()
+			if (i)
+				second_queue.Cut(1, i+1)
+		if (MC_TICK_CHECK)
+			break
+
+/**
+ * Generates a string with details about the timed event for debugging purposes
+ */
 /datum/controller/subsystem/timer/proc/get_timer_debug_string(datum/timedevent/TE)
 	. = "Timer: [TE]"
 	. += "Prev: [TE.prev ? TE.prev : "NULL"], Next: [TE.next ? TE.next : "NULL"]"
 	if(TE.spent)
-		. += ", SPENT"
+		. += ", SPENT([TE.spent])"
 	if(QDELETED(TE))
 		. += ", QDELETED"
+	if(!TE.callBack)
+		. += ", NO CALLBACK"
 
+/**
+  * Destroys the existing buckets and creates new buckets from the existing timed events
+  */
 /datum/controller/subsystem/timer/proc/reset_buckets()
-	var/list/bucket_list = src.bucket_list
+	log_debug("Timer buckets have been reset, this may cause timers to lag")
+	bucket_reset_count++
+
+	var/list/bucket_list = src.bucket_list // Store local reference to datum var, this is faster
 	var/list/alltimers = list()
-	//collect the timers currently in the bucket
+
+	// Get all timers currently in the buckets
 	for (var/bucket_head in bucket_list)
-		if (!bucket_head)
+		if (!bucket_head) // if bucket is empty for this tick
 			continue
 		var/datum/timedevent/bucket_node = bucket_head
 		do
@@ -244,25 +248,38 @@ var/datum/controller/subsystem/timer/SStimer
 			bucket_node = bucket_node.next
 		while(bucket_node && bucket_node != bucket_head)
 
+	// Empty the list by zeroing and re-assigning the length
 	bucket_list.len = 0
 	bucket_list.len = BUCKET_LEN
 
+	// Reset values for the subsystem to their initial values
 	practical_offset = 1
 	bucket_count = 0
 	head_offset = world.time
 	bucket_resolution = world.tick_lag
 
+	// Add all timed events from the secondary queue as well
 	alltimers += second_queue
+
+	// If there are no timers being tracked by the subsystem,
+	// there is no need to do any further rebuilding
 	if (!length(alltimers))
 		return
 
-	sortTim(alltimers, /proc/cmp_timer)
+	// Sort all timers by time to run
+	sortTim(alltimers, .proc/cmp_timer)
 
+	// Get the earliest timer, and if the TTR is earlier than the current world.time,
+	// then set the head offset appropriately to be the earliest time tracked by the
+	// current set of buckets
 	var/datum/timedevent/head = alltimers[1]
-
 	if (head.timeToRun < head_offset)
 		head_offset = head.timeToRun
 
+	// Iterate through each timed event and insert it into an appropriate bucket,
+	// up unto the point that we can no longer insert into buckets as the TTR
+	// is outside the range we are tracking, then insert the remainder into the
+	// secondary queue
 	var/new_bucket_count
 	var/i = 1
 	for (i in 1 to length(alltimers))
@@ -270,19 +287,24 @@ var/datum/controller/subsystem/timer/SStimer
 		if (!timer)
 			continue
 
-		var/bucket_pos = BUCKET_POS(timer)
+		// Check that the TTR is within the range covered by buckets, when exceeded we've finished
 		if (timer.timeToRun >= TIMER_MAX)
 			i--
 			break
 
-
+		// Check that timer has a valid callback and hasn't been invoked
 		if (!timer.callBack || timer.spent)
-			WARNING("Invalid timer: [get_timer_debug_string(timer)] world.time: [world.time], head_offset: [head_offset], practical_offset: [practical_offset]")
+			WARNING("Invalid timer: [get_timer_debug_string(timer)] world.time: [world.time], \
+				head_offset: [head_offset], practical_offset: [practical_offset]")
 			if (timer.callBack)
 				qdel(timer)
 			continue
 
+		// Insert the timer into the bucket, and perform necessary circular doubly-linked list operations
 		new_bucket_count++
+		var/bucket_pos = BUCKET_POS(timer)
+		timer.bucket_pos = bucket_pos
+
 		var/datum/timedevent/bucket_head = bucket_list[bucket_pos]
 		if (!bucket_head)
 			bucket_list[bucket_pos] = timer
@@ -290,14 +312,14 @@ var/datum/controller/subsystem/timer/SStimer
 			timer.prev = null
 			continue
 
-		if (!bucket_head.prev)
-			bucket_head.prev = bucket_head
+		bucket_head.prev = timer
 		timer.next = bucket_head
-		timer.prev = bucket_head.prev
-		timer.next.prev = timer
-		timer.prev.next = timer
+		timer.prev = null
+		bucket_list[bucket_pos] = timer
+
+	// Cut the timers that are tracked by the buckets from the secondary queue
 	if (i)
-		alltimers.Cut(1, i+1)
+		alltimers.Cut(1, i + 1)
 	second_queue = alltimers
 	bucket_count = new_bucket_count
 
@@ -308,46 +330,68 @@ var/datum/controller/subsystem/timer/SStimer
 	timer_id_dict |= SStimer.timer_id_dict
 	bucket_list |= SStimer.bucket_list
 
+/**
+  * # Timed Event
+  *
+  * This is the actual timer, it contains the callback and necessary data to maintain
+  * the timer.
+  *
+  * See the documentation for the timer subsystem for an explanation of the buckets referenced
+  * below in next and prev
+  */
 /datum/timedevent
+	/// ID used for timers when the TIMER_STOPPABLE flag is present
 	var/id
+	/// The callback to invoke after the timer completes
 	var/datum/callback/callBack
+	/// The time at which the callback should be invoked at
 	var/timeToRun
+	/// The length of the timer
 	var/wait
+	/// Unique hash generated when TIMER_UNIQUE flag is present
 	var/hash
+	/// The source of the timedevent, whatever called addtimer
+	var/source
+	/// Flags associated with the timer, see _DEFINES/subsystems.dm
 	var/list/flags
-	var/spent = FALSE //set to true right before running.
-	var/name //for easy debugging.
-
-	//cicular doublely linked list
+	/// Time at which the timer was invoked or destroyed
+	var/spent = 0
+	/// An informative name generated for the timer as its representation in strings, useful for debugging
+	var/name
+	/// Next timed event in the bucket
 	var/datum/timedevent/next
+	/// Previous timed event in the bucket
 	var/datum/timedevent/prev
+	/// Boolean indicating if timer joined into bucket
+	var/bucket_joined = FALSE
+	/// Initial bucket position
+	var/bucket_pos = -1
 
+/datum/timedevent/New(datum/callback/callBack, wait, flags, hash, source)
 	var/static/nextid = 1
-
-/datum/timedevent/New(datum/callback/callBack, wait, flags, hash)
 	id = TIMER_ID_NULL
 	src.callBack = callBack
 	src.wait = wait
 	src.flags = flags
 	src.hash = hash
+	src.source = source
 
-	if (flags & TIMER_CLIENT_TIME)
-		timeToRun = REALTIMEOFDAY + wait
-	else
-		timeToRun = world.time + wait
+	// Determine time at which the timer's callback should be invoked
+	timeToRun = (flags & TIMER_CLIENT_TIME ? REALTIMEOFDAY : world.time) + wait
 
+	// Include the timer in the hash table if the timer is unique
 	if (flags & TIMER_UNIQUE)
 		SStimer.hashes[hash] = src
+
+	// Generate ID for the timer if the timer is stoppable, include in the timer id dictionary
 	if (flags & TIMER_STOPPABLE)
 		id = num2text(nextid, 100)
 		if (nextid >= TIMER_ID_MAX)
-			nextid += min(1, 2**round(nextid/TIMER_ID_MAX))
+			nextid += min(1, 2 ** round(nextid / TIMER_ID_MAX))
 		else
 			nextid++
 		SStimer.timer_id_dict[id] = src
 
-	name = "Timer: " + num2text(id, 8) + ", TTR: [timeToRun], Flags: [jointext(bitfield2list(flags, list("TIMER_UNIQUE", "TIMER_OVERRIDE", "TIMER_CLIENT_TIME", "TIMER_STOPPABLE", "TIMER_NO_HASH_WAIT", "TIMER_LOOP")), ", ")], callBack: \ref[callBack], callBack.object: [callBack.object]\ref[callBack.object]([getcallingtype()]), callBack.delegate:[callBack.delegate]([callBack.arguments ? callBack.arguments.Join(", ") : ""])"
-
 	if ((timeToRun < world.time || timeToRun < SStimer.head_offset) && !(flags & TIMER_CLIENT_TIME))
 		CRASH("Invalid timer state: Timer created that would require a backtrack to run (addtimer would never let this happen): [SStimer.get_timer_debug_string(src)]")
 
@@ -388,64 +432,104 @@ var/datum/controller/subsystem/timer/SStimer
 	prev = null
 	return QDEL_HINT_IWILLGC
 
+/**
+  * Removes this timed event from any relevant buckets, or the secondary queue
+  */
 /datum/timedevent/proc/bucketEject()
-	var/bucketpos = BUCKET_POS(src)
+	// Store local references for the bucket list and secondary queue
+	// This is faster than referencing them from the datum itself
 	var/list/bucket_list = SStimer.bucket_list
 	var/list/second_queue = SStimer.second_queue
+
+	// Attempt to get the head of the bucket
 	var/datum/timedevent/buckethead
-	if(bucketpos > 0)
-		buckethead = bucket_list[bucketpos]
+	if(bucket_pos > 0)
+		buckethead = bucket_list[bucket_pos]
+
+	// Decrement the number of timers in buckets if the timed event is
+	// the head of the bucket, or has a TTR less than TIMER_MAX implying it fits
+	// into an existing bucket, or is otherwise not present in the secondary queue
 	if(buckethead == src)
-		bucket_list[bucketpos] = next
+		bucket_list[bucket_pos] = next
 		SStimer.bucket_count--
-	else if(timeToRun < TIMER_MAX || next || prev)
+	else if(timeToRun < TIMER_MAX)
 		SStimer.bucket_count--
 	else
 		var/l = length(second_queue)
 		second_queue -= src
 		if(l == length(second_queue))
 			SStimer.bucket_count--
-	if(prev != next)
+
+	// Remove the timed event from the bucket, ensuring to maintain
+	// the integrity of the bucket's list if relevant
+	if (prev && prev.next == src)
 		prev.next = next
+	if (next && next.prev == src)
 		next.prev = prev
-	else
-		prev?.next = null
-		next?.prev = null
 	prev = next = null
+	bucket_pos = -1
+	bucket_joined = FALSE
 
+/**
+  * Attempts to add this timed event to a bucket, will enter the secondary queue
+  * if there are no appropriate buckets at this time.
+  *
+  * Secondary queueing of timed events will occur when the timespan covered by the existing
+  * buckets is exceeded by the time at which this timed event is scheduled to be invoked.
+  * If the timed event is tracking client time, it will be added to a special bucket.
+  */
 /datum/timedevent/proc/bucketJoin()
-	var/list/L
+	// Generate debug-friendly name for timer
+	var/static/list/bitfield_flags = list("TIMER_UNIQUE", "TIMER_OVERRIDE", "TIMER_CLIENT_TIME", "TIMER_STOPPABLE", "TIMER_NO_HASH_WAIT", "TIMER_LOOP")
+	name = "Timer: [id] (\ref[src]), TTR: [timeToRun], wait:[wait] Flags: [jointext(bitfield2list(flags, bitfield_flags), ", ")], \
+		callBack: \ref[callBack], callBack.object: [callBack.object]\ref[callBack.object]([getcallingtype()]), \
+		callBack.delegate:[callBack.delegate]([callBack.arguments ? callBack.arguments.Join(", ") : ""]), source: [source]"
 
+	if (bucket_joined)
+		crash_with("Bucket already joined! [name]")
+
+	// Check if this timed event should be diverted to the client time bucket, or the secondary queue
+	var/list/L
 	if (flags & TIMER_CLIENT_TIME)
 		L = SStimer.clienttime_timers
 	else if (timeToRun >= TIMER_MAX)
 		L = SStimer.second_queue
-
 	if(L)
-		BINARY_INSERT(src, L, datum/timedevent, timeToRun)
+		BINARY_INSERT(src, L, /datum/timedevent, src, timeToRun, COMPARE_KEY)
 		return
 
-	//get the list of buckets
+	// Get a local reference to the bucket list, this is faster than referencing the datum
 	var/list/bucket_list = SStimer.bucket_list
 
-	//calculate our place in the bucket list
-	var/bucket_pos = BUCKET_POS(src)
+	// Find the correct bucket for this timed event
+	bucket_pos = BUCKET_POS(src)
+
+	if (bucket_pos < SStimer.practical_offset && timeToRun < (SStimer.head_offset + TICKS2DS(BUCKET_LEN)))
+		WARNING("Bucket pos in past: bucket_pos = [bucket_pos] < practical_offset = [SStimer.practical_offset] \
+			&& timeToRun = [timeToRun] < [SStimer.head_offset + TICKS2DS(BUCKET_LEN)], Timer: [name]")
+		bucket_pos = SStimer.practical_offset // Recover bucket_pos to avoid timer blocking queue
 
-	//get the bucket for our tick
 	var/datum/timedevent/bucket_head = bucket_list[bucket_pos]
 	SStimer.bucket_count++
-	//empty bucket, we will just add ourselves
+
+	// If there is no timed event at this position, then the bucket is 'empty'
+	// and we can just set this event to that position
 	if (!bucket_head)
+		bucket_joined = TRUE
 		bucket_list[bucket_pos] = src
 		return
-	//other wise, lets do a simplified linked list add.
-	if (!bucket_head.prev)
-		bucket_head.prev = bucket_head
-	next = bucket_head
-	prev = bucket_head.prev
-	next.prev = src
-	prev.next = src
 
+	// Otherwise, we merely add this timed event into the bucket, which is a
+	// doubly-linked list
+	bucket_joined = TRUE
+	bucket_head.prev = src
+	next = bucket_head
+	prev = null
+	bucket_list[bucket_pos] = src
+
+/**
+  * Returns a string of the type of the callback for this timer
+  */
 /datum/timedevent/proc/getcallingtype()
 	. = "ERROR"
 	if (callBack.object == GLOBAL_PROC)
@@ -453,39 +537,47 @@ var/datum/controller/subsystem/timer/SStimer
 	else
 		. = "[callBack.object.type]"
 
+/**
+ * Create a new timer and insert it in the queue.
+ * You should not call this directly, and should instead use the addtimer macro, which includes source information.
+ *
+ * Arguments:
+ * * callback the callback to call on timer finish
+ * * wait deciseconds to run the timer for
+ * * flags flags for this timer, see: code\__DEFINES\subsystems.dm
+ */
 /proc/addtimer(datum/callback/callback, wait, flags)
 	if (!callback)
 		CRASH("addtimer called without a callback")
 
 	if (wait < 0)
-		crash_with("addtimer called with a negative wait. Converting to 0")
+		CRASH("addtimer called with a negative wait. Converting to [world.tick_lag]")
 
 	if (callback.object != GLOBAL_PROC && QDELETED(callback.object) && !QDESTROYING(callback.object))
-		crash_with("addtimer called with a callback assigned to a qdeleted object. In the future such timers will not be supported and may refuse to run or run with a 0 wait")
+		CRASH("addtimer called with a callback assigned to a qdeleted object. In the future such timers will not \
+			be supported and may refuse to run or run with a 0 wait")
 
-	wait = max(wait, 0)
+	wait = max(Ceilm(wait, world.tick_lag), world.tick_lag)
 
 	if(wait >= INFINITY)
 		CRASH("Attempted to create timer with INFINITY delay")
 
+	// Generate hash if relevant for timed events with the TIMER_UNIQUE flag
 	var/hash
-
 	if (flags & TIMER_UNIQUE)
-		var/list/hashlist
-		if(flags & TIMER_NO_HASH_WAIT)
-			hashlist = list(callback.object, "(\ref[callback.object])", callback.delegate, flags & TIMER_CLIENT_TIME)
-		else
-			hashlist = list(callback.object, "(\ref[callback.object])", callback.delegate, wait, flags & TIMER_CLIENT_TIME)
+		var/list/hashlist = list(callback.object, "(\ref[callback.object])", callback.delegate, flags & TIMER_CLIENT_TIME)
+		if(!(flags & TIMER_NO_HASH_WAIT))
+			hashlist += wait
 		hashlist += callback.arguments
 		hash = hashlist.Join("|||||||")
 
 		var/datum/timedevent/hash_timer = SStimer.hashes[hash]
 		if(hash_timer)
-			if (hash_timer.spent) //it's pending deletion, pretend it doesn't exist.
-				hash_timer.hash = null
-				SStimer.hashes -= hash
+			if (hash_timer.spent) // it's pending deletion, pretend it doesn't exist.
+				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
 					qdel(hash_timer)
 				else
 					if (hash_timer.flags & TIMER_STOPPABLE)
@@ -497,18 +589,22 @@ var/datum/controller/subsystem/timer/SStimer
 	var/datum/timedevent/timer = new(callback, wait, flags, hash)
 	return timer.id
 
+
+/**
+ * Delete a timer
+ *
+ * Arguments:
+ * * id a timerid or a /datum/timedevent
+ */
 /proc/deltimer(id)
 	if (!id)
 		return FALSE
-
 	if (id == TIMER_ID_NULL)
-		CRASH("Tried to delete a null timerid. Use the TIMER_STOPPABLE flag.")
-
-	if (!istext(id))
-		if (istype(id, /datum/timedevent))
-			qdel(id)
-			return TRUE
-
+		CRASH("Tried to delete a null timerid. Use TIMER_STOPPABLE flag")
+	if (istype(id, /datum/timedevent))
+		qdel(id)
+		return TRUE
+	//id is string
 	var/datum/timedevent/timer = SStimer.timer_id_dict[id]
 	if (timer && !timer.spent)
 		qdel(timer)