Skip to content

Classify blocks by defaultState for runtime-state editions (Bedrock) - #388

Open
Pix3lPirat3 wants to merge 1 commit into
PrismarineJS:masterfrom
Pix3lPirat3:feat/bedrock-support
Open

Pix3lPirat3 wants to merge 1 commit into
PrismarineJS:masterfrom
Pix3lPirat3:feat/bedrock-support

Conversation

@Pix3lPirat3

Copy link
Copy Markdown

Movements builds its fences / carpets / emptyBlocks sets from the collision-shape height of Block.fromStateId(block.minStateId, 0).

That assumes a static minStateId, which Java has but Bedrock does not: Bedrock uses runtime block-state palettes and exposes defaultState instead (minStateId is undefined). On Bedrock fromStateId(undefined) yields empty shapes, so none of those sets are ever populated — fences and carpets come out empty, and fences are then treated as full/physical (walkable) and carpets as unsafe.

Fix: fall back to defaultState when minStateId is absent.

registry.blocksArray.map(x => Block.fromStateId(x.minStateId ?? x.defaultState, 0))
  • Java: minStateId is always present, so ?? is a no-op — behaviour unchanged (verified: 1.21.1 still fences=37, carpets=18, emptyBlocks=236).
  • Bedrock: now populated (fences=58, carpets=19 on bedrock_1.26.45), and defaultState resolves the correct shapes (e.g. oak_fence → 1.5-tall).

Verified live on a Bedrock Dedicated Server (1.26.45) via the in-mineflayer Bedrock adapter (PrismarineJS/mineflayer#4145): pathfinder navigates, routes around walls, auto-jumps steps and digs through, and this fixes the fence/carpet classification. Java lint passes; no change to Java behaviour.

Movements builds its fence/carpet/emptyBlock sets from Block.fromStateId(block.minStateId). Bedrock uses runtime block-state palettes and has no static minStateId (it exposes defaultState), so on Bedrock fromStateId returned empty shapes and those sets were never populated (fences=0, carpets=0). Fall back to defaultState when minStateId is absent, restoring correct classification on Bedrock (fences=58, carpets=19) with no change on Java (minStateId always present). Verified live on BDS 1.26.45.

@rom1504 rom1504 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Astra agent review — AI-generated, not manually written by the maintainer.

No blocking issue found in the fallback itself. I exercised the real Movements/prismarine-block path: the Java classification sets stay unchanged on 1.8.8, 1.20.4 and 1.21.1, and a Bedrock registry with minStateId omitted from its block list classifies the same sets as explicit defaultState lookup (58 fences, 19 carpets and 222 empty blocks on 1.26.45). Lint passes.

One scope clarification: the published Bedrock registry I tested already supplies minStateId, so that run alone does not exercise this fallback; the omitted-field check does. This review covers the local classification change, not the wider adapter or live navigation integration.

Skills used: prismarine-protocol-data-review checked the actual registry shape and state lookup; prismarine-architecture-review checked the existing model boundary; prismarine-review checked the diff and existing discussion.

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.

2 participants