diff --git a/crates/mtgc-core/src/db.rs b/crates/mtgc-core/src/db.rs index 8c05780..6ff210f 100644 --- a/crates/mtgc-core/src/db.rs +++ b/crates/mtgc-core/src/db.rs @@ -48,6 +48,26 @@ pub fn price_subquery() -> String { ) } +/// SQL expression selecting `column` ("image_uri" or "back_image_uri") for +/// the oracle card aliased `oc`, for use in a `SELECT`. +/// +/// When `has_oracle_images` (i.e. `oracle_cards.image_uri` exists in the +/// attached reference db), prefers the representative-printing image +/// captured directly onto `oracle_cards` during ingest — Scryfall's own pick +/// of a recognizable, English printing — over [`image_subquery`]'s arbitrary +/// first-by-rowid printing, so card previews don't show a random +/// promo/showcase/foreign-language version. Falls back to the old subquery +/// when the oracle card has no image of its own, or unconditionally on an +/// older reference.db built before this column shipped. +pub fn image_expr(has_oracle_images: bool, column: &str) -> String { + let fallback = image_subquery(column); + if has_oracle_images { + format!("COALESCE(NULLIF(oc.{column}, ''), {fallback})") + } else { + fallback + } +} + /// Create (if needed) and open a reference.db, applying the reference schema. /// Used by the ingester and by test fixtures. pub fn open_reference_for_build(path: &Path) -> Result { diff --git a/crates/mtgc-core/src/decks.rs b/crates/mtgc-core/src/decks.rs index 0d6d86e..bcda226 100644 --- a/crates/mtgc-core/src/decks.rs +++ b/crates/mtgc-core/src/decks.rs @@ -121,8 +121,8 @@ impl Catalog { LEFT JOIN library_cards lc ON lc.oracle_id = dc.oracle_id WHERE dc.deck_id = ?1 ORDER BY oc.name COLLATE NOCASE", - image = crate::db::image_subquery("image_uri"), - back = crate::db::image_subquery("back_image_uri"), + image = self.image_expr("image_uri"), + back = self.image_expr("back_image_uri"), price = crate::db::price_subquery(), horizontal = self.horizontal_col() ))?; diff --git a/crates/mtgc-core/src/filters.rs b/crates/mtgc-core/src/filters.rs index 887608b..6dcb024 100644 --- a/crates/mtgc-core/src/filters.rs +++ b/crates/mtgc-core/src/filters.rs @@ -103,8 +103,8 @@ impl Catalog { FROM library_cards lc JOIN {REF_DB}.oracle_cards oc ON oc.oracle_id = lc.oracle_id WHERE lc.quantity_owned > 0", - image = crate::db::image_subquery("image_uri"), - back = crate::db::image_subquery("back_image_uri"), + image = self.image_expr("image_uri"), + back = self.image_expr("back_image_uri"), price = crate::db::price_subquery(), horizontal = self.horizontal_col() ); @@ -163,8 +163,8 @@ impl Catalog { FROM {REF_DB}.oracle_cards oc LEFT JOIN library_cards lc ON lc.oracle_id = oc.oracle_id WHERE 1=1", - image = crate::db::image_subquery("image_uri"), - back = crate::db::image_subquery("back_image_uri"), + image = self.image_expr("image_uri"), + back = self.image_expr("back_image_uri"), price = crate::db::price_subquery(), horizontal = self.horizontal_col() ); diff --git a/crates/mtgc-core/src/ingest.rs b/crates/mtgc-core/src/ingest.rs index 075c483..c354d2b 100644 --- a/crates/mtgc-core/src/ingest.rs +++ b/crates/mtgc-core/src/ingest.rs @@ -412,6 +412,12 @@ where #[derive(Deserialize)] struct ScryOracle { + /// This entry's own printing id — the "representative" printing Scryfall + /// includes in the Oracle Cards bulk (its own doc: "the most up-to-date + /// recognizable" version). Captured as `oracle_cards.default_scryfall_id` + /// purely for reference; not currently read back. + #[serde(default)] + id: Option, oracle_id: Option, name: String, #[serde(default)] @@ -429,6 +435,8 @@ struct ScryOracle { #[serde(default)] keywords: Vec, #[serde(default)] + image_uris: Option, + #[serde(default)] card_faces: Option>, /// What colors of mana this card can add (lands, mana rocks, dorks). A /// top-level-only Scryfall field — not per-face, even on DFCs. @@ -452,6 +460,8 @@ struct ScryFace { colors: Option>, #[serde(default)] oracle_text: Option, + #[serde(default)] + image_uris: Option, } fn load_oracle_cards(conn: &Connection, reader: impl Read, cancel: &CancelToken) -> Result { @@ -461,8 +471,9 @@ fn load_oracle_cards(conn: &Connection, reader: impl Read, cancel: &CancelToken) let mut ins_card = conn.prepare( "INSERT OR IGNORE INTO oracle_cards (oracle_id,name,name_lower,mana_value,colors,color_identity,type_line,oracle_text, - produced_mana,produces_colorless,legalities,themes,edhrec_rank,horizontal) - VALUES (?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12,?13,?14)", + produced_mana,produces_colorless,legalities,themes,edhrec_rank,horizontal, + image_uri,back_image_uri,default_scryfall_id) + VALUES (?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12,?13,?14,?15,?16,?17)", )?; let mut ins_fts = conn.prepare("INSERT INTO oracle_names_fts(name,oracle_id) VALUES (?1,?2)")?; @@ -560,6 +571,31 @@ fn load_oracle_cards(conn: &Connection, reader: impl Read, cancel: &CancelToken) .any(|w| w == "Battle"); let horizontal = is_battle || c.layout == "planar"; + // Representative-printing images: top-level for single-faced + // cards, else the front/back faces — same fallback shape as + // `load_printings` below, since Oracle Cards entries mirror a + // printing's image fields. + let image_uri = c + .image_uris + .as_ref() + .and_then(|i| i.normal.clone()) + .or_else(|| { + c.card_faces + .as_ref() + .and_then(|f| f.first()) + .and_then(|f| f.image_uris.as_ref()) + .and_then(|i| i.normal.clone()) + }) + .unwrap_or_default(); + let back_image_uri = c + .card_faces + .as_ref() + .and_then(|f| f.get(1)) + .and_then(|f| f.image_uris.as_ref()) + .and_then(|i| i.normal.clone()) + .unwrap_or_default(); + let default_scryfall_id = c.id.clone().unwrap_or_default(); + let changed = ins_card.execute(params![ oracle_id, c.name, @@ -575,6 +611,9 @@ fn load_oracle_cards(conn: &Connection, reader: impl Read, cancel: &CancelToken) themes, c.edhrec_rank, horizontal as i64, + image_uri, + back_image_uri, + default_scryfall_id, ])?; // Only emit satellite rows the first time we see this oracle_id. if changed == 1 { @@ -1072,6 +1111,59 @@ mod tests { assert!(!horizontal("oid-normal")); } + #[test] + fn captures_representative_printing_image_from_oracle_cards() { + // Oracle Cards entries carry Scryfall's own pick of a recognizable + // printing's images — top-level for single-faced cards, per-face for + // DFCs (mirrors how `load_printings` reads a `printings` entry). + let oracle = r#"[ + {"id":"scry-normal","oracle_id":"oid-normal","name":"Grizzly Bears", + "layout":"normal","cmc":2.0,"type_line":"Creature — Bear", + "color_identity":["G"],"keywords":[], + "image_uris":{"normal":"http://img/normal"}}, + {"id":"scry-dfc","oracle_id":"oid-dfc","name":"Delver of Secrets // Insectile Aberration", + "layout":"transform","cmc":1.0,"color_identity":["U"],"keywords":[], + "card_faces":[ + {"type_line":"Creature — Human Wizard","image_uris":{"normal":"http://img/front"}}, + {"type_line":"Creature — Human Insect","image_uris":{"normal":"http://img/back"}} + ]}, + {"oracle_id":"oid-noimg","name":"No Image Test","layout":"normal","cmc":0.0, + "type_line":"Land","color_identity":[],"keywords":[]} + ]"#; + + let dir = tempfile::tempdir().unwrap(); + let dest = dir.path().join("reference.db"); + let conn = open_reference_for_build(&dest).unwrap(); + load_oracle_cards(&conn, oracle.as_bytes(), &CancelToken::new()).unwrap(); + + let images = |id: &str| -> (String, String, String) { + conn.query_row( + "SELECT image_uri, back_image_uri, default_scryfall_id FROM oracle_cards WHERE oracle_id=?1", + params![id], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), + ) + .unwrap() + }; + + assert_eq!( + images("oid-normal"), + ( + "http://img/normal".to_string(), + "".to_string(), + "scry-normal".to_string() + ) + ); + assert_eq!( + images("oid-dfc"), + ( + "http://img/front".to_string(), + "http://img/back".to_string(), + "scry-dfc".to_string() + ) + ); + assert_eq!(images("oid-noimg"), ("".to_string(), "".to_string(), "".to_string())); + } + #[test] fn captures_back_face_image_for_double_faced_cards() { // Transform/MDFC cards: two `card_faces`, each with its own image. diff --git a/crates/mtgc-core/src/lib.rs b/crates/mtgc-core/src/lib.rs index a8cac4f..2dc5ca1 100644 --- a/crates/mtgc-core/src/lib.rs +++ b/crates/mtgc-core/src/lib.rs @@ -39,6 +39,10 @@ pub struct Catalog { /// Cached at open time (one pragma query) since it's consulted by nearly /// every card-hydrating query — see [`Catalog::horizontal_col`]. pub(crate) has_horizontal_column: bool, + /// Whether `oracle_cards.image_uri` (and its `back_image_uri`/ + /// `default_scryfall_id` siblings) exist in the attached reference db. + /// Cached at open time — see [`Catalog::image_expr`]. + pub(crate) has_oracle_images: bool, } impl Catalog { @@ -47,7 +51,8 @@ impl Catalog { pub fn open(library_path: &Path, reference_path: &Path) -> Result { let conn = db::open(library_path, reference_path)?; let has_horizontal_column = db::has_reference_column(&conn, "oracle_cards", "horizontal")?; - Ok(Self { conn, has_horizontal_column }) + let has_oracle_images = db::has_reference_column(&conn, "oracle_cards", "image_uri")?; + Ok(Self { conn, has_horizontal_column, has_oracle_images }) } /// SQL expression for `oracle_cards.horizontal`, for use in a `SELECT` @@ -65,6 +70,12 @@ impl Catalog { } } + /// SQL expression selecting `column` ("image_uri" or "back_image_uri") + /// for the oracle card aliased `oc`. See [`db::image_expr`]. + pub(crate) fn image_expr(&self, column: &str) -> String { + db::image_expr(self.has_oracle_images, column) + } + /// Build (or rebuild) the in-memory fuzzy name index from the attached /// reference db. Cheap enough to hold in app state and rebuild after a /// reference-data update. diff --git a/crates/mtgc-core/src/library.rs b/crates/mtgc-core/src/library.rs index 0815fd3..18ed915 100644 --- a/crates/mtgc-core/src/library.rs +++ b/crates/mtgc-core/src/library.rs @@ -85,8 +85,8 @@ impl Catalog { JOIN {REF_DB}.oracle_cards oc ON oc.oracle_id = lc.oracle_id WHERE lc.quantity_owned > 0 ORDER BY oc.name COLLATE NOCASE", - image = crate::db::image_subquery("image_uri"), - back = crate::db::image_subquery("back_image_uri"), + image = self.image_expr("image_uri"), + back = self.image_expr("back_image_uri"), price = crate::db::price_subquery(), horizontal = self.horizontal_col() ))?; @@ -314,6 +314,36 @@ mod tests { assert_eq!(stats.color_counts, [0, 0, 1, 0, 3]); } + #[test] + fn oracle_cards_image_wins_over_the_printings_pick() { + // The bug this guards: `printings` can hold an arbitrary (promo, + // showcase, foreign-language...) printing per card. `oracle_cards`'s + // own image — Scryfall's representative-printing pick, captured at + // ingest — must win when present, rather than whatever `printings` + // subquery would otherwise resolve to (first by rowid). + let fx = test_support::make(); + let cat = &fx.catalog; + cat.connection() + .execute( + "UPDATE ref.printings SET image_uri = 'http://img/promo-variant' \ + WHERE oracle_id = 'sol-ring'", + [], + ) + .unwrap(); + cat.connection() + .execute( + "UPDATE ref.oracle_cards SET image_uri = 'http://img/normal' \ + WHERE oracle_id = 'sol-ring'", + [], + ) + .unwrap(); + cat.add_owned("sol-ring", 1).unwrap(); + + let lib = cat.list_library().unwrap(); + let sol = lib.iter().find(|c| c.card.oracle_id == "sol-ring").unwrap(); + assert_eq!(sol.card.image_uri, "http://img/normal"); + } + #[test] fn back_image_uri_is_plumbed_through() { let fx = test_support::make(); diff --git a/crates/mtgc-core/src/search.rs b/crates/mtgc-core/src/search.rs index d3e6c93..3de8acc 100644 --- a/crates/mtgc-core/src/search.rs +++ b/crates/mtgc-core/src/search.rs @@ -39,13 +39,17 @@ impl NameIndex { } else { "0" }; + // See `Catalog::image_expr` / `db::image_expr` — prefer the oracle + // card's own representative-printing image, falling back to the old + // arbitrary-first-printing subquery for an older reference.db. + let has_oracle_images = crate::db::has_reference_column(conn, "oracle_cards", "image_uri")?; let mut stmt = conn.prepare(&format!( "SELECT oc.oracle_id, oc.name, oc.mana_value, oc.colors, oc.color_identity, oc.type_line, {image}, {back}, oc.produced_mana, oc.produces_colorless, oc.legalities, {price}, {horizontal} FROM {REF_DB}.oracle_cards oc", - image = crate::db::image_subquery("image_uri"), - back = crate::db::image_subquery("back_image_uri"), + image = crate::db::image_expr(has_oracle_images, "image_uri"), + back = crate::db::image_expr(has_oracle_images, "back_image_uri"), price = crate::db::price_subquery() ))?; let entries = stmt diff --git a/crates/mtgc-core/src/sql/reference_schema.sql b/crates/mtgc-core/src/sql/reference_schema.sql index 26ad7e7..99d6bd6 100644 --- a/crates/mtgc-core/src/sql/reference_schema.sql +++ b/crates/mtgc-core/src/sql/reference_schema.sql @@ -16,7 +16,15 @@ CREATE TABLE IF NOT EXISTS oracle_cards ( legalities INTEGER NOT NULL DEFAULT 0, -- 2 bits/format x 8 formats, see model::LEGALITY_FORMATS themes INTEGER NOT NULL DEFAULT 0, -- archetype bitmask, see synergy::ARCHETYPES edhrec_rank INTEGER, -- Scryfall EDHREC popularity rank; NULL if unranked - horizontal INTEGER NOT NULL DEFAULT 0 -- boolean: printed/displayed sideways (Scryfall layout "battle"/"planar") + horizontal INTEGER NOT NULL DEFAULT 0, -- boolean: printed/displayed sideways (Scryfall layout "battle"/"planar") + -- Representative printing's images, captured from the Oracle Cards bulk + -- (Scryfall's own pick of "the most up-to-date recognizable" English + -- printing) — preferred over `printings` for display so previews don't + -- show an arbitrary promo/showcase/foreign-language printing. Empty when + -- absent (falls back to `printings`). + image_uri TEXT NOT NULL DEFAULT '', + back_image_uri TEXT NOT NULL DEFAULT '', + default_scryfall_id TEXT NOT NULL DEFAULT '' ); CREATE INDEX IF NOT EXISTS idx_oracle_name_lower ON oracle_cards(name_lower); CREATE INDEX IF NOT EXISTS idx_oracle_ci ON oracle_cards(color_identity); diff --git a/crates/mtgc-core/src/synergy.rs b/crates/mtgc-core/src/synergy.rs index 8a36d42..16c9f6a 100644 --- a/crates/mtgc-core/src/synergy.rs +++ b/crates/mtgc-core/src/synergy.rs @@ -317,8 +317,8 @@ impl Catalog { WHERE oc.oracle_id NOT IN (SELECT oracle_id FROM deck_cards WHERE deck_id = ?) AND (oc.color_identity & ?) = 0 AND (", - image = crate::db::image_subquery("image_uri"), - back = crate::db::image_subquery("back_image_uri"), + image = self.image_expr("image_uri"), + back = self.image_expr("back_image_uri"), price = crate::db::price_subquery(), horizontal = self.horizontal_col(), ); diff --git a/docs/printing-tracking-design.md b/docs/printing-tracking-design.md new file mode 100644 index 0000000..e57bbb6 --- /dev/null +++ b/docs/printing-tracking-design.md @@ -0,0 +1,150 @@ +# Design: optional per-printing tracking + +Status: **not implemented** — design only, for a future session. + +## Context + +`mtgc` currently identifies every owned/decked card purely by `oracle_id` +(one row + a quantity column in both `library_cards` and `deck_cards`). There +is no way to record *which printing* a physical copy is, or to own multiple +copies of the same card across different printings (e.g. one non-foil `2X2` +copy and one foil `PRNA` promo copy of the same card). + +Notably, the schema already anticipated this: `library_card_copies` +([0001_init.sql](../crates/mtgc-core/src/sql/library/0001_init.sql)) was +defined at the very start of the project as "optional per-physical-copy +printing detail (the 'hybrid' identity)" — but it has never been read or +written by any code path. This document designs activating it. + +This was split out from the "normal default card images" fix (see the ingest +change in `crates/mtgc-core/src/ingest.rs` / `db.rs` that made the *default* +image for a card come from Scryfall's representative Oracle Cards printing). +That fix solves "the default preview looks weird"; this document solves "I +want to track/display a *specific* printing for a specific copy or deck." + +**Decision from the requesting conversation:** a tracked copy should record +printing **+ finish** (nonfoil/foil/etched). Condition (NM/LP/etc.) is +deferred — the dormant table already has a `condition` column, so adding it +later is a compatible follow-up, not a redesign. + +## Identity model — "hybrid" + +- `library_cards` **stays authoritative** for totals: `oracle_id` + + `quantity_owned`. Every existing oracle-level feature (legality checks, deck + analysis, synergy suggestions, over-assignment warnings) keeps working + completely untouched. +- `library_card_copies` becomes *optional* annotations: each row means "N of + my copies of this card are this specific printing + finish." Copies are a + subset of `quantity_owned` — a card can have some copies pinned to a + printing and the rest left unspecified. The UI must surface that remainder + (e.g. "3 owned · 1 tracked (2X2 foil) · 2 unspecified") rather than forcing + every copy to be classified. +- Decks get a per-card **display preference** — which printing's art to show + for that deck slot — independent of quantity/legality. This is deliberately + *not* tied to a specific owned copy; it's just "when I look at this deck, + show me this version." + +## Schema / migrations + +### library.db (append-only migrations, per the existing convention in `db.rs`) + +- **`0005_copy_quantity_finish.sql`** — add `quantity INTEGER NOT NULL DEFAULT 1` + to `library_card_copies`. Without this, the dormant table is one-row-per-copy, + which is awkward for e.g. "4 copies, all the same 2X2 nonfoil printing." A + quantity column lets one row represent a group of identical copies. (The + `finish` column already exists in the original table; `condition` stays, + unused for now.) +- **`0006_deck_card_printing.sql`** — add `preferred_scryfall_id TEXT` (nullable) + to `deck_cards`. Pure display preference, no FK enforcement needed (printings + live in the read-only, rebuildable reference.db, not library.db — a dangling + reference after a re-ingest should just fall back to the default image, not + error). + +### reference.db (edit `reference_schema.sql` directly — rebuilt, not migrated) + +- Add `lang TEXT NOT NULL DEFAULT ''` and `released_at TEXT NOT NULL DEFAULT ''` + to `printings`, for picker labels ("English, 2X2, 2022-07-08") and stable + chronological ordering. Both fields are already present on Scryfall printing + objects, so `load_printings` just needs to read two more fields — no new + ingest pass. + +## Core (`mtgc-core`) + +- New `PrintingRef` model: `scryfall_id, set_code, collector_number, lang, + finishes, rarity, image_uri, back_image_uri, price_usd, released_at`. +- `Catalog::list_printings(oracle_id) -> Vec`, ordered by + `(released_at DESC, rowid)`, for the picker UI. +- Library copy CRUD in `library.rs`: + - `set_copy(oracle_id, scryfall_id, finish, quantity)` — upsert a copy group. + - `list_copies(oracle_id) -> Vec`. + - `remove_copy(copy_id)`. + - Validation: sum of copy quantities for an `oracle_id` must stay + `<= quantity_owned`; reducing `quantity_owned` below the tracked sum should + either clamp copies or surface a conflict — pick one and document it before + implementing (leaning toward clamping the newest/most-recently-edited copy + row, since silently deleting user-entered printing data is worse than + losing precise quantity). +- Deck: `set_deck_card_printing(deck_id, oracle_id, scryfall_id | null)`. +- **Display resolution:** extend the image-fallback chain added for the + default-image fix so a deck card with `preferred_scryfall_id` set resolves + its image from that specific printing first: + `COALESCE(, oc.image_uri, )`. No component changes needed — everything still just reads + `card.image_uri` off the `CardRef`/`DeckEntry` it's given. + +## Commands + `src/api.ts` + +- Tauri commands: `list_printings(oracle_id)`, `set_library_copy(...)`, + `list_library_copies(oracle_id)`, `remove_library_copy(copy_id)`, + `set_deck_card_printing(deck_id, oracle_id, scryfall_id)`. +- `src/api.ts`: matching invoke wrappers; new `PrintingRef` and + `LibraryCardCopy` TS interfaces; extend `DeckEntry` with an optional + `preferred_scryfall_id`. + +## Frontend (`src/`) + +- New `PrintingPicker` modal (in `components.tsx`): given an `oracle_id`, fetch + printings and render a thumbnail grid with set / language / finish / price + labels; `onPick(scryfallId)`. +- **DeckView** ([src/views/DeckView.tsx](../src/views/DeckView.tsx)): a + per-card "Choose printing" affordance opens the picker and calls + `set_deck_card_printing`; the deck's thumbnail/preview then reflects that + printing. +- **LibraryBrowser** / **CardEntry**: a per-card "Versions" affordance to + assign printing + finish + quantity to copies; show the + specified-vs-unspecified breakdown next to the existing quantity stepper. + +## Open questions to settle before implementing + +1. Should the library *list/tile thumbnail* follow a pinned copy's printing + (and if a card has multiple different tracked printings, which one wins?), + or should the thumbnail always show the oracle default and per-copy art + only appear in a detail/expanded view? Recommend the latter — simpler, and + avoids an arbitrary tie-break in the common list view. +2. Should decklist import/export (`decklist.rs`) round-trip printing + preferences (many decklist formats support a `(SET) 123` suffix)? Out of + scope for the initial cut; note it as a natural follow-up once the core + model exists. +3. Quantity-vs-copies conflict resolution (see `set_copy` validation above) — + decide the exact clamping behavior before writing the command, and add a + unit test that pins it down. + +## Verification plan (once implemented) + +- Rust unit tests: copy CRUD + quantity clamping; preferred-printing image + resolution (deck card with `preferred_scryfall_id` set vs. unset vs. + dangling after a re-ingest). +- Command round-trip smoke test through `src-tauri`. +- Manual: assign a printing to a deck card and confirm the deck preview/thumbnail + changes to that art; record two copies of one card under two different + printings and confirm the library view shows the specified/unspecified + breakdown correctly. + +## Critical files + +`crates/mtgc-core/src/sql/library/0005_*.sql`, `0006_*.sql`, +`crates/mtgc-core/src/sql/reference_schema.sql`, `crates/mtgc-core/src/ingest.rs`, +`crates/mtgc-core/src/library.rs`, `crates/mtgc-core/src/decks.rs`, +`crates/mtgc-core/src/model.rs`, `src-tauri/src/commands.rs`, `src/api.ts`, +`src/components.tsx`, `src/views/DeckView.tsx`, `src/views/LibraryBrowser.tsx`, +`src/views/CardEntry.tsx`.