-
-
Notifications
You must be signed in to change notification settings - Fork 60
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 Weapon reload freeze #418
base: next
Are you sure you want to change the base?
Conversation
WalkthroughWalkthroughThe recent updates to the weapon handling code primarily address enhancements in reloading logic and state transitions. These changes include adding a transition to Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant WeaponReloadAspect
participant WeaponRenderer
participant WeaponState
Player ->> WeaponReloadAspect: Initiate reloadMainHeldItem
WeaponReloadAspect ->> WeaponState: Check attachment presence
WeaponState -->> WeaponReloadAspect: Attachment info
alt Attachment Present
WeaponReloadAspect ->> WeaponState: Transition to PAUSED
end
WeaponReloadAspect -->> Player: Complete reloadMainHeldItem
Player ->> WeaponReloadAspect: Initiate clientCompoundReload
WeaponReloadAspect ->> WeaponState: Check attachment presence
WeaponState -->> WeaponReloadAspect: Attachment info
alt Attachment Present
WeaponReloadAspect ->> WeaponState: Transition to PAUSED
end
WeaponReloadAspect -->> Player: Complete clientCompoundReload
Player ->> WeaponRenderer: Render weapon
alt First Person View
WeaponRenderer ->> WeaponRenderer: Assign state RECOILED/SHOOTING
else Third Person View
WeaponRenderer ->> WeaponRenderer: Assign state RECOILED/SHOOTING
end
WeaponRenderer -->> Player: Render state updated
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
In my opinion the reload should proceed "as usual", as in it shouldn't reset your ammo count back to 0 but to full and whatnot |
It did not reset Ammo to 0, and it should not be reset to full either |
Shouldn't get rollbacked either, should just not load this be unloaded |
I tried to unload it, but the animation between reload and unload didn't connect well |
You shouldn't animate unload again, when the stall point is reached the magazine is already unloaded, you just need to get it back to ready |
If not rolled back, it will cause the magazine to disappear |
bbeb5e6
to
3734a6e
Compare
Will go back to this after the thing |
16e2cd7
to
0d7c54e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
src/main/java/com/paneedah/weaponlib/WeaponReloadAspect.java (1)
276-283
: Add a comment explaining the new state transition.The new state transition to
WeaponState.PAUSED
when the magazine is not found during the reload process needs an explanation to clarify its purpose and impact.// Transition to PAUSED state if no magazine is found during COMPOUND_RELOAD_FINISH
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/java/com/paneedah/weaponlib/WeaponReloadAspect.java (3 hunks)
- src/main/java/com/paneedah/weaponlib/WeaponRenderer.java (4 hunks)
Additional comments not posted (2)
src/main/java/com/paneedah/weaponlib/WeaponRenderer.java (2)
2357-2357
: Verify correctness of state transition logic.Ensure that the transition to
RenderableState.SHOOTING
is intended and does not introduce any unintended side effects or regressions in the rendering logic.Verification successful
The transition to
RenderableState.SHOOTING
is correctly handled and does not introduce unintended side effects.The rendering logic appropriately checks for the
SHOOTING
state in conditional statements, ensuring the correct behavior.
- The
currentState
is checked againstRenderableState.SHOOTING
in thestateManager.setState
method to determine rendering behavior.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the state transition logic in the `getFirstPersonStateDescriptor` method. # Test: Search for all state transitions in the `getFirstPersonStateDescriptor` method. Expect: Correct state assignments. rg --type java 'currentState = RenderableState\.\w+' src/main/java/com/paneedah/weaponlib/WeaponRenderer.javaLength of output: 3068
Script:
#!/bin/bash # Description: Verify handling of RenderableState.SHOOTING in the rendering logic. # Test: Search for usages of `currentState` to inspect how the `SHOOTING` state is handled. rg --type java '\bcurrentState\b' src/main/java/com/paneedah/weaponlib/WeaponRenderer.javaLength of output: 4188
2330-2330
: Verify correctness of state transition logic.Ensure that the transition to
RenderableState.RECOILED
is intended and does not introduce any unintended side effects or regressions in the rendering logic.Verification successful
Verification of state transition logic is successful.
The transition to
RenderableState.RECOILED
at line 2330 is consistent with the surrounding logic and does not introduce any unintended side effects or regressions in the rendering logic.
src/main/java/com/paneedah/weaponlib/WeaponRenderer.java
:
- Line 2330:
currentState = RenderableState.RECOILED;
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the state transition logic in the `getFirstPersonStateDescriptor` method. # Test: Search for all state transitions in the `getFirstPersonStateDescriptor` method. Expect: Correct state assignments. rg --type java 'currentState = RenderableState\.\w+' src/main/java/com/paneedah/weaponlib/WeaponRenderer.javaLength of output: 3068
Script:
#!/bin/bash # Extract the full implementation of the `getFirstPersonStateDescriptor` method from `WeaponRenderer.java` ast-grep --lang java --pattern 'public $_ getFirstPersonStateDescriptor($_) { $$$ }' src/main/java/com/paneedah/weaponlib/WeaponRenderer.javaLength of output: 143
Script:
#!/bin/bash # Search for the method definition of `getFirstPersonStateDescriptor` in `WeaponRenderer.java` rg --type java 'getFirstPersonStateDescriptor' src/main/java/com/paneedah/weaponlib/WeaponRenderer.javaLength of output: 296
Script:
#!/bin/bash # Extract the full implementation of the `getFirstPersonStateDescriptor` method from `WeaponRenderer.java` using the correct method signature ast-grep --lang java --pattern 'protected StateDescriptor getFirstPersonStateDescriptor(EntityLivingBase player, ItemStack itemStack) { $$$ }' src/main/java/com/paneedah/weaponlib/WeaponRenderer.javaLength of output: 20252
📝 Description
Fix Unable to find magazine while reload, resulting in weapon freezing
🎯 Goals
Weapon reload freeze
❌ Non Goals
Code abbreviation is not the goal
🚦 Testing
When reload cannot find the magazine, it will roll back the status
In the creation mode, perform weapon reload, instant sneak, the weapon will not freeze
In survival mode, the weapon reload and instantly discards the only available magazine, the weapon will not freeze
⏮️ Backwards Compatibility
Fully compatible
🖼️ Screenshots/Recordings
https://discord.com/channels/801852948854079489/1078839351468892241/1219634858368696412
📖 Added to documentation?
😄 [optional] What gif best describes this PR or how it makes you feel?
I discovered this issue in June last year