fix(portal): link nether portals whose chunks are unloaded 🤖🤖🤖 - #3203
Open
ydw1904 wants to merge 2 commits into
Open
fix(portal): link nether portals whose chunks are unloaded 🤖🤖🤖#3203ydw1904 wants to merge 2 commits into
ydw1904 wants to merge 2 commits into
Conversation
`search_for_portal` looks up candidates in the POI index, which survives chunk unloading, then validates them with `World::get_block_and_state`. That resolves through `read_chunk_sync`, a bare `loaded_chunks` lookup that reports an unloaded chunk as air. Leaving a dimension unloads exactly the chunks the portal sits in, so on the return trip every candidate read as air and was discarded, the search returned `None`, and the fallback built a duplicate portal. Repeating this stranded a new portal pair on each crossing. This is a regression. Pumpkin-MC#3001 fixed the same fault with the chunk-loading `get_block_state_id_async`; 5d841d6 ("refactor(entities): from async to sync Part 2") swapped that accessor for the plain sync one and reintroduced it. The async accessor is unusable here now that the call path is sync, and it only ever guarded the first read anyway. Load each surviving candidate's chunk plus its 8 neighbours before reading any blocks, deduplicated by chunk coordinate so one portal costs a single 3x3 load rather than one per portal block. Neighbours are required because `get_on_axis` walks up to MAX_WIDTH/MAX_HEIGHT blocks out from the POI and crosses chunk borders, which the old async accessor never covered. Vanilla gets this for free: `Level#getBlockState` loads the chunk it needs. Also correct SEARCH_RADIUS_NETHER from 128 to 16. Vanilla reduced it in 1.16.2 "in order to correctly account for the 1:8 position scale"; at 128 two overworld portals up to 1024 blocks apart share one nether portal instead of each getting their own. This was unreachable while the search above always failed, so it has to land in the same change rather than after it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
clippy::doc_markdown, denied via clippy::pedantic in CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Nether portals stop linking to each other once their chunks unload. Each crossing builds a fresh portal instead of reusing the existing one, so you end up with a chain of orphaned pairs and get dropped somewhere new every time you switch dimensions.
search_for_portalfinds candidates in the POI index, which survives chunk unloading, then validates them withWorld::get_block_and_state. That resolves throughread_chunk_sync, a bareloaded_chunkslookup that reports an unloaded chunk as air. Leaving a dimension unloads exactly the chunks the portal sits in, so on the return trip every candidate reads as air and is discarded, the search returnsNone, and the fallback builds a duplicate.The chunk loading in
PortalType::get_portal_destinationonly runs in theor_elsearm, after the search has already failed, and the search is never retried.This is a regression
#3001 fixed the same fault using the chunk-loading
get_block_state_id_async. 5d841d6 (refactor(entities): from async to sync Part 2) swapped that accessor for the plain sync one:The async accessor still exists but is unusable here now that the call path is sync, and it only ever guarded the first read —
get_on_axiswalks up to 21 blocks outward from the POI and crosses chunk borders unguarded.Changes
Preload candidate chunks. Filter candidates by Y and world border first, drop the world-border guard, then load each survivor's chunk plus its 8 neighbours before any block read. Deduplicated by chunk coordinate, so one portal costs a single 3×3 load rather than one per portal block. Bridges Rayon → Tokio with the
block_in_place+block_onpattern already used inportal/mod.rs. Vanilla gets this for free:Level#getBlockStateloads the chunk it needs.SEARCH_RADIUS_NETHER: 128 → 16. Vanilla reduced this in 1.16.2 "in order to correctly account for the 1:8 position scale" (wiki); the branch already keys on the destination dimension, only the value was wrong. At 128, two overworld portals up to 1024 blocks apart share one nether portal instead of each getting their own — walk into portal B, come back out of portal A. It was unreachable while the search above always failed, so it has to land here rather than in a follow-up. PaperMC hit the same divergence in PaperMC/Paper#3795 and PaperMC/Paper#4573.Both changes are in one commit deliberately: shipping the chunk fix alone would activate the 1024-block snap bug that was previously dead code.
Testing
master.cargo test— 256 passcargo clippy --all-targets— clean>> 4floor-divides,-1 → -2)There is no end-to-end regression test — that needs a
Worldharness with force-unloaded chunks, which doesn't exist in the repo. The load-before-read ordering is guarded by a comment naming this regression's history so the next async→sync pass doesn't silently undo it again.Not in scope
find_safe_locationscans a ±32 box; vanillacreatePortalusesBlockPos.spiralAround(pos, 16, …). Different size and shape, and its edges read past the chunks theor_elsearm loads — same cold-chunk class, different function.PortalShapeobsidian-frame validator; vanilla usesBlockUtil.getLargestRectangleAroundfor both source and exit rectangles and ignores the frame. Affects sliced/partly-mined portals. Left alone deliberately — it interacts with block-update-suppression behaviour that's a judgement call for the core team.AI assistance
Investigated and implemented with Claude Code (Opus 5). Root cause traced from the reported symptom, confirmed against the reporter's world save by decoding its POI region files, and verified in-game before submission. All vanilla behaviour cited above was checked against primary sources rather than recalled.