Skip to content

Fix ItemDespawnEvent not firing when "despawn-time" has been configured for items.#12561

Closed
kcbleeker wants to merge 1 commit into
PaperMC:mainfrom
kcbleeker:despawn-time-fix
Closed

Fix ItemDespawnEvent not firing when "despawn-time" has been configured for items.#12561
kcbleeker wants to merge 1 commit into
PaperMC:mainfrom
kcbleeker:despawn-time-fix

Conversation

@kcbleeker
Copy link
Copy Markdown

While working on this plugin, I found that if I had a despawn-time value configured for "items" other than the default 5 minutes, it would not fire the ItemDespawnEvent, and I could not hook into it.

I have built this locally and tested that the fix works, but I'm no expert on the impact to performance or if there's a better way to achieve this.

@kcbleeker kcbleeker requested a review from a team as a code owner May 18, 2025 16:55
@github-project-automation github-project-automation Bot moved this to Awaiting review in Paper PR Queue May 18, 2025
@Warriorrrr
Copy link
Copy Markdown
Member

This seems like it would cause the event to get called every following tick when cancelled and prevent items past the despawn time from doing their normal tick logic due to the return.

As an alternative, we already have the item despawn time option in the spigot config and the alt item despawn rate option in the paper world config that you could use, they are better for items in this case

@Owen1212055
Copy link
Copy Markdown
Member

Yeah, in general this behavior is independent from the default item despawn behavior, which is what that event covers... so I am not sure if putting the event there is the best.

This logic is configurable for each server and sets a hard despawn for each entity, which I am not sure should be configured by plugins. Your plugin can listen to this removal by the EntityRemoveEvent and check for DESPAWN reason.

@lynxplay
Copy link
Copy Markdown
Contributor

Yea, also in favour of not using this (very much vanilla despawn behaviour) event for this purpose.
Paper's "despawn time" setting (while maybe inappropriately named) is not at all related to vanilla despawning (items have that alternative despawning behaviour to configure the vanilla values there).
It can very much be interpreted in the same vein of a plugin .remove()-ing the entity after a certain set of ticks.

I'll throw this at one more pair of eyes here tho 👍

@kcbleeker
Copy link
Copy Markdown
Author

Just highlighting in your discussion that the root issue is that the despawn event does not fire, and EntityRemoveEvent has been marked for deprecation. If that is the official answer though, I'm quite happy to use that rather? It makes sense, less risk of blocking existing behaviour.

@Owen1212055
Copy link
Copy Markdown
Member

Fyi, EntityRemoveEvent is no longer deprecated for removal.

@kcbleeker
Copy link
Copy Markdown
Author

Cool. I'm happy to close this then and just use EntityRemoveEvent :)

@kcbleeker kcbleeker closed this May 18, 2025
@github-project-automation github-project-automation Bot moved this from Awaiting review to Closed in Paper PR Queue May 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

4 participants