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 lag spike from stinger grenades #33885

Closed
wants to merge 1 commit into from
Closed

Fix lag spike from stinger grenades #33885

wants to merge 1 commit into from

Conversation

iaada
Copy link
Contributor

@iaada iaada commented Dec 16, 2024

About the PR

ClusterGrenadeComponent - specifically the type that spawns bullets - currently causes a lag spike when it goes off. This is because every bullet tries and fails to reference the already deleted grenade. This PR stops the bullets from looking for the grenade.

Why / Balance

Lag is bad.

Technical details

Changed ShootProjectile in SharedGunSystem to make the parameter for gun source nullable.

Media

Causes ugly admin logging, but previously there was no admin logging at all.
image

Requirements

Breaking changes

Changelog

🆑 aada

  • fix: Stinger grenades no longer cause lag spikes.

@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 Dec 16, 2024
@beck-thompson
Copy link
Contributor

fixes: #32642?

Also amazing first contribution

@beck-thompson beck-thompson added T: Bugfix Type: Bugs and/or bugfixes P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. D2: Medium Difficulty: A good amount of codebase knowledge required. A: Combat Area: Combat features and changes, balancing, feel A: Core Tech Area: Underlying core tech for the game and the Github repository. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Dec 16, 2024
@Plykiya
Copy link
Contributor

Plykiya commented Dec 16, 2024 •
edited
Loading

I took a swing at the problem in my complete cluster grenade refactor and explain it similarly in my latest post, and you can get around the issue by just NEVER setting the ProjectileComponent's gunUid at all which I did at some point, but it also just feels wrong using gun code to shoot a projectile without providing the gunUid because it causes bad admin logs as you've already explained so I didn't go that route

@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Dec 16, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

@iaada
Copy link
Contributor Author

iaada commented Dec 16, 2024

I took a swing at the problem in my complete cluster grenade refactor and explain it similarly in my latest post, and you can get around the issue by just NEVER setting the ProjectileComponent's gunUid at all which I did at some point, but it also just feels wrong using gun code to shoot a projectile without providing the gunUid because it causes bad admin logs as you've already explained so I didn't go that route

I totally agree that my PR is sloppy. I was just looking at a simple fix, but you're is definitely much better done.

@iaada iaada closed this Dec 16, 2024
@Plykiya
Copy link
Contributor

Plykiya commented Dec 16, 2024

I took a swing at the problem in my complete cluster grenade refactor and explain it similarly in my latest post, and you can get around the issue by just NEVER setting the ProjectileComponent's gunUid at all which I did at some point, but it also just feels wrong using gun code to shoot a projectile without providing the gunUid because it causes bad admin logs as you've already explained so I didn't go that route

I totally agree that my PR is sloppy. I was just looking at a simple fix, but you're is definitely much better done.

I mean I still haven't fixed the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

Assignees
No one assigned
Labels
A: Combat Area: Combat features and changes, balancing, feel A: Core Tech Area: Underlying core tech for the game and the Github repository. D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted S: Needs Review Status: Requires additional reviews before being fully accepted size/S Denotes a PR that changes 10-99 lines. T: Bugfix Type: Bugs and/or bugfixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iaada @beck-thompson @Plykiya