Skip to content

CardDb: make loadCard idempotent (fix latent duplicate-PaperCard bug) - #11586

Open
shoeless wants to merge 1 commit into
Card-Forge:masterfrom
shoeless:fix/carddb-loadcard-idempotent
Open

CardDb: make loadCard idempotent (fix latent duplicate-PaperCard bug)#11586
shoeless wants to merge 1 commit into
Card-Forge:masterfrom
shoeless:fix/carddb-loadcard-idempotent

Conversation

@shoeless

@shoeless shoeless commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Problem

CardDb.loadCard is not idempotent: addSetCard unconditionally increments the art index and appends a fresh PaperCard. A redundant call for a (name, set) printing that's already loaded therefore duplicates that PaperCard in allCardsByName.

Fix

Skip when that exact (name, set) printing already exists, via a new side-effect-free hasPrintingInSet check. Guarding on card-name presence alone would break the "add a printing from a not-yet-loaded set" path (StaticData.getOrLoadCommonCard), where the card can already exist under other sets while the requested set's printing is still missing — so the guard is keyed on the specific edition.

Why "any printing present" is a safe guard despite multi-printing sets

A set can contain multiple printings of the same name (CardEdition.getCardInSet returns a List<EditionEntry> — basic-land arts, showcase variants; that's what artIndex is for). The guard is still sound because loadCard is all-or-nothing per (name, set): addFromSetByName loops over every EditionEntry of that name in the set and addSetCards each one. addSetCard has exactly two callers — that loop, and the eager full-set path in initialize — so no code path ever adds a subset of a name's printings for a set. Any-printing-present therefore implies the (name, set) load already ran, art variants included. (Entries excluded by the functional-variant filter are excluded on every call equally, so the invariant holds.)

Single file, forge-core only.

🤖 Generated with Claude Code

@Hanmac

Hanmac commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

artIndex is on my list to be removed anyway

I just need to find a better way to rename the image files #7771

@shoeless
shoeless force-pushed the fix/carddb-loadcard-idempotent branch from 42e35cb to a4bb8e1 Compare August 11, 2026 05:16
@shoeless

Copy link
Copy Markdown
Contributor Author

Good to know — this guard doesn't lean on artIndex, only on loadCard adding all of a (name, set)'s printings in one call, so it should carry over cleanly (or simplify) when artIndex goes away with #7771.

Pushed a small hardening while re-checking it: the existing-printing check now also matches the card's rules name, since a lookup name can be shared across different cards (CopyFaceFrom alt faces, flavor names) — no stock data hits that today, but custom content could.

To be clear, the duplication itself isn't artIndex's doing — allCardsByName is an ArrayList-backed multimap and nothing in the add path checks for an existing entry, so a redundant call appends regardless; artIndex only determines how the duplicates are labeled. The guard becomes droppable once that rework makes the add path uniqueness-enforcing.

@shoeless
shoeless force-pushed the fix/carddb-loadcard-idempotent branch from a4bb8e1 to 36502d6 Compare August 11, 2026 05:32
Comment thread forge-core/src/main/java/forge/card/CardDb.java Outdated
loadCard is not idempotent: addSetCard unconditionally increments the art index
and appends a fresh PaperCard, so a redundant call for a (name, set) printing
already loaded duplicates it in allCardsByName (and, since initialize() resets
artIds per edition, the duplicates repeat artIndex 1..n — inflating the art
counts CardPool's random-art selection uses). Skip when that exact card's
(name, set) printing already exists.

The guard matches the card's rules name, not just the lookup name: names can be
shared across cards (CopyFaceFrom alt faces, flavor names), and it only applies
to resolvable set codes — the null/UNKNOWN-set path is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shoeless
shoeless force-pushed the fix/carddb-loadcard-idempotent branch from 36502d6 to 0afbbe1 Compare August 11, 2026 15:46
@Jetz72 Jetz72 added the CardDB Loading, storing, and searching CardRules and PaperCard data label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CardDB Loading, storing, and searching CardRules and PaperCard data

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants