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 locking storage and conversion monitors #8272

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

Mithi83
Copy link
Contributor

@Mithi83 Mithi83 commented Dec 10, 2024

Fix #8179

The bug was probably introduced during the rewrite in 70838d7

I'm still a bit confused how onUseItemOn and onUseWithoutItem should interact. My findings were that onUseItemOn is always called first (even with an empty hand) and only if the event was not consumed (by returning true) onUseWithoutItem would be called. My intuition looking at those function names would have been that onUseItemOn would never be called with an empty hand in the first place.

The problem was, that the event that would potentially lead to the code for locking and unlocking in the onUseWithoutItem function was consumed earlier by the onUseItemOn function. I chose to check for the !InteractionUtil.isInAlternateUseMode(player) condition to bypass those event consumptions.

Another functionality that was broken due to a similar problem, that could only be observed after fixing the locking, was the insert all items behavior when right-clicking empty-handedly. This was fixed by the additional check for else if (!heldItem.isEmpty()) and moving the return into that block so that the empty-handed case would not return true and continue to run into the onUseWithoutItem code.

Another point for the review: I changed the return super.onUseWithoutItem(player, pos); in onUseWithoutItem in AbstractMonitorPart to return super.onUseItemOn(heldItem, player, hand, pos); because it looked like a copy&paste error during the rewrite. If that served a specific purpose feel free to restore it, the fix should work without it.

@shartte shartte merged commit 3555f55 into AppliedEnergistics:main Dec 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conversion Monitors and Storage Monitors cannot be locked currently
2 participants