Add new Lidded API, deprecating existing bukkit Lidded API. Adds PlayerLiddedOpenEvent by 456dev · Pull Request #11814 · PaperMC/Paper

456dev · 2024-12-25T02:00:59Z

replaces #11379, pre hard-fork pr

minimal changes from previous pr, mostly some variable renames + //paper comments, to be in-line with the new guidelines.

Additionally, I need to double check the implementation of PlayerLiddedOpenEvent for shulker boxes, particularly if players can leak.

previous description below:

Featuring

5 LidModes, 3 of which were not possible before
The ability to get if the lid actually should be open, if a player is in the container
The ability to get if the lid is open, from api or the player itself

This also deprecates Bukkits api because of the confusing and outright incorrect javadocs (so those now accurately represent the behaviour)

Testable with the included testplugin commands.

I'm not 100% sure if the PaperLidded interface is the best way of doing this, but it is working
Also, let me know if the naming needs to change.

see if anything important is skipped in the check method scheduled every 5 ticks on chests

456dev · 2024-12-25T02:13:38Z

Fixed #11364 with new api

456dev · 2024-12-25T03:50:05Z

+/**
+ * @deprecated Incomplete api. Use {@link io.papermc.paper.block.Lidded} instead.
+ */
+@Deprecated // Paper - Deprecate Bukkit's Lidded API


TODO: remove this lone // paper comment

Suggested change

@Deprecated // Paper - Deprecate Bukkit's Lidded API

@Deprecated

456dev · 2024-12-25T03:53:15Z

+ * Cancelling this event prevents the player from being considered in other {@link Lidded} methods:
+ * they will not contribute to the {@link Lidded#getTrueLidState()} and {@link Lidded#getEffectiveLidState()}.
+ * <p>
+ * This event is called twice for double chests, once for each half.


todo: fix global implementation for double chests.

iirc as well as this double-fire behaviour, modifying the state of the non-dominant half doesnt play a sound, but still keeps the animation open seperately.

456dev · 2024-12-25T03:54:51Z

+    LidState getEffectiveLidState();
+
+    /**
+     * Gets how the lid would be without any lidded mode, based on players interacting with the block.


TODO: replace with smth like

Suggested change

* Gets how the lid would be without any lidded mode, based on players interacting with the block.

* Gets how the lid would be with {@link LidMode.DEFAULT}

* I.E. based on players interacting with the block.

456dev · 2024-12-25T03:55:41Z

+
+import org.jspecify.annotations.NullMarked;
+
+@NullMarked


Is this actually needed, since its a very simple enum?

This enum and LidMode don't have anything that would be affected by it, so I would say it's not needed.

- adds `/test_lidded_new <blockpos> query` to get the api's view of the block - adds `/test_lidded_new <blockpos> set <lidmode>` to change it. - adds `/test_lidded_old <blockpos> <is_open | close | toggle>` to use the implementation of the old api on top of the new api. - adds `LiddedTestListener` to test `PlayerLiddedOpenEvent`, which makes any lidded blocks open quietly (ie cancel the event), if done while holding any wool in your main hand. since it's not the easiest thing to test in-game without some api consumer/activator a seconds player might be useful to inspect state while it is open/closed with multiple people.

Warriorrrr · 2025-03-17T08:28:37Z

+
+import org.jspecify.annotations.NullMarked;
+
+@NullMarked


This enum and LidMode don't have anything that would be affected by it, so I would say it's not needed.

Warriorrrr · 2025-03-17T08:58:24Z

+     * @deprecated Misleading name. Use {@link io.papermc.paper.block.Lidded#getLidMode()} for the direct replacement, or {@link io.papermc.paper.block.Lidded#getEffectiveLidState()} to tell if the lid is visibly open to the player instead.
     */
+    @Deprecated
    boolean isOpen();


If these names are misleading, I think it would be better to fix the implementation rather than changing the javadoc and deprecating the methods.

I think it would also be better to add onto this class rather than creating a whole new interface.

Just to confirm, it would be preferred to completely replace this API in 1 step?

I didn't want to create any breaking changes, and making it separate would allow for it to be cleanly removed separately at a later point.

What I'd prefer would be to add the methods to the existing API and leave the old methods the same, and changing the impl of the old methods to use the new api impl.

I'll wait for others to respond with their thoughts in case I'm not making sense.

Warriorrrr · 2025-03-17T09:00:18Z

    @Override
-    public void open() {
+    public CraftBarrel copy() {
+        return new CraftBarrel(this, null);
+    }
+
+    @Override
+    public CraftBarrel copy(Location location) {
+        return new CraftBarrel(this, location);
+    }


The methods being moved around like this creates a lot of unnecessary diff.

lynxplay · 2025-10-05T22:29:36Z

This is the third of fourth time I am reading through this PR and I think the surface area of this is just too large for me to be confortable with merging this.
The fact that we are duplicating seemingly large parts of the process between the shulker and containers screams for externalized handling.

The same can be said for the commented out craftbukkit diff. Just delete it, it makes the code feel even more duplicate.

private final java.util.Set<Player> cancelledPlayer feels dangerous.

But yea, I think the only sane way we will ever get a PR like this in is completely ripping out CB lidded diff in commit A and reintroducing a very much externalised lidded system into the affected types.
With that I mean io.papermc.something.ContainerLiddedState classes that do all the things that are currently copied around like maintaining the lidded state, openCounter etc.

As said, the giant diff in the CB types also does not help making this more reviewable but moving thins from patches into normal types to handle state would be a good start to this.

lynxplay · 2025-11-01T17:34:59Z

Going to close this PR for now, feel free to reopen if / when you find time to heavily reduce / externalize the diff.
Might also be worth to first flesh out the entire API in an API only PR or whatnot given it'd be a shame if you put in a lot of internal work just to then have a lot of changes requested to the API surface.

456dev requested a review from a team as a code owner December 25, 2024 02:01

456dev commented Dec 25, 2024

View reviewed changes

kennytv added the type: feature Request for a new Feature. label Dec 25, 2024

456dev added 5 commits January 8, 2025 04:29

Add new Lidded API

727b00d

Add PlayerLiddedOpenEvent

914506a

Aren't imports cool (new lidded API)

d217683

Aren't imports cool (PlayerLiddedOpenEvent)

7bdba6c

456dev force-pushed the feat/new-lidded-api branch from 4a28d31 to 7bdba6c Compare January 8, 2025 04:29

Warriorrrr linked an issue Feb 18, 2025 that may be closed by this pull request

Lidded containers closing issue #11364

Open

Warriorrrr requested changes Mar 17, 2025

View reviewed changes

github-project-automation Bot moved this from Awaiting review to Changes required in Paper PR Queue Mar 17, 2025

lynxplay closed this Nov 1, 2025

github-project-automation Bot moved this from Changes required to Closed in Paper PR Queue Nov 1, 2025

	@Deprecated // Paper - Deprecate Bukkit's Lidded API
	@Deprecated

	* Gets how the lid would be without any lidded mode, based on players interacting with the block.
	* Gets how the lid would be with {@link LidMode.DEFAULT}
	* I.E. based on players interacting with the block.


		import org.jspecify.annotations.NullMarked;

		@NullMarked


		import org.jspecify.annotations.NullMarked;

		@NullMarked

Uh oh!

Conversation

456dev commented Dec 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

456dev commented Dec 25, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lynxplay commented Oct 5, 2025

Uh oh!

lynxplay commented Nov 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

456dev commented Dec 25, 2024 •

edited

Loading