fix(client): count mana shards serialized as engine variant names#5065
Open
jeffrey701 wants to merge 1 commit into
Open
fix(client): count mana shards serialized as engine variant names#5065jeffrey701 wants to merge 1 commit into
jeffrey701 wants to merge 1 commit into
Conversation
getDominantManaColor / getDeckDominantColor keyed SHARD_TO_COLOR by Scryfall
symbols (W/U/B/R/G), but the engine serializes mana_cost.shards as Rust variant
names ("White", "WhiteBlue", "TwoWhite", "PhyrexianWhite"…), so every
non-land spell's colored pips were silently dropped and the auto-WUBRG
BattlefieldBackground ignored spell colors entirely.
Bridge each shard through the canonical SHARD_ABBREVIATION map (the same one
manaCostToShards/manaAvailability use) before splitting hybrids, with a
`?? shard` fallback that still accepts an already-symbolic shard. Hybrid
("WhiteBlue"→W/U), monocolored-hybrid ("TwoWhite"→2/W → W), and Phyrexian
("PhyrexianWhite"→W/P → W) shards now contribute their colors; X/Colorless
correctly contribute none.
Adds discriminating tests using the real variant-name wire format (they fail
without the fix); the existing symbol-format test still passes via the fallback.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
matthewevans
added
the
defer-fe
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
2 participants
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.
Problem
getDominantManaColor/getDeckDominantColor(client/src/viewmodel/dominantColor.ts) keySHARD_TO_COLORby Scryfall symbols (W/U/B/R/G), but the engine serializesmana_cost.shardsas Rust variant names —"White","WhiteBlue","TwoWhite","PhyrexianWhite", … (seeSHARD_ABBREVIATIONincostLabel.ts,manaValueOfShardinmanaValue.ts, and the fixtures used across the client). So every non-land spell shard misses the map: colored pips are silently dropped and the auto-WUBRGBattlefieldBackground(the sole consumer) ignores spell colors entirely. Hybrid/gold spells are doubly broken — thesplit("/")half-counting never fires because the engine emits"WhiteBlue", not"W/U".Fix
Bridge each shard through the canonical
SHARD_ABBREVIATIONmap (the same mapmanaCostToShards/manaAvailabilityalready use) before splitting hybrids, with a?? shardfallback that still accepts an already-symbolic shard. NowWhite→W,WhiteBlue→W/U(both halves),TwoWhite→2/W(W),PhyrexianWhite→W/P(W);X/Colorlesscorrectly contribute no color.Tests
Adds discriminating tests using the real variant-name wire format — they fail on the current code (the shard misses the map →
null) and pass after the fix. The existing symbol-format test still passes via the?? shardfallback (no regression).tsc -b --noEmitandeslintclean;vitest12/12.Self-contained: only
dominantColor.ts+ its test; no engine/Rust/protocol changes.Model: claude-opus-4-8