progressbar refactor (#50316)

code/__DEFINES/dcs/signals.dm

@@ -173,6 +173,8 @@
 #define COMSIG_MOVABLE_DISPOSING "movable_disposing"			//called when the movable is added to a disposal holder object for disposal movement: (obj/structure/disposalholder/holder, obj/machinery/disposal/source)
 
 // /mob signals
+#define COMSIG_MOB_LOGIN "mob_login"							//from base of /mob/Login(): ()
+#define COMSIG_MOB_LOGOUT "mob_logout"							//from base of /mob/Logout(): ()
 #define COMSIG_MOB_DEATH "mob_death"							//from base of mob/death(): (gibbed)
 #define COMSIG_MOB_STATCHANGE "mob_statchange"					//from base of mob/set_stat(): (new_stat)
 #define COMSIG_MOB_CLICKON "mob_clickon"						//from base of mob/clickon(): (atom/A, params)

code/__HELPERS/mobs.dm

@@ -177,14 +177,15 @@ GLOBAL_LIST_EMPTY(species_list)
 		else
 			return "unknown"
 
-/proc/do_mob(mob/user , mob/target, time = 30, uninterruptible = 0, progress = 1, datum/callback/extra_checks = null)
+///Timed action involving two mobs, the user and the target.
+/proc/do_mob(mob/user , mob/target, time = 3 SECONDS, uninterruptible = FALSE, progress = TRUE, datum/callback/extra_checks = null)
 	if(!user || !target)
-		return 0
+		return FALSE
 	var/user_loc = user.loc
 
-	var/drifting = 0
+	var/drifting = FALSE
 	if(!user.Process_Spacemove(0) && user.inertia_dir)
-		drifting = 1
+		drifting = TRUE
 
 	var/target_loc = target.loc
 
@@ -195,26 +196,26 @@ GLOBAL_LIST_EMPTY(species_list)
 
 	var/endtime = world.time+time
 	var/starttime = world.time
-	. = 1
+	. = TRUE
 	while (world.time < endtime)
 		stoplag(1)
-		if (progress)
+		if(!QDELETED(progbar))
 			progbar.update(world.time - starttime)
 		if(QDELETED(user) || QDELETED(target))
-			. = 0
+			. = FALSE
 			break
 		if(uninterruptible)
 			continue
 
 		if(drifting && !user.inertia_dir)
-			drifting = 0
+			drifting = FALSE
 			user_loc = user.loc
 
 		if((!drifting && user.loc != user_loc) || target.loc != target_loc || user.get_active_held_item() != holding || user.incapacitated() || (extra_checks && !extra_checks.Invoke()))
-			. = 0
+			. = FALSE
 			break
-	if (progress)
-		qdel(progbar)
+	if(!QDELETED(progbar))
+		progbar.end_progress()
 
 
 //some additional checks as a callback for for do_afters that want to break on losing health or on the mob taking action
@@ -231,10 +232,10 @@ GLOBAL_LIST_EMPTY(species_list)
 		checked_health["health"] = health
 	return ..()
 
-/proc/do_after(mob/user, var/delay, needhand = 1, atom/target = null, progress = 1, datum/callback/extra_checks = null)
+///Timed action involving one mob user. Target is optional.
+/proc/do_after(mob/user, var/delay, needhand = TRUE, atom/target = null, progress = TRUE, datum/callback/extra_checks = null)
 	if(!user)
-		return 0
-
+		return FALSE
 	var/atom/Tloc = null
 	if(target && !isturf(target))
 		Tloc = target.loc
@@ -245,51 +246,51 @@ GLOBAL_LIST_EMPTY(species_list)
 
 	var/atom/Uloc = user.loc
 
-	var/drifting = 0
+	var/drifting = FALSE
 	if(!user.Process_Spacemove(0) && user.inertia_dir)
-		drifting = 1
+		drifting = TRUE
 
 	var/holding = user.get_active_held_item()
 
-	var/holdingnull = 1 //User's hand started out empty, check for an empty hand
+	var/holdingnull = TRUE //User's hand started out empty, check for an empty hand
 	if(holding)
-		holdingnull = 0 //Users hand started holding something, check to see if it's still holding that
+		holdingnull = FALSE //Users hand started holding something, check to see if it's still holding that
 
 	delay *= user.do_after_coefficent()
 
 	var/datum/progressbar/progbar
-	if (progress)
-		progbar = new(user, delay, target)
+	if(progress)
+		progbar = new(user, delay, target || user)
 
 	var/endtime = world.time + delay
 	var/starttime = world.time
-	. = 1
+	. = TRUE
 	while (world.time < endtime)
 		stoplag(1)
-		if (progress)
+		if(!QDELETED(progbar))
 			progbar.update(world.time - starttime)
 
 		if(drifting && !user.inertia_dir)
-			drifting = 0
+			drifting = FALSE
 			Uloc = user.loc
 
 		if(QDELETED(user) || user.stat || (!drifting && user.loc != Uloc) || (extra_checks && !extra_checks.Invoke()))
-			. = 0
+			. = FALSE
 			break
 
 		if(isliving(user))
 			var/mob/living/L = user
 			if(L.IsStun() || L.IsParalyzed())
-				. = 0
+				. = FALSE
 				break
 
 		if(!QDELETED(Tloc) && (QDELETED(target) || Tloc != target.loc))
 			if((Uloc != Tloc || Tloc != user) && !drifting)
-				. = 0
+				. = FALSE
 				break
 
 		if(target && !(target in user.do_afters))
-			. = 0
+			. = FALSE
 			break
 
 		if(needhand)
@@ -297,13 +298,13 @@ GLOBAL_LIST_EMPTY(species_list)
 			//i.e the hand is used to pull some item/tool out of the construction
 			if(!holdingnull)
 				if(!holding)
-					. = 0
+					. = FALSE
 					break
 			if(user.get_active_held_item() != holding)
-				. = 0
+				. = FALSE
 				break
-	if (progress)
-		qdel(progbar)
+	if(!QDELETED(progbar))
+		progbar.end_progress()
 
 	if(!QDELETED(target))
 		LAZYREMOVE(user.do_afters, target)
@@ -313,16 +314,19 @@ GLOBAL_LIST_EMPTY(species_list)
 	. = 1
 	return
 
-/proc/do_after_mob(mob/user, list/targets, time = 30, uninterruptible = 0, progress = 1, datum/callback/extra_checks, required_mobility_flags = MOBILITY_STAND)
-	if(!user || !targets)
-		return 0
+///Timed action involving at least one mob user and a list of targets.
+/proc/do_after_mob(mob/user, list/targets, time = 3 SECONDS, uninterruptible = FALSE, progress = TRUE, datum/callback/extra_checks, required_mobility_flags = MOBILITY_STAND)
+	if(!user)
+		return FALSE
 	if(!islist(targets))
 		targets = list(targets)
+	if(!length(targets))
+		return FALSE
 	var/user_loc = user.loc
 
-	var/drifting = 0
+	var/drifting = FALSE
 	if(!user.Process_Spacemove(0) && user.inertia_dir)
-		drifting = 1
+		drifting = TRUE
 
 	var/list/originalloc = list()
 	for(var/atom/target in targets)
@@ -338,32 +342,32 @@ GLOBAL_LIST_EMPTY(species_list)
 	var/mob/living/L
 	if(isliving(user))
 		L = user
-	. = 1
+	. = TRUE
 	mainloop:
 		while(world.time < endtime)
 			stoplag(1)
-			if(progress)
+			if(!QDELETED(progbar))
 				progbar.update(world.time - starttime)
 			if(QDELETED(user) || !targets)
-				. = 0
+				. = FALSE
 				break
 			if(uninterruptible)
 				continue
 
 			if(drifting && !user.inertia_dir)
-				drifting = 0
+				drifting = FALSE
 				user_loc = user.loc
 
 			if(L && !((L.mobility_flags & required_mobility_flags) == required_mobility_flags))
-				. = 0
+				. = FALSE
 				break
 
 			for(var/atom/target in targets)
 				if((!drifting && user_loc != user.loc) || QDELETED(target) || originalloc[target] != target.loc || user.get_active_held_item() != holding || user.incapacitated() || (extra_checks && !extra_checks.Invoke()))
-					. = 0
+					. = FALSE
 					break mainloop
-	if(progbar)
-		qdel(progbar)
+	if(!QDELETED(progbar))
+		progbar.end_progress()
 
 /proc/is_species(A, species_datum)
 	. = FALSE

code/datums/components/storage/storage.dm

@@ -217,7 +217,7 @@
 	var/list/rejections = list()
 	while(do_after(M, 10, TRUE, parent, FALSE, CALLBACK(src, .proc/handle_mass_pickup, things, I.loc, rejections, progress)))
 		stoplag(1)
-	qdel(progress)
+	progress.end_progress()
 	to_chat(M, "<span class='notice'>You put everything you could [insert_preposition] [parent].</span>")
 
 /datum/component/storage/proc/handle_mass_item_insertion(list/things, datum/component/storage/src_object, mob/user, datum/progressbar/progress)
@@ -275,7 +275,7 @@
 	var/datum/progressbar/progress = new(M, length(things), T)
 	while (do_after(M, 10, TRUE, T, FALSE, CALLBACK(src, .proc/mass_remove_from_storage, T, things, progress)))
 		stoplag(1)
-	qdel(progress)
+	progress.end_progress()
 
 /datum/component/storage/proc/mass_remove_from_storage(atom/target, list/things, datum/progressbar/progress, trigger_on_found = TRUE)
 	var/atom/real_location = real_location()

code/datums/progressbar.dm

@@ -2,81 +2,133 @@
 #define PROGRESSBAR_ANIMATION_TIME 5
 
 /datum/progressbar
-	var/goal = 1
-	var/last_progress = 0
+	///The progress bar visual element.
 	var/image/bar
-	var/shown = 0
+	///The target where this progress bar is applied and where it is shown.
+	var/atom/bar_loc
+	///The mob whose client sees the progress bar.
 	var/mob/user
-	var/client/client
-	var/listindex
+	///The client seeing the progress bar.
+	var/client/user_client
+	///Effectively the number of steps the progress bar will need to do before reaching completion.
+	var/goal = 1
+	///Control check to see if the progress was interrupted before reaching its goal.
+	var/last_progress = 0
+	///Variable to ensure smooth visual stacking on multiple progress bars.
+	var/listindex = 0
+
 
 /datum/progressbar/New(mob/User, goal_number, atom/target)
 	. = ..()
 	if (!istype(target))
 		EXCEPTION("Invalid target given")
-	if (goal_number)
-		goal = goal_number
-	bar = image('icons/effects/progessbar.dmi', target, "prog_bar_0", HUD_LAYER)
+	if(QDELETED(User) || !istype(User))
+		stack_trace("/datum/progressbar created with [isnull(User) ? "null" : "invalid"] user")
+		qdel(src)
+		return
+	if(!isnum(goal_number))
+		stack_trace("/datum/progressbar created with [isnull(User) ? "null" : "invalid"] goal_number")
+		qdel(src)
+		return
+	goal = goal_number
+	bar_loc = target
+	bar = image('icons/effects/progessbar.dmi', bar_loc, "prog_bar_0", HUD_LAYER)
 	bar.plane = ABOVE_HUD_PLANE
 	bar.appearance_flags = APPEARANCE_UI_IGNORE_ALPHA
 	user = User
-	if(user)
-		client = user.client
 
-	LAZYINITLIST(user.progressbars)
-	LAZYINITLIST(user.progressbars[bar.loc])
-	var/list/bars = user.progressbars[bar.loc]
-	bars.Add(src)
+	LAZYADDASSOC(user.progressbars, bar_loc, src)
+	var/list/bars = user.progressbars[bar_loc]
 	listindex = bars.len
-	bar.pixel_y = 0
-	bar.alpha = 0
-	animate(bar, pixel_y = 32 + (PROGRESSBAR_HEIGHT * (listindex - 1)), alpha = 255, time = PROGRESSBAR_ANIMATION_TIME, easing = SINE_EASING)
 
-/datum/progressbar/proc/update(progress)
-	if (!user || !user.client)
-		shown = FALSE
-		return
-	if (user.client != client)
-		if (client)
-			client.images -= bar
-		if (user.client)
-			user.client.images += bar
+	if(user.client)
+		user_client = user.client
+		add_prog_bar_image_to_client()
 
-	progress = clamp(progress, 0, goal)
-	last_progress = progress
-	bar.icon_state = "prog_bar_[round(((progress / goal) * 100), 5)]"
-	if (!shown)
-		user.client.images += bar
-		shown = TRUE
+	RegisterSignal(user, COMSIG_PARENT_QDELETING, .proc/on_user_delete)
+	RegisterSignal(user, COMSIG_MOB_LOGOUT, .proc/clean_user_client)
+	RegisterSignal(user, COMSIG_MOB_LOGIN, .proc/on_user_login)
 
-/datum/progressbar/proc/shiftDown()
-	--listindex
-	bar.pixel_y = 32 + (PROGRESSBAR_HEIGHT * (listindex - 1))
-	var/dist_to_travel = 32 + (PROGRESSBAR_HEIGHT * (listindex - 1)) - PROGRESSBAR_HEIGHT
-	animate(bar, pixel_y = dist_to_travel, time = PROGRESSBAR_ANIMATION_TIME, easing = SINE_EASING)
 
 /datum/progressbar/Destroy()
+	if(user)
+		for(var/pb in user.progressbars[bar_loc])
+			var/datum/progressbar/progress_bar = pb
+			if(progress_bar == src || progress_bar.listindex <= listindex)
+				continue
+			progress_bar.listindex--
+
+			progress_bar.bar.pixel_y = 32 + (PROGRESSBAR_HEIGHT * (progress_bar.listindex - 1))
+			var/dist_to_travel = 32 + (PROGRESSBAR_HEIGHT * (progress_bar.listindex - 1)) - PROGRESSBAR_HEIGHT
+			animate(progress_bar.bar, pixel_y = dist_to_travel, time = PROGRESSBAR_ANIMATION_TIME, easing = SINE_EASING)
+
+		LAZYREMOVEASSOC(user.progressbars, bar_loc, src)
+		user = null
+
+	if(user_client)
+		clean_user_client()
+
+	bar_loc = null
+
+	if(bar)
+		QDEL_NULL(bar)
+
+	return ..()
+
+
+///Called right before the user's Destroy()
+/datum/progressbar/proc/on_user_delete(datum/source)
+	user.progressbars = null //We can simply nuke the list and stop worrying about updating other prog bars if the user itself is gone.
+	user = null
+	qdel(src)
+
+
+///Removes the progress bar image from the user_client and nulls the variable, if it exists.
+/datum/progressbar/proc/clean_user_client(datum/source)
+	if(!user_client) //Disconnected, already gone.
+		return
+	user_client.images -= bar
+	user_client = null
+
+
+///Called by user's Login(), it transfers the progress bar image to the new client.
+/datum/progressbar/proc/on_user_login(datum/source)
+	if(user_client)
+		if(user_client == user.client) //If this was not client handling I'd condemn this sanity check. But clients are fickle things.
+			return
+		clean_user_client()
+	if(!user.client) //Clients can vanish at any time, the bastards.
+		return
+	user_client = user.client
+	add_prog_bar_image_to_client()
+
+
+///Adds a smoothly-appearing progress bar image to the player's screen.
+/datum/progressbar/proc/add_prog_bar_image_to_client()
+	bar.pixel_y = 0
+	bar.alpha = 0
+	user_client.images += bar
+	animate(bar, pixel_y = 32 + (PROGRESSBAR_HEIGHT * (listindex - 1)), alpha = 255, time = PROGRESSBAR_ANIMATION_TIME, easing = SINE_EASING)
+
+
+///Updates the progress bar image visually.
+/datum/progressbar/proc/update(progress)
+	progress = clamp(progress, 0, goal)
+	if(progress == last_progress)
+		return
+	last_progress = progress
+	bar.icon_state = "prog_bar_[round(((progress / goal) * 100), 5)]"
+
+
+///Called on progress end, be it successful or a failure. Wraps up things to delete the datum and bar.
+/datum/progressbar/proc/end_progress()
 	if(last_progress != goal)
 		bar.icon_state = "[bar.icon_state]_fail"
-	for(var/I in user.progressbars[bar.loc])
-		var/datum/progressbar/P = I
-		if(P != src && P.listindex > listindex)
-			P.shiftDown()
-
-	var/list/bars = user.progressbars[bar.loc]
-	bars.Remove(src)
-	if(!bars.len)
-		LAZYREMOVE(user.progressbars, bar.loc)
 
 	animate(bar, alpha = 0, time = PROGRESSBAR_ANIMATION_TIME)
-	addtimer(CALLBACK(src, .proc/remove_from_client), PROGRESSBAR_ANIMATION_TIME, TIMER_CLIENT_TIME)
-	QDEL_IN(bar, PROGRESSBAR_ANIMATION_TIME * 2) //for garbage collection safety
-	. = ..()
 
-/datum/progressbar/proc/remove_from_client()
-	if(client)
-		client.images -= bar
-		client = null
+	QDEL_IN(src, PROGRESSBAR_ANIMATION_TIME)
+
 
 #undef PROGRESSBAR_ANIMATION_TIME
 #undef PROGRESSBAR_HEIGHT

code/game/atoms.dm

@@ -754,7 +754,7 @@
 	var/datum/component/storage/STR = GetComponent(/datum/component/storage)
 	while (do_after(user, 10, TRUE, src, FALSE, CALLBACK(STR, /datum/component/storage.proc/handle_mass_item_insertion, things, src_object, user, progress)))
 		stoplag(1)
-	qdel(progress)
+	progress.end_progress()
 	to_chat(user, "<span class='notice'>You dump as much of [src_object.parent]'s contents [STR.insert_preposition]to [src] as you can.</span>")
 	STR.orient2hud(user)
 	src_object.orient2hud(user)

code/game/turfs/turf.dm

@@ -370,7 +370,7 @@
 	var/datum/progressbar/progress = new(user, things.len, src)
 	while (do_after(usr, 10, TRUE, src, FALSE, CALLBACK(src_object, /datum/component/storage.proc/mass_remove_from_storage, src, things, progress)))
 		stoplag(1)
-	qdel(progress)
+	progress.end_progress()
 
 	return TRUE
 

code/modules/mob/login.dm

@@ -39,6 +39,8 @@
 	next_move = 1
 
 	..()
+	SEND_SIGNAL(src, COMSIG_MOB_LOGIN)
+
 	if (client && key != client.key)
 		key = client.key
 	reset_perspective(loc)

code/modules/mob/logout.dm

@@ -1,4 +1,5 @@
 /mob/Logout()
+	SEND_SIGNAL(src, COMSIG_MOB_LOGOUT)
 	log_message("[key_name(src)] is no longer owning mob [src]([src.type])", LOG_OWNERSHIP)
 	SStgui.on_logout(src)
 	unset_machine()

code/modules/mob/mob.dm

@@ -27,6 +27,9 @@
 	remove_from_dead_mob_list()
 	remove_from_alive_mob_list()
 	focus = null
+	if(length(progressbars))
+		stack_trace("[src] destroyed with elements in its progressbars list")
+		progressbars = null
 	for (var/alert in alerts)
 		clear_alert(alert, TRUE)
 	if(observers && observers.len)