From 0d49e42574c12358affc3d89aaf599f708262d8e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 10:01:47 +0000 Subject: [PATCH 1/2] Fix Showdown import corrupting species on real-world sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parseShowdownBlock treated any unrecognized line as a fresh species line, so pasting a real Pokémon Showdown export with a Level:/Tera Type:/Shiny:/ etc. line (verified against sim/teams.ts in smogon/pokemon-showdown - Gen 9 sets almost always carry Tera Type:) silently overwrote the species and broke import. Only the block's first line is ever the species line now, nickname/gender markers are stripped, Trait: is accepted as a legacy Ability: alias, and the exporter no longer writes blank Ability:/EVs:/ Nature placeholder lines or a dangling "@ " - matching the real client's own grammar exactly. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018Khih3hZr4mzVhb3TmDnVU --- CHANGELOG.md | 18 ++++ .../domain/showdown/ShowdownFormat.kt | 76 +++++++++++----- .../domain/showdown/ShowdownFormatTest.kt | 91 +++++++++++++++++-- docs/implementation-decisions.md | 54 +++++++++++ docs/test-plan.md | 9 +- 5 files changed, 218 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b300a93..b762068 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,24 @@ versions follow [Semantic Versioning](https://semver.org/). ## [Unreleased] +- **Fixed Showdown import silently corrupting the species on any set with a + `Level:`, `Tera Type:`, `Shiny:`, or similar optional field.** Verified + against Pokémon Showdown's own text-format grammar + (`sim/teams.ts` in smogon/pokemon-showdown): the parser used to treat + any unrecognized line as a fresh species line, so pasting a real, + everyday Showdown export (Gen 9 sets almost always carry a + `Tera Type:` line) overwrote the species with that line's text and + broke import. Only the block's first line is a species line now, every + other unrecognized line is ignored, and nickname/gender markers + (`Volt Turtle (Pikachu) @ Light Ball`, `Pikachu (F)`) are stripped + before matching — see `docs/implementation-decisions.md`, "Showdown + format compatibility". +- **Showdown export no longer writes blank `Ability:`/`EVs:`/`Nature` + placeholder lines or a dangling `@ ` with nothing after it.** Real + Showdown omits a field's line entirely when it's unset; the exported + text now matches that shape exactly instead of relying on the real + client happening to ignore the old placeholders. + ## [2.0.1] - 2026-09-06 - **Fixed a crash in "Regenerate" (Surprise Me).** `regenerateSlot`'s diff --git a/app/src/main/java/com/marcogn/coverdex/domain/showdown/ShowdownFormat.kt b/app/src/main/java/com/marcogn/coverdex/domain/showdown/ShowdownFormat.kt index 4e3a10c..a021755 100644 --- a/app/src/main/java/com/marcogn/coverdex/domain/showdown/ShowdownFormat.kt +++ b/app/src/main/java/com/marcogn/coverdex/domain/showdown/ShowdownFormat.kt @@ -12,7 +12,10 @@ import java.util.UUID * A direct port of `legacy-web/src/utils/showdownParser.ts` — same function names, same contract * (`docs/plan/native-spec.md`, "Showdown format contract"). **External users may rely on * round-tripping**, so this is a contract, not an implementation: change the written/read shape - * only with a deliberate, documented reason. + * only with a deliberate, documented reason. One such reason already applied: real compatibility + * with Pokémon Showdown's own text format (verified against `sim/teams.ts` in + * smogon/pokemon-showdown) — see `docs/implementation-decisions.md`, "Showdown format + * compatibility", for what was wrong before and why. * * Both resolver parameters are plain, synchronous lookups rather than `PokedexRepository` calls * directly, keeping this file Android-free and unit-testable on the plain JVM — the caller @@ -41,18 +44,20 @@ private fun MoveEntry.toPokemonMove(): PokemonMove = PokemonMove( isCustom = false, ) -/** Convert a [TeamMember] to a Showdown-style block. [TeamMember.item] round-trips as the - * standard `Species @ Item` line (Phase 7 — see - * docs/plan/phase-7-accuracy-and-customization.md §4.3); EVs and nature are still untracked and - * emitted as placeholders that are valid to re-import. */ +/** Convert a [TeamMember] to a Showdown-style block, matching the real client's own `exportSet` + * grammar (`sim/teams.ts` in smogon/pokemon-showdown: a field's line is omitted entirely when + * unset, never written blank) so the output pastes cleanly into Pokémon Showdown itself. + * [TeamMember.item] round-trips as the standard `Species @ Item` line (Phase 7 — see + * docs/plan/phase-7-accuracy-and-customization.md §4.3). EVs, IVs and nature are still untracked + * and simply omitted, exactly as real Showdown omits them for a set with no EVs/IVs/nature set. */ fun exportMemberToShowdown(m: TeamMember): String { val lines = mutableListOf() - lines += "${m.speciesName} @ ${m.item ?: ""}" - lines += "Ability: ${m.ability ?: ""}" - lines += "EVs: " - lines += " Nature" + lines += if (m.item != null) "${m.speciesName} @ ${m.item}" else m.speciesName + if (m.ability != null) lines += "Ability: ${m.ability}" m.moves.forEach { mv -> if (mv != null) lines += "- ${mv.name}" } - // Include the typing as a comment so a round-trip preserves type overrides. + // Include the typing as a comment so a round-trip preserves type overrides. Real Showdown's + // parser only recognizes known line prefixes and silently ignores anything else, so this is + // safe to paste into the real client too. val typesStr = listOfNotNull(m.types.first, m.types.second).joinToString("/") { it.apiName } lines += "# Types: $typesStr" return lines.joinToString("\n") @@ -73,7 +78,18 @@ data class ImportedMember( ) private val EVS_IVS_NATURE_REGEX = Regex("EVs:|IVs:|Nature", RegexOption.IGNORE_CASE) -private val ABILITY_LINE_REGEX = Regex("^Ability:\\s*", RegexOption.IGNORE_CASE) +private val ABILITY_LINE_REGEX = Regex("^(Ability|Trait):\\s*", RegexOption.IGNORE_CASE) + +/** + * Line prefixes real Pokémon Showdown's own `exportSet`/`parseExportedTeamLine` + * (`sim/teams.ts` in smogon/pokemon-showdown) recognizes for fields this app doesn't track. + * Must be checked so these common real-Showdown-paste lines are dropped rather than + * mistaken for the species line — see the "species line" branch below. + */ +private val IGNORED_DETAIL_PREFIXES = listOf( + "level:", "shiny:", "happiness:", "pokeball:", "hidden power:", + "dynamax level:", "gigantamax:", "tera type:", +) /** Parse a single Showdown block into a [TeamMember]. */ fun parseShowdownBlock( @@ -90,8 +106,29 @@ fun parseShowdownBlock( var moveIdx = 0 val unknown = mutableListOf() - for (line in lines) { + lines.forEachIndexed { index, line -> + val lower = line.lowercase() when { + // Only the block's first line is ever the species line — matches real Showdown's + // own `isFirstLine` handling. Any *other* unrecognized line (a field this app + // doesn't model, or genuine garbage) is simply ignored rather than clobbering the + // species already parsed, exactly like the real client does. + index == 0 -> { + var speciesLine = line.substringBefore("@").trim() + if (line.contains("@")) { + val itemValue = line.substringAfter("@").trim() + if (itemValue.isNotEmpty()) item = itemValue + } + // Strip a trailing gender marker, then a "Nickname (Species)" wrapper — same + // order and shape as real Showdown's own first-line parsing. + if (speciesLine.endsWith(" (M)") || speciesLine.endsWith(" (F)")) { + speciesLine = speciesLine.dropLast(4) + } + if (speciesLine.endsWith(")") && speciesLine.contains("(")) { + speciesLine = speciesLine.dropLast(1).substringAfter("(").trim() + } + if (speciesLine.isNotEmpty()) speciesName = speciesLine + } line.startsWith("- ") -> { val moveName = line.substring(2).trim() val known = resolveMove(moveName) @@ -103,11 +140,11 @@ fun parseShowdownBlock( } if (moveIdx < 4) moves[moveIdx++] = mv } - line.startsWith("ability:", ignoreCase = true) -> { + lower.startsWith("ability:") || lower.startsWith("trait:") -> { val value = line.replaceFirst(ABILITY_LINE_REGEX, "").trim() if (value.isNotEmpty()) ability = value } - EVS_IVS_NATURE_REGEX.containsMatchIn(line) -> Unit // ignored + EVS_IVS_NATURE_REGEX.containsMatchIn(line) -> Unit // ignored, untracked line.startsWith("# Types:") -> { val parts = line.removePrefix("# Types:").trim().split("/").map { it.trim().lowercase() } val t1 = parts.getOrNull(0)?.let { PokemonType.fromApiName(it) } @@ -116,15 +153,8 @@ fun parseShowdownBlock( overrideTypes = t1 to t2 } } - !line.startsWith("#") -> { - // Species line, possibly with "@ item". - val speciesLine = line.substringBefore("@").trim() - if (speciesLine.isNotEmpty()) speciesName = speciesLine - if (line.contains("@")) { - val itemValue = line.substringAfter("@").trim() - if (itemValue.isNotEmpty()) item = itemValue - } - } + IGNORED_DETAIL_PREFIXES.any { lower.startsWith(it) } -> Unit // ignored, untracked + else -> Unit // unrecognized line: ignored, never overwrites the species } } diff --git a/app/src/test/java/com/marcogn/coverdex/domain/showdown/ShowdownFormatTest.kt b/app/src/test/java/com/marcogn/coverdex/domain/showdown/ShowdownFormatTest.kt index 4d97657..44eb5f1 100644 --- a/app/src/test/java/com/marcogn/coverdex/domain/showdown/ShowdownFormatTest.kt +++ b/app/src/test/java/com/marcogn/coverdex/domain/showdown/ShowdownFormatTest.kt @@ -54,12 +54,18 @@ class ShowdownFormatTest { val out = exportMemberToShowdown(m) assertTrue(out.startsWith("Charizard @")) assertTrue(out.contains("Ability:")) - assertTrue(out.contains("EVs:")) - assertTrue(out.contains("Nature")) assertTrue(out.contains("- fire-move")) assertTrue(out.contains("# Types: fire/flying")) } + @Test + fun `never emits blank EVs or Nature lines, real Showdown omits them when unset`() { + val m = buildMember("Charizard", PokemonType.FIRE to PokemonType.FLYING, listOf(PokemonType.FIRE)) + val out = exportMemberToShowdown(m) + assertTrue(out.lines().none { it.startsWith("EVs:") }) + assertTrue(out.lines().none { it.trim() == "Nature" || it.endsWith(" Nature") }) + } + @Test fun `exports a member with null moves, placeholders skipped`() { val m = buildMember("Snorlax", PokemonType.NORMAL to null) @@ -210,10 +216,10 @@ class ShowdownFormatTest { } @Test - fun `exports empty ability line when ability is null`() { + fun `omits the Ability line entirely when ability is null, matches real Showdown`() { val m = buildMember("Charizard", PokemonType.FIRE to PokemonType.FLYING) val out = exportMemberToShowdown(m) - assertTrue(out.lines().any { it == "Ability: " }) + assertTrue(out.lines().none { it.startsWith("Ability:") }) } @Test @@ -255,10 +261,10 @@ class ShowdownFormatTest { } @Test - fun `exports the bare @ line when no item is set, same as before Phase 7`() { + fun `exports the bare species line with no trailing at-sign when no item is set`() { val m = buildMember("Charizard", PokemonType.FIRE to PokemonType.FLYING) val out = exportMemberToShowdown(m) - assertTrue(out.lines().first() == "Charizard @ ") + assertTrue(out.lines().first() == "Charizard") } @Test @@ -282,4 +288,77 @@ class ShowdownFormatTest { val imp = parseShowdownBlock(paste, ::resolveMove, ::resolveSpecies) assertNull(imp.member.item) } + + // ---- real Showdown compatibility (verified against sim/teams.ts in + // smogon/pokemon-showdown) — a genuine paste from the real client commonly includes lines + // this app doesn't model (Level, Tera Type, Shiny, ...); those must never be mistaken for + // the species line, which is always the block's first line only. ---- + + @Test + fun `a real Showdown paste with Level and Tera Type does not corrupt the species`() { + val paste = listOf( + "Charizard @ Choice Scarf", + "Ability: Blaze", + "Level: 50", + "Shiny: Yes", + "Tera Type: Water", + "EVs: 252 Atk / 4 Def / 252 Spe", + "Jolly Nature", + "- Flamethrower", + "- Earthquake", + ).joinToString("\n") + val imp = parseShowdownBlock(paste, ::resolveMove, ::resolveSpecies) + assertTrue(imp.speciesKnown) + assertEquals("Charizard", imp.member.speciesName) + assertEquals("Blaze", imp.member.ability) + assertEquals("Choice Scarf", imp.member.item) + } + + @Test + fun `Happiness, Pokeball, Hidden Power and Dynamax Level lines are ignored, not treated as species`() { + val paste = listOf( + "Snorlax @ Leftovers", + "Happiness: 0", + "Pokeball: Friend Ball", + "Hidden Power: Ice", + "Dynamax Level: 10", + "Gigantamax: Yes", + "- Tackle", + ).joinToString("\n") + val imp = parseShowdownBlock(paste, ::resolveMove, ::resolveSpecies) + assertTrue(imp.speciesKnown) + assertEquals("Snorlax", imp.member.speciesName) + } + + @Test + fun `strips a nickname wrapper from the species line, real Showdown nickname format`() { + val paste = listOf("Volt Turtle (Pikachu) @ Light Ball", "- Thunderbolt").joinToString("\n") + val imp = parseShowdownBlock(paste, ::resolveMove, ::resolveSpecies) + assertTrue(imp.speciesKnown) + assertEquals("Pikachu", imp.member.speciesName) + assertEquals("Light Ball", imp.member.item) + } + + @Test + fun `strips a trailing gender marker from the species line`() { + val paste = listOf("Pikachu (F) @ Light Ball", "- Thunderbolt").joinToString("\n") + val imp = parseShowdownBlock(paste, ::resolveMove, ::resolveSpecies) + assertTrue(imp.speciesKnown) + assertEquals("Pikachu", imp.member.speciesName) + } + + @Test + fun `Trait line is a legacy alias for Ability`() { + val paste = listOf("Charizard", "Trait: Blaze", "- Flamethrower").joinToString("\n") + val imp = parseShowdownBlock(paste, ::resolveMove, ::resolveSpecies) + assertEquals("Blaze", imp.member.ability) + } + + @Test + fun `an unrecognized non-first line is ignored rather than overwriting the species`() { + val paste = listOf("Pikachu @ Light Ball", "totally unrecognized junk", "- Thunderbolt").joinToString("\n") + val imp = parseShowdownBlock(paste, ::resolveMove, ::resolveSpecies) + assertTrue(imp.speciesKnown) + assertEquals("Pikachu", imp.member.speciesName) + } } diff --git a/docs/implementation-decisions.md b/docs/implementation-decisions.md index 79a9077..1179016 100644 --- a/docs/implementation-decisions.md +++ b/docs/implementation-decisions.md @@ -925,3 +925,57 @@ findings not yet acted on. way — and the plan's own §5.4 explicitly frames "displayName match first" as the behavior to preserve, so the two-map version is the intended, not merely tolerated, semantics. + +## Showdown format compatibility (post-Phase 7) + +- **`parseShowdownBlock` mistook any unrecognized line for a new species + line, not just the block's first one.** Verified against the real + client's own grammar (`sim/teams.ts` in smogon/pokemon-showdown, + `exportSet`/`parseExportedTeamLine`): a genuine Showdown export commonly + includes `Level:`, `Shiny: Yes`, `Tera Type:`, `Happiness:`, + `Pokeball:`, `Hidden Power:`, `Dynamax Level:` and `Gigantamax: Yes` + lines, none of which this app tracks — but the old `when` block's final + branch (`!line.startsWith("#")`) treated *every* line that wasn't a + move, an `Ability:` line, or an EVs/IVs/Nature line as a fresh species + line, silently overwriting whatever had already been parsed. Pasting + almost any real-world set with a `Level:` or (very common in Gen 9) + `Tera Type:` line therefore corrupted `speciesName` to that line's text + and broke species resolution. Fixed by tracking each line's position + (`forEachIndexed`) and only ever treating index 0 as the species line — + the same `isFirstLine` split the real client's own parser uses — with + every other unrecognized line now silently ignored instead of + overwriting anything, and an explicit ignore-list for the ten field + prefixes above. +- **The species line now also strips a nickname wrapper and a trailing + gender marker**, e.g. `Volt Turtle (Pikachu) @ Light Ball` or + `Pikachu (F) @ Light Ball` — both common in real pastes (nicknamed + Pokémon, VGC sets with explicit gender) and previously left whole, + which meant `resolveSpecies` was called with a string that could never + match a real species name. Same order of operations as the real + client's `parseExportedTeamLine`: split the item off first, then strip + `" (M)"`/`" (F)"`, then unwrap `"Name (Species)"`. +- **`Trait: ` is now accepted as a legacy alias for + `Ability: `** — the real client still parses it (pre-Gen-6 + sets, and some third-party tools, still emit it) and previously it fell + into the same species-line bug above. +- **The exporter no longer writes blank `Ability:`/`EVs:`/`Nature` + placeholder lines, or a bare trailing `@ ` with no item.** These were + only ever safe to re-import because CoverDex's own parser explicitly + ignored them and the real client's line-matching happens to fail too + once each line is `.trim()`-med (`"EVs: "` trimmed no longer starts + with the 5-character prefix `"EVs: "`) — accidentally harmless, not + correct. Real Showdown's own `exportSet` omits a field's line entirely + when it's unset (`if (set.ability) …`, `if (stats.length) …`), so the + exporter now does the same: `Ability:` is only written when + `TeamMember.ability` is non-null, and EVs/IVs/Nature are omitted + outright since they are still untracked (out of scope — see + `docs/plan/native-spec.md`, "Explicitly out of scope", "EV/IV + tracking"). Existing tests asserting the old blank-line shape were + updated to assert the lines are absent instead. +- **The `# Types: fire/flying` comment line is intentionally left as + CoverDex's own extension.** Confirmed against the real parser that any + line not matching a known prefix (species/`Ability:`/`Trait:`/EVs:`/ + `IVs:`/nature/moves/the eight ignored-detail prefixes) is silently + skipped, never erroring or corrupting a later field — so this + round-trips through the real client untouched, and through CoverDex's + own parser as the type-override signal it always was. diff --git a/docs/test-plan.md b/docs/test-plan.md index ec91475..a233bc1 100644 --- a/docs/test-plan.md +++ b/docs/test-plan.md @@ -344,7 +344,14 @@ None yet. ### Known regressions -None yet. +- **Fixed 2026-09-12.** Importing a real Pokémon Showdown export containing + a `Level:`, `Tera Type:`, `Shiny: Yes`, or similar field this app doesn't + track corrupted the species (the parser mistook any unrecognized line + for a new species line) — see `docs/implementation-decisions.md`, + "Showdown format compatibility". Found by auditing the parser against + the real client's own grammar, not by manual testing. Re-run "Import + from pasted text" above with a Gen 9 set that includes `Tera Type:` to + confirm the species now imports correctly. ## Phase 6 — Release From 5bd2fbce79d92431cf5bf68f6f02802a6a14813d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 10:07:06 +0000 Subject: [PATCH 2/2] Fix test that relied on the old unconditional-blank-line export shape "exports a complete team member with all fields" never actually set an item or ability on the fixture - it only passed before because the old exporter always wrote "Species @ " and "Ability: " even when blank. Now that the exporter omits those lines entirely when unset (matching real Showdown), the fixture needs to genuinely set both fields to test what its name claims. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018Khih3hZr4mzVhb3TmDnVU --- .../coverdex/domain/showdown/ShowdownFormatTest.kt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/src/test/java/com/marcogn/coverdex/domain/showdown/ShowdownFormatTest.kt b/app/src/test/java/com/marcogn/coverdex/domain/showdown/ShowdownFormatTest.kt index 44eb5f1..5ca0343 100644 --- a/app/src/test/java/com/marcogn/coverdex/domain/showdown/ShowdownFormatTest.kt +++ b/app/src/test/java/com/marcogn/coverdex/domain/showdown/ShowdownFormatTest.kt @@ -50,10 +50,15 @@ class ShowdownFormatTest { @Test fun `exports a complete team member with all fields`() { - val m = buildMember("Charizard", PokemonType.FIRE to PokemonType.FLYING, listOf(PokemonType.FIRE, PokemonType.GROUND, PokemonType.DRAGON, PokemonType.FIRE)) + val m = buildMember( + "Charizard", + PokemonType.FIRE to PokemonType.FLYING, + listOf(PokemonType.FIRE, PokemonType.GROUND, PokemonType.DRAGON, PokemonType.FIRE), + ability = "Blaze", + ).copy(item = "Charcoal") val out = exportMemberToShowdown(m) - assertTrue(out.startsWith("Charizard @")) - assertTrue(out.contains("Ability:")) + assertTrue(out.startsWith("Charizard @ Charcoal")) + assertTrue(out.contains("Ability: Blaze")) assertTrue(out.contains("- fire-move")) assertTrue(out.contains("# Types: fire/flying")) }