Fix getOwner() taking snapshot of chest NBT#381
Fix getOwner() taking snapshot of chest NBT#381personalized-advertising wants to merge 4 commits intoJikoo:masterfrom
Conversation
| import org.bukkit.inventory.InventoryHolder; | ||
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| import static com.lishid.openinv.internal.AnySilentContainerBase.getHolder; |
There was a problem hiding this comment.
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.
| 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); | ||
| } |
There was a problem hiding this comment.
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.
|
Superseded by #382. Thank you for taking the time to make an initial attempt - I had completely forgotten there was reflection in use for the BlockState side for compatibility that is no longer necessary. |
2 participants
Fixes #380