-
-
Notifications
You must be signed in to change notification settings - Fork 681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Port] Emote Update #11617
base: master
Are you sure you want to change the base?
[Port] Emote Update #11617
Conversation
This should probably be TM'd in order to make sure nothing fucks up with say code. I tested everything i could, but knowing this is my code, I have doubts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't really see any fundamental issues with the code.
I have only these to offer.
code/modules/mob/living/emote.dm
Outdated
var/type = input("Is this a visible or hearable emote?") as null|anything in list("Visible", "Hearable") | ||
if(type == "Hearable") | ||
emote_type |= EMOTE_AUDIBLE | ||
var/type = input("Is this a visible or hearable emote?") as null|anything in list("Visible", "Hearable", "Both") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the amount of lines here could be cut down by making these inputs TGUI lists or the like.
Ideally we want most used player-facing verbs/objects to send TGUI prompts, not html.
@@ -57,6 +58,9 @@ | |||
mob_type_blacklist_typecache = typecacheof(mob_type_blacklist_typecache) | |||
mob_type_ignore_stat_typecache = typecacheof(mob_type_ignore_stat_typecache) | |||
|
|||
if(!name) | |||
name = key | |||
|
|||
/datum/emote/proc/run_emote(mob/user, params, type_override, intentional = FALSE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can see all of the children of this explicitly call parent except for the custom emote.
I would make the parent require calls to parent, and just exclude /datum/emote/living/custom/run_emote()
with SHOULD_CALL_PARENT(FALSE)
This will prevent future screw ups by first time coders I'm sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emote/living/custom/run_emote also calls the parent proc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then the parent proc should be SHOULD_CALL_PARENT(TRUE)
|
||
///Returns the client runechat visible messages preference according to the message type. | ||
/atom/proc/runechat_prefs_check(mob/target, list/visible_message_flags) | ||
if(!target.client?.prefs.read_player_preference(/datum/preference/toggle/enable_runechat)) | ||
return FALSE | ||
if (!target.client?.prefs.read_player_preference(/datum/preference/toggle/enable_runechat_non_mobs)) | ||
return FALSE | ||
if(LAZYFIND(visible_message_flags, CHATMESSAGE_EMOTE) && !target.client.prefs.read_player_preference(/datum/preference/toggle/see_rc_emotes)) | ||
if((LAZYFIND(visible_message_flags, CHATMESSAGE_EMOTE)) && !target.client.prefs.read_player_preference(/datum/preference/toggle/see_rc_emotes)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's with the double paren?
It may have a operator use, but I don't really see it. Did TG do that too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just my paranoia.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind. The parentheses solve a critical logic flaw. Without them, all emotes would not be visible in runechat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lmao
About The Pull Request
Ports the following TGstation PRs:
*Me
invoked via verb defaults to visible | audible, rather than just audible tgstation/tgstation#83283Added sounds are from Sadboysuss's tgstation/tgstation#82748 PR, as well as freesound.org. Attributions added where able.
male_giggle_1.ogg through male_giggle_3.ogg sourced from: https://freesound.org/people/SnowFightStudios/sounds/643665/ , license of CC0 1.0
Why It's Good For The Game
Proper functionality of emotes is nice to have, being able to hear custom emotes while deaf is also quite critical, as you are.. deaf. Also having a proper window for all the emotes and what they're like is good.
Testing Photographs and Procedure
Screenshots&Videos
Emote Menu
2024-10-16.15-43-56.mp4
2024-10-16.15-43-28.mp4
2024-10-16.15-44-28.mp4
Tested speech in order:
Test.
Test|
;Test
;test|
Testing|testing
testing|testing
2024-10-02.06-37-10.mp4
Changelog
🆑 AnturK, Mooshimi, nikothedude, lebedev, kawoppi, RunKitenzRComing, DrDiasyl, larentoun, Mothblocks, jlsnow301, MrMelbert, Sadboysuss, XeonMations
fix: Fixed custom emotes not showing up when a person is deaf.
qol: Added a TGUI emote menu that shows every emote to the IC tab.
refactor: Refactored some emote code.
qol: You can now see people doing emotes that require sight without hearing.
qol: You can now hear people doing emotes that require hearing without sight.
add: People wearing a security mask now have a special deathgasp.
add: Added *smirk emote.
add: Added *sweat, which is the same as *sweatdrop.
/:cl: