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

Fixed bola effect stacking #34723

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

Conversation

impubbi
Copy link

@impubbi impubbi commented Jan 28, 2025

About the PR

Bolas had a bug that allowed them to stack and apply to the same target (#34694)
I changed the shared ensnareable system to only allow one bola to be applied as mentioned in (#34694)

Fixes #34694

Why / Balance

Removes the ability to movelock and makes it less annoying to remove many bolas.

Technical details

Changed the checks in the SharedEnsnareableSystem's TryEnsnare method to only ensnare if they aren't already ensnared. (Try saying that 3 times fast lol)

Media

image

Requirements

Breaking changes

Changelog
🆑

  • fix: Bolas will now only be applied once.

@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. labels Jan 28, 2025
@impubbi impubbi changed the title Bola fix 34694 Fixed bola effect stacking Jan 29, 2025
@lzk228
Copy link
Contributor

lzk228 commented Jan 29, 2025

you can link your pr to the issue so it could be autoclosed along with pr merging
https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue

@lzk228 lzk228 added T: Bugfix Type: Bugs and/or bugfixes P3: Standard Priority: Default priority for repository items. D3: Low Difficulty: Some codebase knowledge required. A: General Interactions Area: General in-game interactions that don't relate to another area. S: Requires Content PR Status: Requires a change to SS14, for which there is no open PR currently. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Jan 29, 2025
@impubbi
Copy link
Author

impubbi commented Jan 29, 2025

Thanks for the info!

@slarticodefast
Copy link
Member

slarticodefast commented Jan 29, 2025

Now that I look a the code in more detail it does not seem to be a bug to be able to get ensnared more than once, and more like a design choice. Before it was limited to one bola for each leg (which will become more relevant once we have surgery). According to the linked issue it was possible to get ensnared 5 or 6 times, so this check is failing somewhere
grafik

Made walk/sprint speed AutoNetworkFields to fix some networking issues when removing bolas while walking.
@impubbi
Copy link
Author

impubbi commented Jan 30, 2025

Thanks for the input!
It's now limited to the number of legs the target has as originally intended
This implementation had some weird network problems though:
Content Client_ZaVXOJl4S6

So I also made the sprint and walk speeds AutoNetworkedFields in the ensnareable component:
Content Client_lprFT3dMbm

Results:
Content Client_yA6whuenEu
Content Client_rAWNlOlVbk
Content Client_uf3aY5v1d6

@K-Dynamic
Copy link
Contributor

K-Dynamic commented Jan 30, 2025

Huge success

I still think the cumulative speed reduction from multiple bolas needs to be looked into at some point, but that's out of scope of this PR

Also bolas need to work on mobs like dogs or spiders but that's also out of scope :trollface:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: General Interactions Area: General in-game interactions that don't relate to another area. D3: Low Difficulty: Some codebase knowledge required. P3: Standard Priority: Default priority for repository items. S: Requires Content PR Status: Requires a change to SS14, for which there is no open PR currently. size/S Denotes a PR that changes 10-99 lines. T: Bugfix Type: Bugs and/or bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bolas can apply multiple times on the same mob
4 participants