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..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,16 +50,27 @@ 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.contains("EVs:")) - assertTrue(out.contains("Nature")) + assertTrue(out.startsWith("Charizard @ Charcoal")) + assertTrue(out.contains("Ability: Blaze")) 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 +221,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 +266,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 +293,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