Skip to content

Fix getOwner() taking snapshot of chest NBT#381

Open
personalized-advertising wants to merge 4 commits intoJikoo:masterfrom
personalized-advertising:fix-holder-snapshot
Open

Fix getOwner() taking snapshot of chest NBT#381
personalized-advertising wants to merge 4 commits intoJikoo:masterfrom
personalized-advertising:fix-holder-snapshot

Conversation

@personalized-advertising

Fixes #380

import org.bukkit.inventory.InventoryHolder;
import org.jetbrains.annotations.NotNull;

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

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.

Comment on lines -103 to 111
InventoryHolder holder = event.getInventory().getHolder();
InventoryHolder holder = getHolder(event.getInventory());
if (PlayerToggles.silent().is(player)
&& holder != null
&& this.accessor.getAnySilentContainer().isAnySilentContainer(holder)) {
this.accessor.getAnySilentContainer().deactivateContainer(player);
}
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.

I think we should instead be considering adding IAnySilentContainer#isAnySilentContainer(Inventory) - this would prevent abuse by regular players on Spigot by skipping anyone not currently in SilentContainer mode. This block would become

if (PlayerToggles.silent().is(player)
    && this.accessor.getAnySilentContainer().isAnySilentContainer(event.getInventory())) {
  this.accessor.getAnySilentContainer().deactivateContainer(player);
}

implementation will still be in the same place, roughly the same way.

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.

Large nbt in chest lagging out server due to use of snapshot get holder

2 participants