From beea79c76a7e6096785cbad4443562441334c5cb Mon Sep 17 00:00:00 2001 From: matt Date: Mon, 6 Oct 2025 14:12:17 -0700 Subject: [PATCH] maintenance: properly implemented preview popup for alternative cards, allowed re-selection of alternative cards --- CHANGELOG.md | 11 +- RELEASE_NOTES_TEMPLATE.md | 16 +- code/tests/test_alternatives_filters.py | 11 ++ code/web/routes/build.py | 177 +++++++++++++++++++- code/web/templates/base.html | 4 + code/web/templates/build/_alternatives.html | 18 +- 6 files changed, 214 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bce468..38847e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,16 @@ This format follows Keep a Changelog principles and aims for Semantic Versioning ## [Unreleased] ### Summary -- _No changes yet._ +- Alternative suggestions in the build wizard now surface the replacement card preview immediately and reload the list after a swap. + +### Added +- Alternatives panel includes a "New pool" button so you can request a fresh batch of suggestions without rerunning the stage. + +### Changed +- Alternative suggestion buttons expose role, mana, and rarity metadata to hover previews for better at-a-glance context. + +### Fixed +- Previewing an alternative card now shows the replacement instead of the currently slotted card, and the list refreshes automatically after choosing an alternative. ## [2.5.0] - 2025-10-06 ### Summary diff --git a/RELEASE_NOTES_TEMPLATE.md b/RELEASE_NOTES_TEMPLATE.md index 4bea7cb..d581a63 100644 --- a/RELEASE_NOTES_TEMPLATE.md +++ b/RELEASE_NOTES_TEMPLATE.md @@ -1,13 +1,13 @@ # MTG Python Deckbuilder ${VERSION} -## Summary -- _Add one-line highlights for the release._ +### Summary +- Alternative suggestions in the build wizard now surface the replacement card preview immediately and reload the list after a swap. -## Added -- _Bullet list of new features._ +### Added +- Alternatives panel includes a "New pool" button so you can request a fresh batch of suggestions without rerunning the stage. -## Changed -- _Bullet list of user-facing changes._ +### Changed +- Alternative suggestion buttons expose role, mana, and rarity metadata to hover previews for better at-a-glance context. -## Fixed -- _Bullet list of bug fixes._ +### Fixed +- Previewing an alternative card now shows the replacement instead of the currently slotted card, and the list refreshes automatically after choosing an alternative. \ No newline at end of file diff --git a/code/tests/test_alternatives_filters.py b/code/tests/test_alternatives_filters.py index 0dda908..e4762de 100644 --- a/code/tests/test_alternatives_filters.py +++ b/code/tests/test_alternatives_filters.py @@ -66,3 +66,14 @@ def test_alternatives_filters_out_commander_in_deck_and_locked(): assert 'alt commander' not in body assert 'alt in deck' not in body assert 'alt locked' not in body + assert '"owned_only":"0"' in r.text + assert 'New pool' in r.text + + +def test_alternatives_refresh_query(): + app_module = importlib.import_module('code.web.app') + client = TestClient(app_module.app) + _inject_fake_ctx(client, commander='Alt Commander', locks=['alt locked']) + r = client.get('/build/alternatives?name=Target%20Card&owned_only=0&refresh=1') + assert r.status_code == 200 + assert 'New pool' in r.text diff --git a/code/web/routes/build.py b/code/web/routes/build.py index c174e80..f24b36f 100644 --- a/code/web/routes/build.py +++ b/code/web/routes/build.py @@ -3215,7 +3215,13 @@ async def build_lock_toggle(request: Request, name: str = Form(...), locked: str @router.get("/alternatives", response_class=HTMLResponse) -async def build_alternatives(request: Request, name: str, stage: str | None = None, owned_only: int = Query(0)) -> HTMLResponse: +async def build_alternatives( + request: Request, + name: str, + stage: str | None = None, + owned_only: int = Query(0), + refresh: int = Query(0), +) -> HTMLResponse: """Suggest alternative cards for a given card name, preferring role-specific pools. Strategy: @@ -3235,6 +3241,7 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No # Owned library owned_set = owned_set_helper() require_owned = bool(int(owned_only or 0)) or bool(sess.get("use_owned_only")) + refresh_requested = bool(int(refresh or 0)) # If builder context missing, show a guidance message if not b: html = '
Start the build to see alternatives.
' @@ -3291,6 +3298,112 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No # Build role-specific pool from combined DataFrame items: list[dict] = [] + + def _clean(value: Any) -> str: + try: + if value is None: + return "" + if isinstance(value, float) and value != value: + return "" + text = str(value) + return text.strip() + except Exception: + return "" + + def _normalize_tags(raw: Any) -> list[str]: + if not raw: + return [] + if isinstance(raw, (list, tuple, set)): + return [str(t).strip() for t in raw if str(t).strip()] + if isinstance(raw, str): + txt = raw.strip() + if not txt: + return [] + if txt.startswith("[") and txt.endswith("]"): + try: + import json as _json + parsed = _json.loads(txt) + if isinstance(parsed, list): + return [str(t).strip() for t in parsed if str(t).strip()] + except Exception: + pass + return [s.strip() for s in txt.split(',') if s.strip()] + return [] + + def _meta_from_row(row_obj: Any) -> dict[str, Any]: + meta = { + "mana": "", + "rarity": "", + "role": "", + "tags": [], + "hover_simple": True, + } + if row_obj is None: + meta["role"] = _clean(used_role or "") + return meta + + def _pull(*keys: str) -> Any: + for key in keys: + try: + if isinstance(row_obj, dict): + val = row_obj.get(key) + elif hasattr(row_obj, "get"): + val = row_obj.get(key) + else: + val = getattr(row_obj, key, None) + except Exception: + val = None + if val not in (None, ""): + if isinstance(val, float) and val != val: + continue + return val + return None + + meta["mana"] = _clean(_pull("mana_cost", "manaCost", "mana", "manaValue", "cmc", "mv")) + meta["rarity"] = _clean(_pull("rarity")) + role_val = _pull("role", "primaryRole", "subRole") + if not role_val: + role_val = used_role or "" + meta["role"] = _clean(role_val) + tags_val = _pull("themeTags", "_ltags", "tags") + meta_tags = _normalize_tags(tags_val) + meta["tags"] = meta_tags + meta["hover_simple"] = not (meta["mana"] or meta["rarity"] or (meta_tags and len(meta_tags) > 0)) + return meta + + def _build_meta_map(df_obj) -> dict[str, dict[str, Any]]: + mapping: dict[str, dict[str, Any]] = {} + try: + if df_obj is None or not hasattr(df_obj, "iterrows"): + return mapping + for _, row in df_obj.iterrows(): + try: + nm_val = str(row.get("name") or "").strip() + except Exception: + nm_val = "" + if not nm_val: + continue + key = nm_val.lower() + if key in mapping: + continue + mapping[key] = _meta_from_row(row) + except Exception: + return mapping + return mapping + + def _sampler(seq: list[str], limit: int) -> list[str]: + if limit <= 0: + return [] + if len(seq) <= limit: + return list(seq) + rng = getattr(b, "rng", None) + try: + if rng is not None: + return rng.sample(seq, limit) if len(seq) >= limit else list(seq) + import random as _rnd # type: ignore + return _rnd.sample(seq, limit) if len(seq) >= limit else list(seq) + except Exception: + return list(seq[:limit]) used_role = role if isinstance(role, str) and role else None # Promote to 'land' role when the seed card is a land (regardless of stored role) try: @@ -3314,8 +3427,9 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No # Use a cache key that includes the exclusions version and deck fingerprint cache_key = (name_l, commander_l, used_role or "_fallback_", require_owned, alts_exclude_v, deck_fp) - # Disable caching for land alternatives to keep randomness per request - cached = None if used_role == 'land' else _alts_get_cached(cache_key) + cached = None + if used_role != 'land' and not refresh_requested: + cached = _alts_get_cached(cache_key) if cached is not None: return HTMLResponse(cached) @@ -3326,8 +3440,8 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No "require_owned": require_owned, "items": _items, }) - # Skip caching when used_role == land for per-call randomness - if used_role != 'land': + # Skip caching when used_role == land or refresh requested for per-call randomness + if used_role != 'land' and not refresh_requested: try: _alts_set_cached(cache_key, html_str) except Exception: @@ -3525,6 +3639,7 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No pool = bu.prefer_owned_first(pool, {str(n).lower() for n in getattr(b, "owned_card_names", set())}) except Exception: pass + row_meta = _build_meta_map(pool) # Land role: random 12 from top 60-100 window if used_role == 'land': import random as _rnd @@ -3562,11 +3677,16 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No continue if nm_lc == name_l or (in_deck and nm_lc in in_deck): continue + meta = row_meta.get(nm_lc) or _meta_from_row(None) items.append({ 'name': display_map.get(nm_lc, orig), 'name_lower': nm_lc, 'owned': is_owned, - 'tags': [] + 'tags': meta.get('tags') or [], + 'role': meta.get('role', ''), + 'mana': meta.get('mana', ''), + 'rarity': meta.get('rarity', ''), + 'hover_simple': bool(meta.get('hover_simple', True)), }) if items: return _render_and_cache(items) @@ -3578,17 +3698,29 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No except Exception: lower_pool = [] display_map = _display_map_for(set(lower_pool)) - for nm_l in lower_pool: + iteration_order = lower_pool + if refresh_requested and len(lower_pool) > 12: + window_size = min(len(lower_pool), 30) + window = lower_pool[:window_size] + sampled = _sampler(window, min(window_size, 12)) + seen_sampled = set(sampled) + iteration_order = sampled + [nm for nm in lower_pool if nm not in seen_sampled] + for nm_l in iteration_order: is_owned = (nm_l in owned_set) if require_owned and not is_owned: continue if nm_l == name_l or (in_deck and nm_l in in_deck): continue + meta = row_meta.get(nm_l) or _meta_from_row(None) items.append({ "name": display_map.get(nm_l, nm_l), "name_lower": nm_l, "owned": is_owned, - "tags": [], + "tags": meta.get("tags") or [], + "role": meta.get("role", ""), + "mana": meta.get("mana", ""), + "rarity": meta.get("rarity", ""), + "hover_simple": bool(meta.get("hover_simple", True)), }) if len(items) >= 12: break @@ -3633,6 +3765,20 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No def _owned(nm: str) -> bool: return nm in owned_set candidates.sort(key=lambda x: (-x[1], 0 if _owned(x[0]) else 1, x[0])) + if refresh_requested and len(candidates) > 1: + name_sequence = [nm for nm, _score in candidates] + sampled_names = _sampler(name_sequence, min(len(name_sequence), 10)) + sampled_set = set(sampled_names) + reordered: list[tuple[str, int]] = [] + for nm in sampled_names: + for cand_nm, cand_score in candidates: + if cand_nm == nm: + reordered.append((cand_nm, cand_score)) + break + for cand_nm, cand_score in candidates: + if cand_nm not in sampled_set: + reordered.append((cand_nm, cand_score)) + candidates = reordered pool_lower = {nm for (nm, _s) in candidates} display_map = _display_map_for(pool_lower) seen = set() @@ -3648,6 +3794,10 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No "name_lower": nm, "owned": is_owned, "tags": list(tags_idx.get(nm) or []), + "role": "", + "mana": "", + "rarity": "", + "hover_simple": True, }) if len(items) >= 10: break @@ -3657,7 +3807,7 @@ async def build_alternatives(request: Request, name: str, stage: str | None = No @router.post("/replace", response_class=HTMLResponse) -async def build_replace(request: Request, old: str = Form(...), new: str = Form(...)) -> HTMLResponse: +async def build_replace(request: Request, old: str = Form(...), new: str = Form(...), owned_only: str = Form("0")) -> HTMLResponse: """Inline replace: swap `old` with `new` in the current builder when possible, and suppress `old` from future alternatives. Falls back to lock-and-rerun guidance if no active builder is present. @@ -3668,6 +3818,8 @@ async def build_replace(request: Request, old: str = Form(...), new: str = Form( n_disp = str(new).strip() o = o_disp.lower() n = n_disp.lower() + owned_only_flag = str(owned_only or "").strip().lower() + owned_only_int = 1 if owned_only_flag in {"1", "true", "yes", "on"} else 0 # Maintain locks to bias future picks and enforcement locks = set(sess.get("locks", [])) @@ -3762,6 +3914,12 @@ async def build_replace(request: Request, old: str = Form(...), new: str = Form( # Success panel and OOB updates (refresh compliance panel) # Compute ownership of the new card for UI badge update is_owned = (n in owned_set_helper()) + refresh = ( + '' + ) html = ( '
' f'
Replaced {o_disp} with {new_key}.
' @@ -3769,6 +3927,7 @@ async def build_replace(request: Request, old: str = Form(...), new: str = Form( '
' '' '
' + + refresh + '
' ) # Inline mutate the nearest card tile to reflect the new card without a rerun diff --git a/code/web/templates/base.html b/code/web/templates/base.html index dafe0f5..f935e4d 100644 --- a/code/web/templates/base.html +++ b/code/web/templates/base.html @@ -1204,6 +1204,10 @@ document.addEventListener('mousemove', move); function getCardFromEl(el){ if(!el) return null; + if(el.closest){ + var altBtn = el.closest('.alts button[data-card-name]'); + if(altBtn){ return altBtn; } + } // If inside flip button var btn = el.closest && el.closest('.dfc-toggle'); if(btn) return btn.closest('.card-sample, .commander-cell, .commander-thumb, .commander-card, .card-tile, .candidate-tile, .card-preview, .stack-card'); diff --git a/code/web/templates/build/_alternatives.html b/code/web/templates/build/_alternatives.html index ec16038..36e6ee4 100644 --- a/code/web/templates/build/_alternatives.html +++ b/code/web/templates/build/_alternatives.html @@ -4,12 +4,16 @@ ] #}
-
+
Alternatives {% set toggle_q = '0' if require_owned else '1' %} {% set toggle_label = 'Owned only: On' if require_owned else 'Owned only: Off' %} - +
+ + +
{% if not items or items|length == 0 %}
No alternatives found{{ ' (owned only)' if require_owned else '' }}.
@@ -21,9 +25,13 @@ {% set tags = (it.tags or []) %}
  • {{ badge }} -