Skip to content

Read bisected halves through the WorldEdit edit, not the live world - #1016

Open
tricrotism wants to merge 4 commits into
PlayPro:masterfrom
tricrotism:for-upstream/worldedit-bisected-halves
Open

tricrotism wants to merge 4 commits into
PlayPro:masterfrom
tricrotism:for-upstream/worldedit-bisected-halves

Conversation

@tricrotism

Copy link
Copy Markdown
Contributor

Summary

When a WorldEdit or FastAsyncWorldEdit edit removes a door or a tall plant, CoreProtect reads the other half of the block from the live world, on the edit thread. That thread does not own the chunk (always with FAWE, and on Folia with plain WorldEdit too), and the live world can already hold the edit's result or not yet hold it. This change reads the other half from the edit itself.

The problem

CoreProtect logs double blocks (doors, sunflowers, tall grass and so on) by their lower half. Two places read that half from the world:

  • consumer/Queue.java:201-215, queueBlockBreak: for a top half it replaces the block with block.getWorld().getBlockAt(x, y - 1, z).getState().
  • worldedit/WorldEditLogger.java:154: for a removed bisected block it reads the other half with location.getWorld().getBlockAt(...).getState() and queues a break for it.

Both run on whatever thread applies the edit. With FAWE that is an async worker. On Folia it is never the region thread that owns the chunk. getBlockAt(...).getState() from those threads reads chunk data the owning thread may be writing, and on Folia it can throw. Even without a thread problem, the read races the edit: the half below may already be changed by the same edit, or not yet, so the row is logged from the wrong block or dropped when the types no longer match.

The fix

The edit supplies the lower half, so no edit thread reads the world:

  • worldedit/WorldEditBlockState.java: an optional lowerHalf field with a getter and setter.
  • worldedit/WorldEditLogger.java: for a top half that Queue logs by its lower half, the vanilla WorldEdit path reads the block below through the edit's own extent (extent.getBlock(position.add(0, -1, 0))) and attaches it. The removed bisected partner is also read through the extent instead of the world.
  • worldedit/FastAsyncWorldEditLogger.java: the FAWE processor already holds the chunk's IChunkGet and IChunkSet arrays, so it reads the half below from those. If the edit changes that block too, it passes null, because that block gets its own row, which covers the double block.
  • consumer/Queue.java: for a WorldEditBlockState it uses the attached lower half instead of the world, and skips the row when there is none. Anything that is not a WorldEdit state goes through the old code.
  • model/BlockGroup.java: the type list Queue tested inline (IRON_DOOR, DOORS, SUNFLOWER, LILAC, TALL_GRASS, LARGE_FERN, ROSE_BUSH, PEONY) moves into LOGGED_BY_LOWER_HALF, so the edit side can use the same list to decide when to read a lower half. DOORS is added in initialize(), after the version adapters have filled it, the same way TRACK_TOP.addAll(DOORS) already works, so the membership matches what Queue tested before.

Behaviour change

For WorldEdit edits only:

  • The other half of a removed bisected block is logged from what the edit sees rather than the live world.
  • A non-plant bisected neighbour such as stairs is now built from block data only, so its row does not carry block entity data (skull or spawner type). Upstream read that from the live world.

Nothing changes for normal block breaks.

Risk

  • FAWE index math: ((y & 15) << 8) | (z << 4) | x, the layout FAWE uses for its section arrays, with the section taken from lowerY >> 4 so a lower half in the section below is found. A lower half below the world's minimum section returns null.
  • A WorldEditBlockState top half with no lower half attached is skipped. Every path that builds one for a logged top half attaches it first.

Testing

Build: mvn package passes.
Row parity without WorldEdit installed: the 47-step scenario on Paper 26.2 with SQLite matches upstream apart from the random plant that bone meal grows and one far-basin water row that lands either side of its settling window depending on run pacing. That covers the Queue.queueBlockBreak change for normal breaks, which still read the half below from the world.

I could not run a live FAWE edit on the testbed: FastAsyncWorldEdit 2.15.0 does not start on Paper 26.2 and does not support Folia. Suggested test on a Paper version FAWE supports: build a row of doors and tall flowers, //set air over the top halves only, then over whole blocks, then /co lookup r:10 t:1m and /co rollback r:10 t:1m. Compare against upstream: the same rows, and the rollback restores both halves.

When an edit removes a door or tall plant, the other half was read from the live world on the edit thread, which does not own the chunk and can see blocks the edit already changed. The half is now read through the edit extent, or the FAWE processor's chunk arrays, and carried on WorldEditBlockState. The type list Queue checked inline moves to BlockGroup.LOGGED_BY_LOWER_HALF so both sides use the same one.
@netlify

netlify Bot commented Sep 23, 2026

Copy link
Copy Markdown

❌ Deploy Preview for coreprotect failed. Why did it fail? →

Name Link
🔨 Latest commit ce999a1
🔍 Latest deploy log https://app.netlify.com/projects/coreprotect/deploys/6ab3ef01edbb1b0008495f10

@Intelli

Intelli commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Thanks -- automated review is requesting the following changes:

  1. The plain WorldEdit extent is not necessarily a snapshot: our BEFORE_CHANGE hook sits below batching, and reads can still reach the live Bukkit world. These reads also occur after setBlock(). Please either narrow the fix and its claims to FAWE, or capture the required original partner state before it can be changed.
  2. Please validate that the adjacent block is actually the matching opposite half before queuing its break. The new WorldEditBlockState drops metadata that the previous Bukkit state preserved when an unrelated skull/spawner occupies that position. If logging such blocks is intentional, their metadata needs to be retained.

CoreProtect's extent sits at BEFORE_CHANGE, below WorldEdit's batching, so its reads reach the live world, and both the lower half and the removed partner were read after setBlock(). Read the other half before setBlock() instead, and only use it when it is the same material with the opposite half, so an unrelated block next to a removed half is no longer logged from block data alone. A door or tall flower is already logged by its lower half, so its partner no longer gets a second, duplicate row. The FAWE lower half is validated the same way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants