Skip to content
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

Status Effects - Part 3A (v2) - Daze Refactor #5153

Merged
merged 117 commits into from
Jan 3, 2024

Conversation

fira
Copy link
Member

@fira fira commented Dec 7, 2023

About the pull request

Depends on Part 2 (#4842), reopening of #4844

Refactors daze and speech problems handling to back them by new status_effect-s and traits, fixes a few bugs, and gets rid of an unused and horrible bay12 relic horsehead with snowflake speech handling

This just generally makes it more reliable timewise and allows further interactions in the future

🆑
code: Refactored Daze to use new Status backend
fix: Dazed screen effect now applies immediately
fix: Stuttering now starts properly when dazed
del: Removed unused disabilities code
del: Removed an old, goofy and unused decade old horse mask
/:cl:

Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general the code changes on the surface look fine and I've not heard anything from anyone as this has been test merged but I've not personally tested aspects of this in game.

code/modules/clothing/masks/miscellaneous.dm Show resolved Hide resolved
code/modules/mob/living/living_health_procs.dm Outdated Show resolved Hide resolved
code/modules/mob/living/living_health_procs.dm Outdated Show resolved Hide resolved
code/modules/mob/living/living_health_procs.dm Outdated Show resolved Hide resolved
dazed = max(dazed + amount,0)
return
if(!(status_flags & CANDAZE))
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're now using this as a means to set Daze to 0 such as in /datum/chem_property/positive/neuroshielding/process(mob/living/M, potency = 1, delta_time) do we want to always perform nothing if they can't be dazed? As in can we guarantee something that has this flag will never be dazed? If not probably should return only if the amount isn't 0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah i've thought about that, i'm not very happy about it either. Debated changing it but that's just how our old code works and i felt it was overarching to change this behavior when it's also used in GODMODE and such. Everything should go through these handlers so it should be fine...
/tg/ still has this behavior with signaling and trait cancellation aswell
Can change it though then need to change all status and GODMODE to match

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno - up to you. Just wanted to bring attention to it.

@fira
Copy link
Member Author

fira commented Jan 2, 2024

Fixed spacing/spelling/var name, intended to keep S to match upstream tg style but it's no big deal

Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look fine.

@Drulikar Drulikar added this pull request to the merge queue Jan 3, 2024
Merged via the queue into cmss13-devs:master with commit 0417120 Jan 3, 2024
26 checks passed
cm13-github added a commit that referenced this pull request Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Improvement Make the code longer Fix Fix one bug, make ten more Removal snap Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants