Adds sanity checking to prefs checks, fixing a bug that can cause emotes to stop displaying intermittently (#76946)

## About The Pull Request

This PR adds a bunch of sanity checking to the `prefs.chat_toggles` var
accesses.

Prefs can be null when client is creating/deleting and this causes a
runtime where you might not be able to use emotes.


![image](https://github.com/tgstation/tgstation/assets/13398309/d89649d4-19c7-439f-b080-332709196505)

EDIT: as per suggestion I stopped masking the issue and added stack
traces instead. And made them into helper procs to avoid code
duplication.

For some of these procs it is important that we are able to continue
running to get to the end. **A ghost having null prefs should not stop
everyone else from hearing/seeing the emote.**

So now they will put the stack trace and keep on trucking if that issue
occurs. This is the main 'point' of the PR, which has seemingly gotten
bogged down in the creation of the two helper procs--apologies.

See https://github.com/tgstation/tgstation/pull/70404 for essentially
the same issue but applied to chat messages. I have gone back and
replaced the duplicated code from that to use the new helper procs
instead.

## Why It's Good For The Game

Anything chat related is a bad place to runtime. Some messages can take
a while to type out and to have it not display is frustrating at worst,
possibly detrimental if you have to communicate something urgently.

## Changelog

🆑
fix: fixes a bug that can cause emotes to stop working if a client is
being created or deleted
/🆑
This commit is contained in:
Bloop
2023-08-14 22:30:35 +02:00
committed by GitHub
parent 8089a6e7e1
commit ed8445a312
14 changed files with 66 additions and 39 deletions
+1 -1
View File
@@ -1037,7 +1037,7 @@ GLOBAL_LIST_INIT(blacklisted_builds, list(
void.UpdateGreed(actualview[1],actualview[2])
/client/proc/AnnouncePR(announcement)
if(prefs && prefs.chat_toggles & CHAT_PULLR)
if(get_chat_toggles(src) & CHAT_PULLR)
to_chat(src, announcement)
///Redirect proc that makes it easier to call the unlock achievement proc. Achievement type is the typepath to the award, user is the mob getting the award, and value is an optional variable used for leaderboard value increments
+43
View File
@@ -439,6 +439,49 @@ GLOBAL_LIST_EMPTY(preferences_datums)
if(GetQuirkBalance() < 0)
all_quirks = list()
/**
* Safely read a given preference datum from a given client.
*
* Reads the given preference datum from the given client, and guards against null client and null prefs.
* The client object is fickle and can go null at times, so use this instead of read_preference() if you
* want to ensure no runtimes.
*
* returns client.prefs.read_preference(prefs_to_read) or FALSE if something went wrong.
*
* Arguments:
* * client/prefs_holder - the client to read the pref from
* * datum/preference/pref_to_read - the type of preference datum to read.
*/
/proc/safe_read_pref(client/prefs_holder, datum/preference/pref_to_read)
if(!prefs_holder)
return FALSE
if(prefs_holder && !prefs_holder?.prefs)
stack_trace("[prefs_holder?.mob] ([prefs_holder?.ckey]) had null prefs, which shouldn't be possible!")
return FALSE
return prefs_holder?.prefs.read_preference(pref_to_read)
/**
* Get the given client's chat toggle prefs.
*
* Getter function for prefs.chat_toggles which guards against null client and null prefs.
* The client object is fickle and can go null at times, so use this instead of directly accessing the var
* if you want to ensure no runtimes.
*
* returns client.prefs.chat_toggles or FALSE if something went wrong.
*
* Arguments:
* * client/prefs_holder - the client to get the chat_toggles pref from.
*/
/proc/get_chat_toggles(client/prefs_holder)
if(!prefs_holder)
return FALSE
if(prefs_holder && !prefs_holder?.prefs)
stack_trace("[prefs_holder?.mob] ([prefs_holder?.ckey]) had null prefs, which shouldn't be possible!")
return FALSE
return prefs_holder?.prefs.chat_toggles
/// Sanitizes the preferences, applies the randomization prefs, and then applies the preference to the human mob.
/datum/preferences/proc/safe_transfer_prefs_to(mob/living/carbon/human/character, icon_updates = TRUE, is_antag = FALSE)
apply_character_randomization_prefs(is_antag)
+2 -2
View File
@@ -68,7 +68,7 @@ GLOBAL_VAR_INIT(normal_ooc_colour, "#002eb8")
message_admins("[key_name_admin(src)] has attempted to advertise in OOC: [msg]")
return
if(!(prefs.chat_toggles & CHAT_OOC))
if(!(get_chat_toggles(src) & CHAT_OOC))
to_chat(src, span_danger("You have OOC muted."))
return
@@ -85,7 +85,7 @@ GLOBAL_VAR_INIT(normal_ooc_colour, "#002eb8")
for(var/client/receiver as anything in GLOB.clients)
if(!receiver.prefs) // Client being created or deleted. Despite all, this can be null.
continue
if(!(receiver.prefs.chat_toggles & CHAT_OOC))
if(!(get_chat_toggles(receiver) & CHAT_OOC))
continue
if(holder?.fakekey in receiver.prefs.ignoring)
continue