garrettmills/mtgc
1
0
Fork 0
Code Issues 14 Pull Requests Releases Activity

Prefer Scryfall's representative printing for default card images

Browse Source 
Card previews were picking whichever printing happened to load first
during ingest (ORDER BY rowid over every printing, promos and foreign
versions included), so a card's default image was effectively random.
Capture the representative printing's images straight from the Oracle
Cards bulk pass instead, and prefer that over the old printings-table
guess, falling back gracefully on an older reference.db that predates
these columns.

Also add docs/printing-tracking-design.md, a design (not yet
implemented) for optionally tracking which specific printing/finish a
library or deck card is.
This commit is contained in:
Garrett Mills
2026-07-22 22:05:44 -05:00
parent aaf632a417
commit d3e8a8f25a
10 changed files with 331 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
crates/mtgc-core/src/db.rs
View File

@@ -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<Connection> {

4
crates/mtgc-core/src/decks.rs
View File

@@ -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()
))?;

8
crates/mtgc-core/src/filters.rs
View File

@@ -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()
);

96
crates/mtgc-core/src/ingest.rs
View File

@@ -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<String>,
oracle_id: Option<String>,
name: String,
#[serde(default)]
@@ -429,6 +435,8 @@ struct ScryOracle {
#[serde(default)]
keywords: Vec<String>,
#[serde(default)]
image_uris: Option<ImageUris>,
#[serde(default)]
card_faces: Option<Vec<ScryFace>>,
/// 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<Vec<String>>,
#[serde(default)]
oracle_text: Option<String>,
#[serde(default)]
image_uris: Option<ImageUris>,
}
fn load_oracle_cards(conn: &Connection, reader: impl Read, cancel: &CancelToken) -> Result<usize> {
@@ -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.

13
crates/mtgc-core/src/lib.rs
View File

@@ -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<Self> {
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.

34
crates/mtgc-core/src/library.rs
View File

@@ -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();

8
crates/mtgc-core/src/search.rs
View File

@@ -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

10
crates/mtgc-core/src/sql/reference_schema.sql
View File

@@ -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);

4
crates/mtgc-core/src/synergy.rs
View File

@@ -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(),
);

150
docs/printing-tracking-design.md Normal file
View File

@@ -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<PrintingRef>`, 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<LibraryCardCopy>`.
- `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(<preferred printing image>, oc.image_uri, <old per-printing
subquery>)`. 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`.