Skip to content

validate switch cases against mapper values - #22

Open
VasilisDragon wants to merge 1 commit into
ProtoDef-io:masterfrom
VasilisDragon:fix-mapper-switch-validation
Open

VasilisDragon wants to merge 1 commit into
ProtoDef-io:masterfrom
VasilisDragon:fix-mapper-switch-validation

Conversation

@VasilisDragon

Copy link
Copy Markdown

Reject literal switch cases that aren't outputs of a known mapper. Resolves container paths and aliases, and skips relationships with unknown semantics.

Adds generic tests. npm test passes on Node 14 and 24; the unchanged Minecraft protocol suite and all 112 physical PC/Bedrock protocol definitions pass.

Fixes #21.

@VasilisDragon

Copy link
Copy Markdown
Author

Found one edge here: mapper output "1" with switch case "0x1" round-trips in the compiler, but the interpreter and this check reject it. I’d leave ambiguous numeric matches unchecked while keeping the named-case checks. Does that fit what you had in mind?

@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.

The generic mapper/switch check belongs in this validator and the opaque-scope skips are appropriate, but the numeric-key ambiguity already identified here still needs handling. Mapper output "1" can match switch key "0x1" in the supported compiler while this literal-string membership check rejects it.

Leave ambiguous numeric relationships unchecked until the engine contract is unified, while retaining strict validation of semantic named cases. Add that control to the generic tests. I have no additional root cause to duplicate inline and did not rerun the full protocol matrix.

Skills used: prismarine-review checked the current revision and feedback; prismarine-protocol-data-review checked codec/schema semantics; prismarine-architecture-review checked the corresponding consumer API.

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.

Add protocol mapper sanity check to protodef-validator

2 participants