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.

Sign up for GitHub

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

Jump to bottom

Fix two zombie bugs #34472

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix two zombie bugs #34472

wants to merge 2 commits into from
+9 −3

Conversation

IProduceWidgets
Copy link
Contributor

@IProduceWidgets IProduceWidgets commented Jan 16, 2025 •
edited
Loading

About the PR

I identified two bugs with zombies and fixed them.

  • Vox health regen from Give vox health regen for poison below 20 damage #33722 was the same strength as the poisoning effect from the PendingZombieComponent meaning Vox would never naturally succumb to the virus. This is bad because it prevents zombies from snowballing, but also because it means Vox could be II forever with no issues which was not intended. Solution: make the poison effect from zombie virus slightly stronger to compensate.
  • All zombies were erroneously initialized with the grace period of Initial Infected. This made the zombie virus really really slow by letting anyone bit by a zombie wait 15 minutes before they start taking damage. Solution: Non-Initial Infected zombies no longer get a grace period.

Why / Balance

I squash bug.
This will make zombies virus scarier and faster.
It only matters if you survive a fight with a zombie.

Technical details

Media

Requirements

Breaking changes

Changelog

🆑

  • fix: Vox are no longer immune to the zombie virus under certain circumstances
  • fix: The zombie virus no longer provides non-Initial Infected an undue grace period before poisoning them.

@github-actions github-actions bot added size/S Denotes a PR that changes 10-99 lines. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. S: Needs Review Status: Requires additional reviews before being fully accepted labels Jan 16, 2025
@pheenty
Copy link
Contributor

pheenty commented Jan 16, 2025

Buffing it two times is definitely not "make it slightly stronger", I don't even think Vox being kinda immune to it is an issue, yet the second fix is nice

slarticodefast
slarticodefast reviewed Jan 17, 2025
View reviewed changes
Content.Server/Zombies/ZombieSystem.cs
if (HasComp<ZombieComponent>(uid) || HasComp<ZombieImmuneComponent>(uid))
return;

EnsureComp<PendingZombieComponent>(uid, out PendingZombieComponent pendingComp);
Copy link
Member

@slarticodefast slarticodefast Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, adding a component in the initialization of another component is a little messy and afaik there are some integration tests that can fail due to this for certain prototypes.
And as far as I can see it should already be added where ever the IncurableZombieComponent is added.

Copy link
Contributor Author

@IProduceWidgets IProduceWidgets Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, but that causes a race condition between the two. Its what caused the bug.

Well, partially, the original author also seemed to think the map init wouldn't get hit on new components added after the map was initialized, but that's not how that works.

Copy link
Contributor Author

@IProduceWidgets IProduceWidgets Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I can admit that they should probably just be the same component, but tbh all of zombie system and its components and zombiesystem.transform needs a rewrite. Its a mess in there.
image

@Aeshus Aeshus added P3: Standard Priority: Default priority for repository items. D3: Low Difficulty: Some codebase knowledge required. T: Balance Change Type: Balance changes through direct value changes, or changes to mechanics that affect it A: Character/Species Area: Player characters and species features and content. A: Medical Area: Medical department, including Chemistry A: Roundflow/Antag Area: Roundflow - "What happens in the game", including antagonist roles and their capabilities and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slarticodefast slarticodefast slarticodefast left review comments

@DrSmugleaf DrSmugleaf Awaiting requested review from DrSmugleaf DrSmugleaf is a code owner

Assignees
No one assigned
Labels
A: Character/Species Area: Player characters and species features and content. A: Medical Area: Medical department, including Chemistry A: Roundflow/Antag Area: Roundflow - "What happens in the game", including antagonist roles and their capabilities D3: Low Difficulty: Some codebase knowledge required. P3: Standard Priority: Default priority for repository items. S: Needs Review Status: Requires additional reviews before being fully accepted size/S Denotes a PR that changes 10-99 lines. T: Balance Change Type: Balance changes through direct value changes, or changes to mechanics that affect it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@IProduceWidgets @pheenty @slarticodefast @Aeshus