Skip to content

Fix getOwner() taking snapshot of chest NBT #381

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.

Sign up for GitHub

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

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

Fix getOwner() taking snapshot of chest NBT #381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3bc1743
get holder without snapshot
personalized-advertising Mar 15, 2026
0418e86
use new method
personalized-advertising Mar 15, 2026
4b799e1
oops
personalized-advertising Mar 15, 2026
ff3750e
Update ContainerListener.java
personalized-advertising Mar 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions common/src/main/java/com/lishid/openinv/internal/AnySilentContainerBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@
import org.bukkit.block.data.type.Chest;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.bukkit.inventory.Inventory;
import org.bukkit.inventory.InventoryHolder;

import java.lang.reflect.Method;

public abstract class AnySilentContainerBase implements IAnySilentContainer {

private static final @Nullable Method BLOCK_GET_STATE_BOOLEAN;
private static final @Nullable Method INVENTORY_GET_HOLDER_BOOLEAN;

static {
@Nullable Method getState;
Expand All @@ -26,6 +29,15 @@ public abstract class AnySilentContainerBase implements IAnySilentContainer {
getState = null;
}
BLOCK_GET_STATE_BOOLEAN = getState;

@Nullable Method getHolder;
try {
//noinspection JavaReflectionMemberAccess
getHolder = Inventory.class.getMethod("getHolder", boolean.class);
} catch (NoSuchMethodException e) {
getHolder = null;
}
INVENTORY_GET_HOLDER_BOOLEAN = getHolder;
}

private static BlockState getBlockState(Block block) {
Expand All @@ -40,6 +52,18 @@ private static BlockState getBlockState(Block block) {
return block.getState();
}

public static @Nullable InventoryHolder getHolder(@NotNull Inventory inventory) {
if (INVENTORY_GET_HOLDER_BOOLEAN != null) {
try {
return (InventoryHolder) INVENTORY_GET_HOLDER_BOOLEAN.invoke(inventory, false);
} catch (ReflectiveOperationException ignored) {
// fallback
}
}

return inventory.getHolder();
}

@Override
public boolean isAnyContainerNeeded(@NotNull Block block) {
BlockState blockState = getBlockState(block);
Expand Down
5 changes: 4 additions & 1 deletion plugin/src/main/java/com/lishid/openinv/listener/ContainerListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.errorprone.annotations.Keep;
import com.lishid.openinv.internal.ViewOnly;
import com.lishid.openinv.util.InternalAccessor;
import com.lishid.openinv.util.InventoryAccess;
import com.lishid.openinv.util.Permissions;
import com.lishid.openinv.util.lang.LanguageManager;
import com.lishid.openinv.util.setting.PlayerToggles;
Expand All @@ -39,6 +40,8 @@
import org.bukkit.inventory.InventoryHolder;
import org.jetbrains.annotations.NotNull;

import static com.lishid.openinv.internal.AnySilentContainerBase.getHolder;
Copy link
Owner

@Jikoo Jikoo Mar 15, 2026

Choose a reason for hiding this comment

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

This shouldn't be imported directly - the only part of the plugin that really should be actually directly interfacing with the version-dependent internals is the InternalAccessor. Particularly on Spigot, there's a risk that static initialization of an internal class will cause a LinkageError. It might not currently, but it's a future headache that can be avoided. Spigot has its own remapped version of all of the "common" classes.
I should probably restructure the project a little so the internals aren't accessible in the main plugin here, but an entire new module just for that felt like a bit of overkill.

If you would, please add the signature to IAnySilentContainer and then implement in AnySilentContainerBase. It should then be able to be called just like the companion method is on old L67.


/**
* A listener managing AnyContainer, SilentContainer, and more.
*/
Expand Down Expand Up @@ -100,7 +103,7 @@ private void onInventoryClose(@NotNull final InventoryCloseEvent event) {
return;
}

InventoryHolder holder = event.getInventory().getHolder();
InventoryHolder holder = getHolder(event.getInventory());
if (PlayerToggles.silent().is(player)
&& holder != null
&& this.accessor.getAnySilentContainer().isAnySilentContainer(holder)) {
Expand Down