Skip to content
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.

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

Add removal reason to EntityRemoveFromWorldEvent #10149

Closed

Conversation

TonytheMacaroni
Copy link
Contributor

Adds a removal reason to EntityRemoveFromWorldEvent. The RemovalReason enum mirrors the NMS Entity.RemovalReason, except that the methods shouldDestroy/shouldSave are renamed to willDestroy/willSave. Due to the chunk system rewrite, unloaded entities are not set as removed with RemovalReason.UNLOAD_TO_CHUNK, so currently if the removal reason is not set and the entity is inaccessible, it'll default to RemovalReason.UNLOAD_TO_CHUNK as the removal reason.

@TonytheMacaroni TonytheMacaroni requested a review from a team as a code owner January 8, 2024 17:34
@TonytheMacaroni TonytheMacaroni force-pushed the removal-reason branch 2 times, most recently from 7c58493 to 39b9d10 Compare January 23, 2024 20:59
kennytv
kennytv previously approved these changes Jan 24, 2024
@TonytheMacaroni TonytheMacaroni force-pushed the removal-reason branch 2 times, most recently from 68721e7 to 80d564f Compare February 2, 2024 18:25
NoahvdAa
NoahvdAa previously approved these changes Mar 3, 2024
@lynxplay lynxplay dismissed stale reviews from kennytv and NoahvdAa March 16, 2024 19:16

We should expose upstreams resasons here too.

@lynxplay
Copy link
Contributor

Given upstream has merged into an entity remove from world event, can you expand this PR to include the new reasons?

Would be fixing #10280

@prototype464
Copy link

Yes please!

@TonytheMacaroni TonytheMacaroni force-pushed the removal-reason branch 2 times, most recently from abb4c83 to 94119a3 Compare July 16, 2024 18:15
@TonytheMacaroni
Copy link
Contributor Author

Updated the PR to include the causes upstream added. Also moved the primary event call to CraftEventFactory#callEntityRemoveEvent, as using ServerLevel.EntityCallbacks#onTrackingEnd didn't provide the additional cause context, and that method isn't called on entity unload. The previous event call there was made to use a new INACCESSIBLE cause, for when the chunk an entity is in is made inaccessible (but prior to fully unloading).

@TonytheMacaroni
Copy link
Contributor Author

Rebased. Also removed some unneeded diff.

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.

6 participants