Skip to content

Commit

Permalink
Fix locking storage and conversion monitors (#8272)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Mithi83 authored Dec 10, 2024
1 parent 96de5f7 commit 3555f55
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/main/java/appeng/parts/reporting/AbstractMonitorPart.java
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public boolean onUseWithoutItem(Player player, Vec3 pos) {

@Override
public boolean onUseItemOn(ItemStack heldItem, Player player, InteractionHand hand, Vec3 pos) {
if (!this.isLocked) {
if (!this.isLocked && !InteractionUtil.isInAlternateUseMode(player)) {
if (isClientSide()) {
return true;
}
Expand All @@ -240,7 +240,7 @@ public boolean onUseItemOn(ItemStack heldItem, Player player, InteractionHand ha
return true;
}

return super.onUseWithoutItem(player, pos);
return super.onUseItemOn(heldItem, player, hand, pos);
}

// update the system...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ public boolean onUseItemOn(ItemStack heldItem, Player player, InteractionHand ha
return false;
}

if (this.isLocked()) {
if (this.isLocked() && !InteractionUtil.isInAlternateUseMode(player)) {
if (InteractionUtil.canWrenchRotate(heldItem)
&& (this.getDisplayed() == null || !AEItemKey.matches(getDisplayed(), heldItem))) {
// wrench it
return super.onUseWithoutItem(player, pos);
} else {
} else if (!heldItem.isEmpty()) {
this.insertItem(player, heldItem);
return true;
}
return true;
} else if (this.getDisplayed() != null && AEItemKey.matches(getDisplayed(), heldItem)) {
this.insertItem(player, heldItem);
return true;
Expand All @@ -97,7 +97,7 @@ public boolean onUseItemOn(ItemStack heldItem, Player player, InteractionHand ha

@Override
public boolean onUseWithoutItem(Player player, Vec3 pos) {
if (this.isLocked()) {
if (this.isLocked() && !InteractionUtil.isInAlternateUseMode(player)) {
if (isClientSide()) {
return true;
}
Expand Down

0 comments on commit 3555f55

Please sign in to comment.