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

Mobility Atomization 3: Unconsciousness #9570

Merged
merged 11 commits into from
Oct 17, 2023

Conversation

Tsar-Salat
Copy link
Contributor

@Tsar-Salat Tsar-Salat commented Aug 1, 2023

Ports:

About The Pull Request

closes #9829

Adds a knockedout trait. Cuts a significant amount of code out of update_mobility.

Removed a lot of `update_stat' calls that called solely for unconciousness.

Fixes bug where monochromatic client color would persist much longer than it should, after being woken up from an unconcious state

Fixes bug where a sleeping check in regenerative coma was never met. Voluntarily sleeping is supposed to heal you less than being in a unconscious in general(i.e. "comatose"), but the unconsciousness check was before the sleeping check, making this code never run.

Why It's Good For The Game

reduces reliance on big update procs, which are clunky and generate more edge-cases than they should. Smaller updates like this should make the code much more responsive as well.

A lot of procs called update_stat solely for unconciousness changes, their refactoring improves performance a lot.

Testing Photographs and Procedure

Screenshots&Videos

There are several things we need to test. We ne

Forced Unconciousness/Coma from oxyloss

dreamseeker_DPNuYCTbIZ.mp4

Sleep verb

PjYhvyB3W8.mp4

Crit unconciousness leading to death

CYfMjejsel.mp4

Tested crit unconciousness through oxyloss being applied and removed.(Could not move at 180 oxyloss)

yneYtOA2tV.mp4

Changelog

🆑 RKz, Rohesie
code: unconsciousness is now a trait, cleaned up its usage in mobility and stat, which should make it perform a lot quicker and generate less edge-cases
fix: being woken up from unconsciousness should always clear trauma-induced monochromatic vision
fix: moves a sleeping check in regenerative coma a few lines ahead. It's intended use in code that voluntarily sleeping provides less healing benefit than entering a coma through trauma/medicine.
balance: Which... now technically makes it a balance change. It was always intended, but using the sleep verb will now heal you less than involuntary unconciousness, in terms of the regen rate of the regenerative coma
/:cl:

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Tsar-Salat Tsar-Salat marked this pull request as ready for review August 16, 2023 18:00
@Tsar-Salat Tsar-Salat closed this Aug 18, 2023
Copy link
Member

@PowerfulBacon PowerfulBacon left a comment

Choose a reason for hiding this comment

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

Checking of if a mob is unconcious is insanely inconsistent. We should either always check if (stat), or we should always check HAS_TRAIT(src, TRAIT_KNOCKEDOUT). I would prefer if we always checked the stat var rather than if they have the trait.

code/__DEFINES/traits.dm Outdated Show resolved Hide resolved
@Tsar-Salat
Copy link
Contributor Author

Tsar-Salat commented Sep 16, 2023

I would prefer if we always checked the stat var rather than if they have the trait.

Im trying to do the opposite of that.

The goal of these PRs is to remove the different status changes (stat, mobility_flags, eventually health in later ports) away from processing loops like life() and towards updating properly when they change.

Superfluous checks are cut out for more efficient ones.

image

Checks should be much more specific and granular. There are a bunch of checks that we are either checking 3+ things when we can group them under a trait, or we are calling something for a singular change and running very lengthy procs for no reason.

As for why we are specifically focusing on traits, mobility_flags are intended to represent a mob's potential for things, while traits should be the preferred method to adding temporary changes, as we can track and test the changes very easily.

Looks like Jupyterkat already added the support for attaching signals to traits anyway in #4825, which was part of this system on tg

tl;dr

Mobilty as traits have signals, which we can append behavior to and track much easier than stat checks or update_X.

  • Being knocked out
  • being immobilized from buckled
  • being immobilized from stuns
  • being unable to use hands in handcuffs
  • being unable to use UI & hands in straightjacket
  • etc...
    -all easy to track and debug.

Copy link
Member

@itsmeow itsmeow left a comment

Choose a reason for hiding this comment

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

Looks good to me - it functionally cleans up a lot of unconsciousness shitcode.

Yes, the distinction between the trait and the stat are annoying - but stat is a core part of the mob health system - it's extra but legacy. Unconsciousness is also distinct from Knocked Out - you can be unconscious as the result of sleep, being knocked out, or lacking oxygen.

Many sources do not care if you are unconscious due to being knocked out, or because of sleep, etc. - all that matters is if you are unconscious. So checking sleep, knockedout, etc instead of just checking stat would be contrary to the entire purpose here.

@itsmeow
Copy link
Member

itsmeow commented Sep 27, 2023

Sleeping and oxyloss do directly add the TRAIT_KNOCKED_OUT, making that and stat redundant, but stat is also just a core part of the game - this PR is simply making it react via signals rather than via update_stat, which is a huge mess of a proc and calling it anytime sleeping status etc changes is a huge pain in the ass. So the PR accomplishes its purpose and nothing more, rewriting uses of stat is pointless because IsUnconcious() was that original proc that did this. Most things just check if(stat) rather than if(IsUnconcious() || IsDead())...

Copy link
Member

@PowerfulBacon PowerfulBacon left a comment

Choose a reason for hiding this comment

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

I get the original intention with using traits now and I do like it, however we need to make it clear why we are using traits and be consistent with checking stat over has_trait so that coders in the future don't use the wrong thing.

code/modules/mob/living/silicon/robot/robot.dm Outdated Show resolved Hide resolved
code/modules/mob/living/simple_animal/simple_animal.dm Outdated Show resolved Hide resolved
code/__DEFINES/traits.dm Outdated Show resolved Hide resolved
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@PowerfulBacon PowerfulBacon added this pull request to the merge queue Oct 17, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 17, 2023
@Crossedfall Crossedfall added this pull request to the merge queue Oct 17, 2023
Merged via the queue into BeeStation:master with commit 7becccf Oct 17, 2023
8 checks passed
@Tsar-Salat Tsar-Salat deleted the mobility-atomization-3 branch October 18, 2023 01:51
Tyranicranger4 added a commit to Tyranicranger4/BeeStation-Hornet that referenced this pull request Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Monochromatic client color persists after waking up from being unconscious
4 participants