Skip to content

Conversation

@Princess-Cheeseballs
Copy link
Member

@Princess-Cheeseballs Princess-Cheeseballs commented Jun 27, 2025

About the PR

Helps with: #38575
Requires: #38617
Requires: #38660

Project Zero warnings PR basically.

Cleaned up every warning in Stunnable and moved Stun and Knockdown to New Status

Moved SlowedDown to its own system in Movement and updated it to the new status system.

Got rid of a number of miscellaneous warnings in areas I was touching anyways.

Should be compatible with: #36881 (Although I'll have to eat some fat merge conflicts)

Why / Balance

Warnings bad, new status good.

New status good, warnings bad.

Technical details

  • Converted Stunnable Statuses to new status system
  • Moved all the slowdown stuff to a new system called the MovementModStatusSystem
  • Fixed a bug with Stamina Slowdown where it could delete other slowdown modifiers
  • Fixed a bug with paralysis where it wouldn't stun if the entity was knockdown immune, but would knockdown even if the entity was stun immune
  • Cleaned up every other warning I saw in SharedStunSystem
  • CanAddStatusEffect has been moved to public.
  • Changed SleepingComponents a bit to be compatible with stun and not have either ended early
  • TryUpdateStatusEffect now accepts a bool "refresh" which lets you choose if you want to choose the greater time or add time.

Media

Requirements

Breaking changes

TrySlowdown and relevant components have been moved to MovementModStatusSystem in the Movement folder.
TrySlowdown has had its refresh bool moved to the end of the component to match how other status effects work.
TrySlowdown has been renamed to TryModMovement.

Changelog

no cl no fun

@github-actions github-actions bot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. size/M Denotes a PR that changes 100-999 lines. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Jun 27, 2025
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jun 27, 2025
@Princess-Cheeseballs
Copy link
Member Author

Me when I forget to update my master branch x2

@github-actions github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jun 27, 2025
@Princess-Cheeseballs
Copy link
Member Author

What the fuck is causing this test fail goddamn

@Princess-Cheeseballs
Copy link
Member Author

image
wuh oh

@Fildrance Fildrance self-assigned this Jun 27, 2025
@TheShuEd
Copy link
Member

Also need remove all yml references to removed old status effects

@TheShuEd
Copy link
Member

you can use #38620
as reference now

@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jun 27, 2025
@github-actions github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jun 27, 2025
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jul 20, 2025
# Conflicts:
#	Content.Server/Stunnable/Systems/StunOnCollideSystem.cs
#	Content.Shared/Bed/Sleep/SleepingSystem.cs
#	Content.Shared/Movement/Systems/MovementModStatusSystem.cs
#	Content.Shared/Slippery/SlipperySystem.cs
#	Content.Shared/StatusEffectNew/StatusEffectSystem.Relay.cs
#	Content.Shared/Stunnable/SharedStunSystem.cs
#	Resources/Prototypes/Entities/Mobs/Cyborgs/base_borg_chassis.yml
#	Resources/Prototypes/Entities/Mobs/NPCs/elemental.yml
#	Resources/Prototypes/Entities/Mobs/NPCs/silicon.yml
#	Resources/Prototypes/Entities/Mobs/NPCs/simplemob.yml
#	Resources/Prototypes/Entities/Mobs/Species/base.yml
@github-actions github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jul 20, 2025
Copy link
Contributor

@Fildrance Fildrance left a comment

Choose a reason for hiding this comment

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

Kinda okay but i want to talk about that StunnedStatusEffectComponent.Remove flag.
Other then that - i am pretty much ready to approve :3
Testing is a bit hard, i think. The whole thing can be nice with some admin commands? Not in this PR but in following stuff. I had to change timers on falsh and on vomiting to do my testing, and it worked.

@PJBot PJBot added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Jul 20, 2025
Copy link
Contributor

@Fildrance Fildrance left a comment

Choose a reason for hiding this comment

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

Yep.

@PJBot PJBot added the S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. label Jul 20, 2025
@Fildrance Fildrance added S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. and removed S: Awaiting Changes Status: Changes are required before another review can happen S: Undergoing Discussion Status: Currently going through an extended discussion, as per procedure. labels Jul 20, 2025
Copy link
Member

@TheShuEd TheShuEd left a comment

Choose a reason for hiding this comment

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

everything looks good, I didn't notice any errors

@PJBot PJBot removed the S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. label Jul 21, 2025
@ScarKy0
Copy link
Contributor

ScarKy0 commented Jul 21, 2025

PR so good i approved twice (somehow)

@ScarKy0 ScarKy0 merged commit e85bc1b into space-wizards:master Jul 21, 2025
11 checks passed
FairlySadPanda pushed a commit to FairlySadPanda/honk-station-15 that referenced this pull request Jul 25, 2025
Co-authored-by: Princess Cheeseballs <[email protected]>
Co-authored-by: pa.pecherskij <[email protected]>
Princess-Cheeseballs added a commit to Princess-Cheeseballs/space-station-14 that referenced this pull request Aug 1, 2025
Co-authored-by: Princess Cheeseballs <[email protected]>
Co-authored-by: pa.pecherskij <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: Core Tech Area: Underlying core tech for the game and the Github repository. D3: Low Difficulty: Some codebase knowledge required. P3: Standard Priority: Default priority for repository items. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. size/M Denotes a PR that changes 100-999 lines. T: Refactor Type: Refactor of notable amount of codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants