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

fix #31963: Stopped climbing movent while in containers #32033

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

Conversation

benev0
Copy link
Contributor

@benev0 benev0 commented Sep 10, 2024

About the PR

This resolves a bug that allows players to shoot while inside crates/lockers

Why / Balance

resolves #31963

Technical details

Block final transform for climbing while entity (player) is in a container.
Final climb may succeed still while in container, but the player remains in place.

Media

Requirements

Breaking changes

Changelog

🆑

  • fix: Stopped climbing movent while in containers

@lzk228
Copy link
Contributor

lzk228 commented Sep 10, 2024

thank you very much, also i suggest you to read this

@benev0
Copy link
Contributor Author

benev0 commented Sep 10, 2024

thanks, I was unaware that could be done automatically

@metalgearsloth metalgearsloth added the Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. label Sep 10, 2024
@benev0
Copy link
Contributor Author

benev0 commented Sep 10, 2024

After interacting with this problem it appears that there is a problem with the methods registered to events, and will require further attention.

It appears that event registers to stop the doAfter for Buckling and Storing

@benev0
Copy link
Contributor Author

benev0 commented Sep 10, 2024

issues from #31963 should be correctly addressed: awaiting further review

  • entities which are in storage cannot initiate climbing
  • entities which enter storage have climb DoAfters canceled
  • entities which become buckled have climb DoAfters canceled

@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. and removed Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. labels Sep 11, 2024
Content.Shared/Climbing/Systems/ClimbSystem.cs Outdated Show resolved Hide resolved
Comment on lines 262 to 265

if (_containers.IsEntityInContainer(uid))
return;

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be done up the callstack in canclimb or the likes not after climbing is happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ClimbingComponent field CanClimb is effectively unused as it is never altered from a true state. The code alteration could be made by subscribing to container remove as well, but if CanClimb is to be used I would recommend scoping this up from a bug fix to a ClimbSystem Rework.

I have moved the inventory check into OnDoAfter where it will stop the call of climb. I have also observed that it is technicality unnecessary but will result in visible rollback if not checked before climbing is initiated.

@metalgearsloth metalgearsloth added Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. and removed Status: Needs Review This PR requires new reviews before it can be merged. labels Sep 19, 2024
@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. and removed Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. labels Sep 21, 2024
@benev0
Copy link
Contributor Author

benev0 commented Sep 21, 2024

glorious, test failed; I shall look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review This PR requires new reviews before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

climbing while in container (dangerous bug)
3 participants