Skip to content

Make sure block entities are valid when loading them#10383

Closed
QuickGlare wants to merge 1 commit into
PaperMC:masterfrom
QuickGlare:remove-invalid-block-entity
Closed

Make sure block entities are valid when loading them#10383
QuickGlare wants to merge 1 commit into
PaperMC:masterfrom
QuickGlare:remove-invalid-block-entity

Conversation

@QuickGlare
Copy link
Copy Markdown
Contributor

This PR fixes these 2 issues (#10022 and sachingorkar102/Lootin-plugin#24) but for existing worlds

When a chunk loads it checks all the existing block entities and makes sure they are valid, if they are not the block entities are removed from the world.

@QuickGlare QuickGlare requested a review from a team as a code owner April 2, 2024 19:49
@Owen1212055
Copy link
Copy Markdown
Member

This seems more like a bandaid fix, where exactly are these block entities being added from? I would think it would be better to patch where these are coming from.

@QuickGlare
Copy link
Copy Markdown
Contributor Author

This seems more like a bandaid fix, where exactly are these block entities being added from? I would think it would be better to patch where these are coming from.

These block entities are coming from this other issue (#10375), the other PR fixes it only for new worlds and this one is a fix for existing ones

@Owen1212055
Copy link
Copy Markdown
Member

Is this only an issue for old worlds that use the custom generator override stuff but then load those chunks?

@QuickGlare
Copy link
Copy Markdown
Contributor Author

Is this only an issue for old worlds that use the custom generator override stuff but then load those chunks?

Yes, because they would have a Sculk Sensor block entity where there is a Sculk Catalyst block

@Owen1212055
Copy link
Copy Markdown
Member

Then maybe we should have this under a config. Not many plugins use this system, and i'd hate to keep this workaround forever.

@electronicboy
Copy link
Copy Markdown
Member

adding a config option generally offers a level of commitment beyond just having something sitting around for a while, otherwise, we'd need to investigate putting it as part of the forceUpgrade pathway so that we can tell people to just run that before we remove it, but, that seems meh

@QuickGlare
Copy link
Copy Markdown
Contributor Author

adding a config option generally offers a level of commitment beyond just having something sitting around for a while, otherwise, we'd need to investigate putting it as part of the forceUpgrade pathway so that we can tell people to just run that before we remove it, but, that seems meh

I didn't check how the forceUpdate flag works in the code but that sounds like a good idea, it's something that doesn't run everytime a chunk loads and it's a thing that servers with corrupted worlds would have to run only once

@electronicboy
Copy link
Copy Markdown
Member

not all hosts offer the means to do stuff like that, however; is part of the concern; but, if it was a patch that we wanted to only keep for a short period of time, considering that it causes crashes, we'd need at least some tool that we can recommend to people in order to scan their worlds and clean up such busted state

@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Apr 6, 2024

After a bit of discussion, I'll have a look at simply porting this check to a plugin that can perform this check via command.
Maintaining this fix in paper for a single issue caused by a shitty spigot api implementation is rather annoying, so a plugin for people affected by this seems more friendly.

I'll leave this open in case my timeline tomorrow explodes and I don't find the time for this soon.

@lynxplay
Copy link
Copy Markdown
Contributor

Threw together a quick plugin to iterate and process all chunks to find and remove these invalid block entities:

https://github.com/lynxplay/block-entity-bin

People that had their worlds corrupted by the respective bug may use the plugin to fix their chunk data.
Any form of issue regarding the plugin is to be opened on the plugin's github page, not paper's repository.
As with any form of world data shenanigans, backup your data before running this.

@lynxplay lynxplay closed this Apr 19, 2024
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.

4 participants