Add job_globals sanitization, fix duplicate value (#24795)

* Add job_globals sanitization, fix duplicate value

* Remove 'support_positions' job category

* Add Supply dept to jobban list

code/modules/admin/db_ban/functions.dm

@@ -160,10 +160,10 @@
 		"rounds" = (rounds ? "[rounds]" : "0"), // And here
 		"ckey" = ckey,
 		"computerid" = computerid,
-		"ip" = ip,
+		"ip" = "[ip ? ip : ""]", // This is important. NULL is not the same as "", and if you directly open the `.dmb` file, you get a NULL IP.
 		"a_ckey" = a_ckey,
 		"a_computerid" = a_computerid,
-		"a_ip" = a_ip,
+		"a_ip" = "[a_ip ? a_ip : ""]",
 		"who" = who,
 		"adminwho" = adminwho,
 		"roundid" = GLOB.round_id,
@@ -490,7 +490,7 @@
 		output += "<option value='[j]'>[j]</option>"
 	for(var/j in GLOB.other_roles)
 		output += "<option value='[j]'>[j]</option>"
-	for(var/j in list("commanddept","securitydept","engineeringdept","medicaldept","sciencedept","supportdept","nonhumandept"))
+	for(var/j in list("commanddept","securitydept","engineeringdept","medicaldept","sciencedept","servicedept","nonhumandept"))
 		output += "<option value='[j]'>[j]</option>"
 	for(var/j in list("Syndicate") + GLOB.antag_roles)
 		output += "<option value='[j]'>[j]</option>"

code/modules/admin/topic.dm

@@ -161,7 +161,7 @@
 			message_admins("Ban process: A mob matching [playermob.ckey] was found at location [playermob.x], [playermob.y], [playermob.z]. Custom IP and computer id fields replaced with the IP and computer id from the located mob")
 
 		if(job_ban)
-			if(banjob in list("commanddept","securitydept","engineeringdept","medicaldept","sciencedept","supportdept","nonhumandept"))
+			if(banjob in list("commanddept","securitydept","engineeringdept","medicaldept","sciencedept","servicedept","supplydept","nonhumandept"))
 				multi_job = TRUE
 				switch(banjob)
 					if("commanddept")
@@ -194,8 +194,14 @@
 							var/datum/job/temp = SSjobs.GetJob(jobPos)
 							if(!temp) continue
 							jobs_to_ban += temp.title
-					if("supportdept")
-						for(var/jobPos in GLOB.support_positions)
+					if("servicedept")
+						for(var/jobPos in GLOB.service_positions)
+							if(!jobPos)	continue
+							var/datum/job/temp = SSjobs.GetJob(jobPos)
+							if(!temp) continue
+							jobs_to_ban += temp.title
+					if("supplydept")
+						for(var/jobPos in GLOB.supply_positions)
 							if(!jobPos)	continue
 							var/datum/job/temp = SSjobs.GetJob(jobPos)
 							if(!temp) continue
@@ -553,11 +559,32 @@
 				counter = 0
 		jobs += "</tr></table>"
 
-	//Support (Grey)
+	//Service (Grey)
 		counter = 0
 		jobs += "<table cellpadding='1' cellspacing='0' width='100%'>"
-		jobs += "<tr bgcolor='dddddd'><th colspan='[length(GLOB.support_positions)]'><a href='?src=[UID()];jobban3=supportdept;jobban4=[M.UID()];dbbanaddckey=[M.ckey]'>Support Positions</a></th></tr><tr align='center'>"
-		for(var/jobPos in GLOB.support_positions)
+		jobs += "<tr bgcolor='dddddd'><th colspan='[length(GLOB.service_positions)]'><a href='?src=[UID()];jobban3=servicedept;jobban4=[M.UID()];dbbanaddckey=[M.ckey]'>Service Positions</a></th></tr><tr align='center'>"
+		for(var/jobPos in GLOB.service_positions)
+			if(!jobPos)	continue
+			var/datum/job/job = SSjobs.GetJob(jobPos)
+			if(!job) continue
+
+			if(jobban_isbanned(M, job.title))
+				jobs += "<td width='20%'><a href='?src=[UID()];jobban3=[job.title];jobban4=[M.UID()];dbbanaddckey=[M.ckey]'><font color=red>[replacetext(job.title, " ", "&nbsp")]</font></a></td>"
+				counter++
+			else
+				jobs += "<td width='20%'><a href='?src=[UID()];jobban3=[job.title];jobban4=[M.UID()];dbbanaddckey=[M.ckey]'>[replacetext(job.title, " ", "&nbsp")]</a></td>"
+				counter++
+
+			if(counter >= 5) //So things dont get squiiiiished!
+				jobs += "</tr><tr align='center'>"
+				counter = 0
+		jobs += "</tr></table>"
+
+	//Supply (Brown)
+		counter = 0
+		jobs += "<table cellpadding='1' cellspacing='0' width='100%'>"
+		jobs += "<tr bgcolor='e2c59d'><th colspan='[length(GLOB.supply_positions)]'><a href='?src=[UID()];jobban3=supplydept;jobban4=[M.UID()];dbbanaddckey=[M.ckey]'>Supply Positions</a></th></tr><tr align='center'>"
+		for(var/jobPos in GLOB.supply_positions)
 			if(!jobPos)	continue
 			var/datum/job/job = SSjobs.GetJob(jobPos)
 			if(!job) continue
@@ -668,7 +695,7 @@
 			to_chat(usr, "<span class='warning'>SSjobs has not been setup!</span>")
 			return
 
-		//get jobs for department if specified, otherwise just returnt he one job in a list.
+		//get jobs for department if specified, otherwise just return the one job in a list.
 		var/list/joblist = list()
 		switch(href_list["jobban3"])
 			if("commanddept")
@@ -701,8 +728,14 @@
 					var/datum/job/temp = SSjobs.GetJob(jobPos)
 					if(!temp) continue
 					joblist += temp.title
-			if("supportdept")
-				for(var/jobPos in GLOB.support_positions)
+			if("servicedept")
+				for(var/jobPos in GLOB.service_positions)
+					if(!jobPos)	continue
+					var/datum/job/temp = SSjobs.GetJob(jobPos)
+					if(!temp) continue
+					joblist += temp.title
+			if("supplydept")
+				for(var/jobPos in GLOB.supply_positions)
 					if(!jobPos)	continue
 					var/datum/job/temp = SSjobs.GetJob(jobPos)
 					if(!temp) continue

code/modules/client/client_procs.dm

@@ -590,6 +590,9 @@
 		if(check_randomizer(connectiontopic))
 			return
 
+	var/client_address = address
+	if(!client_address) // Localhost can sometimes have no address set
+		client_address = "127.0.0.1"
 
 	if(sql_id)
 		//Just the standard check to see if it's actually a number
@@ -598,10 +601,6 @@
 		if(!isnum(sql_id))
 			return // Return here because if we somehow didnt pull a number from an INT column, EVERYTHING is breaking
 
-		var/client_address = address
-		if(!client_address) // Localhost can sometimes have no address set
-			client_address = "127.0.0.1"
-
 		//Player already identified previously, we need to just update the 'lastseen', 'ip' and 'computer_id' variables
 		var/datum/db_query/query_update = SSdbcore.NewQuery("UPDATE player SET lastseen=NOW(), ip=:sql_ip, computerid=:sql_cid, lastadminrank=:sql_ar WHERE id=:sql_id", list(
 			"sql_ip" = client_address,
@@ -622,7 +621,7 @@
 		//New player!! Need to insert all the stuff
 		var/datum/db_query/query_insert = SSdbcore.NewQuery("INSERT INTO player (id, ckey, firstseen, lastseen, ip, computerid, lastadminrank) VALUES (null, :ckey, Now(), Now(), :ip, :cid, :rank)", list(
 			"ckey" = ckey,
-			"ip" = address,
+			"ip" = client_address,
 			"cid" = computer_id,
 			"rank" = admin_rank
 		))

code/modules/unit_tests/_unit_tests.dm

@@ -2,6 +2,7 @@
 //Keep this sorted alphabetically
 
 #ifdef UNIT_TESTS
+#include "jobs\test_job_globals.dm"
 #include "aicard_icons.dm"
 #include "announcements.dm"
 #include "areas_apcs.dm"

code/modules/unit_tests/jobs/test_job_globals.dm

@@ -0,0 +1,26 @@
+/datum/unit_test/job_globals/Run()
+	return
+
+/datum/unit_test/job_globals/proc/is_list_unique(list/L)
+	var/list_length = length(L)
+	var/unique_list = uniqueList(L)
+	var/unique_list_length = length(unique_list)
+	return list_length == unique_list_length
+
+/datum/unit_test/job_globals/proc/validate_list(list/L, list_name)
+	if(!is_list_unique(L))
+		Fail("job_globals list '[list_name]' contains duplicate values.")
+
+/datum/unit_test/job_globals/no_duplicates/Run()
+	validate_list(GLOB.station_departments, "station_departments")
+	validate_list(GLOB.command_positions, "command_positions")
+	validate_list(GLOB.command_head_positions, "command_head_positions")
+	validate_list(GLOB.engineering_positions, "engineering_positions")
+	validate_list(GLOB.medical_positions, "medical_positions")
+	validate_list(GLOB.science_positions, "science_positions")
+	validate_list(GLOB.supply_positions, "supply_positions")
+	validate_list(GLOB.service_positions, "service_positions")
+	validate_list(GLOB.security_positions, "security_positions")
+	validate_list(GLOB.active_security_positions, "active_security_positions")
+	validate_list(GLOB.assistant_positions, "assistant_positions")
+	validate_list(GLOB.nonhuman_positions, "nonhuman_positions")