From dafef2aa4dcad7578d00772ae226282b09bf234d Mon Sep 17 00:00:00 2001 From: distributivgesetz Date: Fri, 16 Dec 2022 15:01:50 +0100 Subject: [PATCH] Refactor /ui_state/new_player_state to what it's designed for (#71959) ## About The Pull Request `/datum/ui_state/new_player_state` now no longer lets users with R_ADMIN permission see the UI. Changed `/datum/interview` to return always_state when an admin watches an interview because it is important there. `/datum/latejoin_menu` now uses new_player_state instead of its own solution. ## Why It's Good For The Game `/datum/ui_state/new_player_state` is designed to let lobby players interact with the UI, so it does not make sense for it to make a weird exception for admins too. My theory is that this was an oversight when this was first implemented, since only `/datum/interview` used this ui_state. It doesn't and it shouldn't be like this though. Code clarity. ## Changelog No playerfacing changes. --- code/modules/interview/interview.dm | 2 ++ code/modules/mob/dead/new_player/latejoin_menu.dm | 14 ++++++-------- code/modules/tgui/states/new_player.dm | 7 ++----- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/code/modules/interview/interview.dm b/code/modules/interview/interview.dm index ebabb9d942c..d96997dc015 100644 --- a/code/modules/interview/interview.dm +++ b/code/modules/interview/interview.dm @@ -117,6 +117,8 @@ ui.open() /datum/interview/ui_state(mob/user) + if(check_rights_for(user.client, R_ADMIN)) + return GLOB.always_state return GLOB.new_player_state /datum/interview/ui_act(action, list/params, datum/tgui/ui, datum/ui_state/state) diff --git a/code/modules/mob/dead/new_player/latejoin_menu.dm b/code/modules/mob/dead/new_player/latejoin_menu.dm index 9f4d3dfffc4..6e0634ef169 100644 --- a/code/modules/mob/dead/new_player/latejoin_menu.dm +++ b/code/modules/mob/dead/new_player/latejoin_menu.dm @@ -26,8 +26,10 @@ GLOBAL_DATUM_INIT(latejoin_menu, /datum/latejoin_menu, new) ui = SStgui.try_update_ui(user, src, ui) if(!ui) // In case they reopen the GUI - user.jobs_menu_mounted = FALSE - addtimer(CALLBACK(src, PROC_REF(scream_at_player), user), 5 SECONDS) + // FIXME: this can cause a runtime since user can be a living mob + if(istype(user)) + user.jobs_menu_mounted = FALSE + addtimer(CALLBACK(src, PROC_REF(scream_at_player), user), 5 SECONDS) ui = new(user, src, "JobSelection", "Latejoin Menu") ui.open() @@ -111,9 +113,8 @@ GLOBAL_DATUM_INIT(latejoin_menu, /datum/latejoin_menu, new) return list("departments_static" = departments) -// we can't use GLOB.new_player_state here since it also allows any admin to see the ui, which will cause runtimes -/datum/latejoin_menu/ui_status(mob/user) - return isnewplayer(user) ? UI_INTERACTIVE : UI_CLOSE +/datum/latejoin_menu/ui_state(mob/user) + return GLOB.new_player_state /datum/latejoin_menu/ui_act(action, list/params, datum/tgui/ui, datum/ui_state/state) . = ..() @@ -158,9 +159,6 @@ GLOBAL_DATUM_INIT(latejoin_menu, /datum/latejoin_menu, new) // SAFETY: AttemptLateSpawn has it's own sanity checks. This is perfectly safe. owner.AttemptLateSpawn(params["job"]) - - return TRUE - if("viewpoll") var/datum/poll_question/poll = locate(params["viewpoll"]) in GLOB.polls if(!poll) diff --git a/code/modules/tgui/states/new_player.dm b/code/modules/tgui/states/new_player.dm index cf6f83ed3a1..cd33bcfcfae 100644 --- a/code/modules/tgui/states/new_player.dm +++ b/code/modules/tgui/states/new_player.dm @@ -1,13 +1,10 @@ /** * tgui state: new_player_state * - * Checks that the user is a new_player, or if user is an admin + * Checks that the user is a /mob/dead/new_player */ GLOBAL_DATUM_INIT(new_player_state, /datum/ui_state/new_player_state, new) /datum/ui_state/new_player_state/can_use_topic(src_object, mob/user) - if(isnewplayer(user) || check_rights_for(user.client, R_ADMIN)) - return UI_INTERACTIVE - return UI_CLOSE - + return isnewplayer(user) ? UI_INTERACTIVE : UI_CLOSE