From 06969b4da1524771f8a023bea840ec73776b3818 Mon Sep 17 00:00:00 2001 From: Zantox Date: Wed, 27 Mar 2024 20:31:08 +0100 Subject: [PATCH] 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/game/jobs/job_globals.dm | 7 +-- code/game/objects/items/weapons/cards_ids.dm | 2 +- code/modules/admin/db_ban/functions.dm | 6 +-- code/modules/admin/topic.dm | 51 +++++++++++++++---- code/modules/client/client_procs.dm | 9 ++-- code/modules/unit_tests/_unit_tests.dm | 1 + .../unit_tests/jobs/test_job_globals.dm | 26 ++++++++++ 7 files changed, 79 insertions(+), 23 deletions(-) create mode 100644 code/modules/unit_tests/jobs/test_job_globals.dm diff --git a/code/game/jobs/job_globals.dm b/code/game/jobs/job_globals.dm index 5f922af3975..b469381bbfe 100644 --- a/code/game/jobs/job_globals.dm +++ b/code/game/jobs/job_globals.dm @@ -55,8 +55,7 @@ GLOBAL_LIST_INIT(science_positions, list( "Roboticist", )) -//BS12 EDIT -GLOBAL_LIST_INIT(support_positions, list( +GLOBAL_LIST_INIT(service_positions, list( "Head of Personnel", "Bartender", "Botanist", @@ -76,8 +75,6 @@ GLOBAL_LIST_INIT(supply_positions, list( "Shaft Miner" )) -GLOBAL_LIST_INIT(service_positions, (list("Head of Personnel") + (support_positions - supply_positions))) - /// Roles that include any semblence of security, mostly for jobbans GLOBAL_LIST_INIT(security_positions, list( "Head of Security", @@ -133,7 +130,7 @@ GLOBAL_LIST_INIT(nonhuman_positions, list( GLOBAL_LIST_INIT(exp_jobsmap, list( EXP_TYPE_LIVING = list(), // all living mobs - EXP_TYPE_CREW = list(titles = command_positions | engineering_positions | medical_positions | science_positions | support_positions | supply_positions | security_positions | assistant_positions | list("AI","Cyborg")), // crew positions + EXP_TYPE_CREW = list(titles = command_positions | engineering_positions | medical_positions | science_positions | service_positions | supply_positions | security_positions | assistant_positions | list("AI","Cyborg")), // crew positions EXP_TYPE_SPECIAL = list(), // antags, ERT, etc EXP_TYPE_GHOST = list(), // dead people, observers EXP_TYPE_COMMAND = list(titles = command_head_positions), diff --git a/code/game/objects/items/weapons/cards_ids.dm b/code/game/objects/items/weapons/cards_ids.dm index aa53d533db2..64940e3e2d6 100644 --- a/code/game/objects/items/weapons/cards_ids.dm +++ b/code/game/objects/items/weapons/cards_ids.dm @@ -512,7 +512,7 @@ "Medical" = GLOB.medical_positions, "Science" = GLOB.science_positions, "Security" = GLOB.security_positions, - "Support" = GLOB.support_positions, + "Service" = GLOB.service_positions, "Supply" = GLOB.supply_positions, "Command" = GLOB.command_positions, "Custom" = null, diff --git a/code/modules/admin/db_ban/functions.dm b/code/modules/admin/db_ban/functions.dm index ee6b850f2c8..7d42d900578 100644 --- a/code/modules/admin/db_ban/functions.dm +++ b/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 += "" for(var/j in GLOB.other_roles) output += "" - 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 += "" for(var/j in list("Syndicate") + GLOB.antag_roles) output += "" diff --git a/code/modules/admin/topic.dm b/code/modules/admin/topic.dm index c9538c22c09..021efa163c2 100644 --- a/code/modules/admin/topic.dm +++ b/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 += "" - //Support (Grey) + //Service (Grey) counter = 0 jobs += "" - jobs += "" - for(var/jobPos in GLOB.support_positions) + jobs += "" + 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 += "" + counter++ + else + jobs += "" + counter++ + + if(counter >= 5) //So things dont get squiiiiished! + jobs += "" + counter = 0 + jobs += "
Support Positions
Service Positions
[replacetext(job.title, " ", " ")][replacetext(job.title, " ", " ")]
" + + //Supply (Brown) + counter = 0 + jobs += "" + jobs += "" + 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, "SSjobs has not been setup!") 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 diff --git a/code/modules/client/client_procs.dm b/code/modules/client/client_procs.dm index bc0de582ba8..d2412ed6d54 100644 --- a/code/modules/client/client_procs.dm +++ b/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 )) diff --git a/code/modules/unit_tests/_unit_tests.dm b/code/modules/unit_tests/_unit_tests.dm index 11434dea319..7f234307d41 100644 --- a/code/modules/unit_tests/_unit_tests.dm +++ b/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" diff --git a/code/modules/unit_tests/jobs/test_job_globals.dm b/code/modules/unit_tests/jobs/test_job_globals.dm new file mode 100644 index 00000000000..7ed56ff9980 --- /dev/null +++ b/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")
Supply Positions