PaperMC / PaperMC/Paper

AsyncStructureGenerateEvent transformers provide incorrect region interface

Open
#10,124 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: accepted type: bug
Dominant language
Java
Stars
12.7k
Forks
3.5k
Avg merge
3d 13h
Merged PRs (30d)
11

Description

Expected behavior

AsyncStructureGenerateEvent has transfomers for modifying added entities or placed blocks in EntityTransformer and BlockTransformer. The world interface provided should be RegionAccessor to reflect that structure generation can happen to WorldGenRegion and ServerLevel.

Observed/Actual behavior

These functional interfaces provide a LimitedRegion to interact with the world during that process. This reflects an incorrect implementation by upstream which thought that this all happened to WorldGenRegion. If a ServerLevel becomes wrapped in a CraftLimitedRegion all sorts of assumptions in the implementation of CraftLimitedRegion fail, like assumed casts to ProtoChunk.

Steps/models to reproduce
  1. Setup an event handler in a test plugin.
@EventHandler
public void onEvent(AsyncStructureGenerateEvent event) {
    event.setEntityTransformer(new NamespacedKey(this, "test"), (region, x, y, z, entity, allowedToSpawn) -> {
		// execute a single call to getEntities which will throw CCE (except its hidden because debug logging for commands is disabled but the command will show an error message
		System.out.println(region.getEntities());
        return true;
    });
}
  1. Run a /place structure ... command with a village. Structure generation via /place operates on a ServerLevel whereas normal chunk generation places structures via WorldGenRegion.
  2. Observe the command failing. There won't be an explicit error message unless you change the Commands.java slf4j logger to have debug enabled. You can add a breakpoint yourself to view the actual exception which is a CCE from LevelChunk to ProtoChunk.
Plugin and Datapack List

N/A

Paper version

https://github.com/PaperMC/Paper/commit/1fa48d140c5469d894b436a7ba0cb5cb6f819ee0

Other

Similar to https://github.com/PaperMC/Paper/issues/10123, this is caused by the incorrect assumption that it will always be a WorldGenRegion when running the transformers set by the API. Unfortunately, I think this requires an API break to fix. Changing the type of region in BlockTransformer and EntityTransformer from LimitedRegion to RegionAccessor. It should never have been LimitedRegion in the first place and having LimitedRegion is broken functionality.

I think breaking API here is completely fine because any plugins which based their logic on this assumption are also incorrect and should be required to fix it. This API is also relatively new and so hopefully not too many plugins will be forced to fix and re-compile.

EDIT: I'm now realizing that its marked as Experimental API by upstream so we can easily break it.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the AsyncStructureGenerateEvent API declarations for EntityTransformer and BlockTransformer and compare their region parameter types with RegionAccessor and LimitedRegion. Reproduce the issue with a /place structure command and a transformer calling getEntities(). Done means both transformers expose the correct region interface without the ServerLevel-to-ProtoChunk cast failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.