Skip to content

Commit baa1938

Browse files
committed
media: four defects in asset detection and claiming
Case: the scan compared addon folder names exactly as stored, so PUPVideos - the casing PinUP Popper writes - was not detected, while the API lowercased and found it. One table, two answers. The scan already folded case for .directb2s and .ini three lines above. Claiming: --claim-user-media only checked medias/<canonical name>, so a hand-placed wheel.jpg, a spec-named "(Wheel) <folder>.png" or a file at the folder root was never claimed and stayed replaceable by the next download. It now claims what actually resolves - the file the frontend displays. The chain grew in 3.0 and this path never followed it. specs_for_table_type: the per-table-type copies were rebuilt from four fields, so every copy reported no spec token, no fallback kind, no set support, and the image family for video kinds. Nothing reads those off a copy today, which is the only reason it wasn't a bug. Copy the spec and change the key instead. Media download: dropped the bg_video request - vpinmediadb has never carried one at any resolution - and stopped asking for fss_video, which does not exist either. Both were silent no-ops. The backglass video and the FSS playfield video are yours to supply. PAR-16 and PAR-17 record the two a user can see.
1 parent 1b339c5 commit baa1938

6 files changed

Lines changed: 207 additions & 21 deletions

File tree

common/media_paths.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import annotations
22

3-
from dataclasses import dataclass
3+
from dataclasses import dataclass, replace
44
from pathlib import Path
55

66
# Extension families, ordered: resolution tries them in order and the first hit
@@ -73,10 +73,16 @@ def stem(self, table_type: str = "table") -> str:
7373

7474

7575
def specs_for_table_type(table_type: str = "table") -> list[MediaSpec]:
76+
"""The spec list with the playfield keys renamed for this table type.
77+
78+
Only the key changes: replace() copies the rest, so a spec from here still
79+
carries its token, extension family, fallback and set support. Rebuilding one
80+
field by field silently handed back defaults for everything not passed.
81+
"""
7682
specs: list[MediaSpec] = []
7783
for spec in MEDIA_SPECS:
7884
key = table_type if spec.key == "table" else f"{table_type}_video" if spec.key == "table_video" else spec.key
79-
specs.append(MediaSpec(key, spec.attr, spec.filename_template, spec.asset_group))
85+
specs.append(replace(spec, key=key))
8086
return specs
8187

8288

common/online/vpsdb_media.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,11 @@ def process(media_type, metadata, key, filename, default_filename):
9393
process("realdmd_color", table_media, "realdmd_color", table.realDMDColorImagePath, str(default_media_path(table.fullPathTable, "realdmd_color", self.tabletype)))
9494
process("flyer", table_media, "flyer", table.FlyerImagePath, str(default_media_path(table.fullPathTable, "flyer", self.tabletype)))
9595
process(self.tabletype, table_media.get(self.tableresolution), self.tabletype, table.TableImagePath, str(default_media_path(table.fullPathTable, self.tabletype, self.tabletype)))
96-
process("bg_video", table_media.get(self.tablevideoresolution), "bg_video", table.BGVideoPath, str(default_media_path(table.fullPathTable, "bg_video", self.tabletype)))
96+
# Videos, and only the ones the index actually carries. There has never been
97+
# a bg_video at any resolution, so the backglass video is yours to supply.
98+
# Nor is there an fss_video: under table type fss the playfield video is
99+
# simply not offered, and asking would quietly fetch nothing.
97100
process("dmd_video", table_media.get(self.tablevideoresolution), "dmd_video", table.DMDVideoPath, str(default_media_path(table.fullPathTable, "dmd_video", self.tabletype)))
98-
process(f"{self.tabletype}_video", table_media.get(self.tablevideoresolution), f"{self.tabletype}_video", table.TableVideoPath, str(default_media_path(table.fullPathTable, f"{self.tabletype}_video", self.tabletype)))
101+
if self.tabletype == "table":
102+
process("table_video", table_media.get(self.tablevideoresolution), "table_video", table.TableVideoPath, str(default_media_path(table.fullPathTable, "table_video", self.tabletype)))
99103
process("audio", table_media, "audio", table.AudioPath, str(default_media_path(table.fullPathTable, "audio", self.tabletype)))

common/tables/metadata_service.py

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from common.config_access import MediaConfig, SettingsConfig
77
from common.iniconfig import IniConfig
88
from common.jobs import JobReporter
9-
from common.media_paths import media_filename_map
9+
from common.media_paths import MEDIA_SPECS, resolve_media_files
1010
from common.tables.metaconfig import MetaConfig
1111
from common.paths import get_ini_config
1212
from common.tables.standalonescripts import StandaloneScripts
@@ -139,25 +139,45 @@ def claim_media_for_table(table, tabletype, log=None):
139139
log(f" Skipping {table.tableDirName}: no .info file")
140140
return 0
141141

142-
media_files = {
143-
key: filename
144-
for key, filename in media_filename_map(tabletype).items()
145-
if key != "audio" and (key != "fss" or tabletype == "fss")
146-
}
142+
# Claim whatever actually resolves, not just the canonical filename. Asking the
143+
# resolver is the only way a hand-placed wheel.jpg, a spec-named
144+
# "(Wheel) <build>.png" or a file at the folder root gets claimed - checking
145+
# medias/wheel.png alone left every one of them unclaimed, and therefore still
146+
# replaceable by the next media download.
147+
table_dir = table.fullPathTable
148+
medias_dir = os.path.join(table_dir, "medias")
149+
try:
150+
table_contents = {entry.name for entry in os.scandir(table_dir) if entry.is_file()}
151+
except OSError:
152+
table_contents = set()
153+
medias_contents = set()
154+
for root, _dirs, files in os.walk(medias_dir):
155+
rel = os.path.relpath(root, medias_dir)
156+
prefix = "" if rel == "." else f"{rel.replace(os.sep, '/')}/"
157+
for filename in files:
158+
medias_contents.add(f"{prefix}{filename}")
159+
160+
resolved = resolve_media_files(table_dir, table_contents, medias_contents, tabletype)
147161

148-
medias_dir = os.path.join(table.fullPathTable, "medias")
149162
meta = MetaConfig(info_path)
150163
claimed = 0
151164

152-
for media_key, filename in media_files.items():
153-
filepath = os.path.join(medias_dir, filename)
154-
if os.path.exists(filepath):
155-
existing = meta.getMedia(media_key)
156-
if existing and existing.get("Source") == "user":
157-
continue
158-
meta.addMedia(media_key, "user", filepath, "")
159-
log(f" Claimed {media_key} ({filename}) as user media")
160-
claimed += 1
165+
for spec in MEDIA_SPECS:
166+
if spec.key == "audio" or (spec.key == "fss" and tabletype != "fss"):
167+
continue
168+
path = resolved.get(spec.key)
169+
if path is None:
170+
continue
171+
# The key the download path checks before it skips a kind, so a claim here
172+
# is a claim it will honour.
173+
media_key = (tabletype if spec.key == "table"
174+
else f"{tabletype}_video" if spec.key == "table_video" else spec.key)
175+
existing = meta.getMedia(media_key)
176+
if existing and existing.get("Source") == "user":
177+
continue
178+
meta.addMedia(media_key, "user", str(path), "")
179+
log(f" Claimed {media_key} ({path.name}) as user media")
180+
claimed += 1
161181

162182
return claimed
163183

common/tables/tableparser.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ def loadTables(self, reload=False): # reload if you want to rescan the tables
6262
with os.scandir(table_dir) as entries:
6363
for entry in entries:
6464
if entry.is_dir():
65-
table_subdirs.add(entry.name)
65+
# Folded like the extension checks below, and like the
66+
# API's own listing: a folder someone named PUPVideos
67+
# holds a PUP pack whatever the shift key was doing.
68+
table_subdirs.add(entry.name.lower())
6669
continue
6770
table_contents.add(entry.name)
6871
except OSError:

docs/compatibility-3.0.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,26 @@ with a `manufacturers.json` alias map for the exceptions.
141141
had nothing to render for it. A shared root exists because a manufacturer logo is neither
142142
per-table nor per-theme. Covered by `tests/test_shared_assets.py`.
143143

144+
**PAR-16 — Addon folders are detected whatever their casing.**
145+
The library scan matched `pupvideos`, `serum`, `vni`, `music` and `medias` against the
146+
folder name exactly as stored, so a folder named `PUPVideos` — the casing PinUP Popper
147+
itself writes — was not detected. The API had always lowercased before comparing, so the
148+
same table reported a PUP pack there and none in the Manager UI and themes. The scan now
149+
folds case too. Tables whose folders are not all-lowercase will start reporting addons
150+
they always had (`pupPackExists`, `altColorExists`, `vniExists`, `altSoundExists`).
151+
*Why:* one table cannot have two answers, and the scan was already case-insensitive about
152+
`.directb2s` and `.ini` three lines away. Covered by `tests/test_media_resolution.py`.
153+
154+
**PAR-17 — Claiming user media follows the resolution chain.**
155+
`--claim-user-media` only ever looked for the fixed canonical name in `medias/`, so a
156+
hand-placed `wheel.jpg`, a spec-named `(Wheel) <folder>.png`, or a file at the folder root
157+
was never claimed — and therefore stayed replaceable by the next media download. It now
158+
claims whatever actually resolves for each kind, which is the same file the frontend
159+
displays.
160+
*Why:* the resolution chain grew in 3.0 (PAR-09/PAR-10) and the claim path didn't follow
161+
it, which made the feature silently miss the files it exists to protect. Covered by
162+
`tests/test_media_resolution.py`.
163+
144164
## Explicitly *not* exceptions
145165

146166
The theme-facing payload (`tables_json` keys, media path fields, stable values) and

tests/test_media_resolution.py

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,139 @@ def test_list_media_sets_unions_the_library_and_adds_logo(self) -> None:
263263
["colorful", "logo", "tarcisio"])
264264

265265

266+
class SpecCopyTests(unittest.TestCase):
267+
def test_a_table_type_copy_keeps_every_field_but_the_key(self) -> None:
268+
"""It used to be rebuilt from four fields, so the copies quietly reported
269+
no token, no fallback, no set support, and the image family for videos."""
270+
from common.media_paths import MEDIA_SPECS, specs_for_table_type
271+
272+
# One copy per spec, in order, so they pair up exactly.
273+
for original, copy in zip(MEDIA_SPECS, specs_for_table_type("fss"), strict=True):
274+
self.assertEqual(copy.token, original.token, original.key)
275+
self.assertEqual(copy.family, original.family, original.key)
276+
self.assertEqual(copy.fallback_kind, original.fallback_kind, original.key)
277+
self.assertEqual(copy.supports_sets, original.supports_sets, original.key)
278+
self.assertEqual(copy.attr, original.attr, original.key)
279+
280+
def test_the_fss_key_collision_stays_harmless(self) -> None:
281+
"""Under table type fss the playfield spec is renamed onto the fss key, so
282+
two specs share it. Benign only because both resolve the same filename -
283+
worth pinning, since a divergence would be silent."""
284+
from common.media_paths import media_filename_map, specs_for_table_type
285+
286+
keyed_fss = [spec for spec in specs_for_table_type("fss") if spec.key == "fss"]
287+
288+
self.assertEqual(len(keyed_fss), 2)
289+
self.assertEqual({spec.filename("fss") for spec in keyed_fss}, {"fss.png"})
290+
self.assertEqual(media_filename_map("fss")["fss"], "fss.png")
291+
292+
def test_the_video_copies_keep_the_video_family(self) -> None:
293+
from common.media_paths import VIDEO_FAMILY, specs_for_table_type
294+
295+
by_key = {spec.key: spec for spec in specs_for_table_type("table")}
296+
297+
self.assertEqual(by_key["table_video"].family, VIDEO_FAMILY)
298+
self.assertEqual(by_key["dmd_video"].family, VIDEO_FAMILY)
299+
300+
301+
class ParserCasingTests(unittest.TestCase):
302+
def test_addon_folders_are_found_whatever_their_casing(self) -> None:
303+
"""PUPVideos is the casing PinUP Popper writes, and the scanner used to
304+
miss it while the API found it - the same table, two answers."""
305+
import json
306+
307+
from common.tables.tableparser import TableParser
308+
309+
with TemporaryDirectory() as tmp:
310+
root = Path(tmp) / FOLDER
311+
(root / "medias").mkdir(parents=True)
312+
(root / f"{BUILD}.vpx").write_bytes(b"vpx")
313+
for name in ("PUPVideos", "Serum", "VNI", "Music"):
314+
(root / name).mkdir()
315+
(root / f"{FOLDER}.info").write_text(json.dumps({
316+
"Info": {"Title": "Cactus Canyon"},
317+
"VPXFile": {"filename": f"{BUILD}.vpx"},
318+
}), encoding="utf-8")
319+
320+
table = TableParser(tmp).getAllTables()[0]
321+
322+
self.assertTrue(table.pupPackExists, "PUPVideos holds a PUP pack")
323+
self.assertTrue(table.altColorExists)
324+
self.assertTrue(table.vniExists)
325+
self.assertTrue(table.musicExists)
326+
327+
328+
class ClaimTests(unittest.TestCase):
329+
"""--claim-user-media marks media as the user's so a refresh cannot replace it."""
330+
331+
def _table(self, tmp, *files):
332+
import json
333+
334+
root = Path(tmp) / FOLDER
335+
(root / "medias").mkdir(parents=True)
336+
for rel in files:
337+
path = root / rel
338+
path.parent.mkdir(parents=True, exist_ok=True)
339+
path.write_bytes(b"art")
340+
(root / f"{FOLDER}.info").write_text(json.dumps({
341+
"Info": {"Title": "Cactus Canyon"},
342+
"VPXFile": {"filename": f"{BUILD}.vpx"},
343+
}), encoding="utf-8")
344+
return root
345+
346+
def _claim(self, root):
347+
from types import SimpleNamespace
348+
349+
from common.tables.metaconfig import MetaConfig
350+
from common.tables.metadata_service import claim_media_for_table
351+
352+
table = SimpleNamespace(fullPathTable=str(root), tableDirName=FOLDER)
353+
count = claim_media_for_table(table, "table", log=lambda _m: None)
354+
return count, MetaConfig(str(root / f"{FOLDER}.info")).getConfig().get("Medias", {})
355+
356+
def test_a_canonical_file_is_claimed_as_before(self) -> None:
357+
with TemporaryDirectory() as tmp:
358+
count, medias = self._claim(self._table(tmp, "medias/wheel.png"))
359+
360+
self.assertEqual(count, 1)
361+
self.assertEqual(medias["wheel"]["Source"], "user")
362+
363+
def test_a_hand_placed_jpg_is_claimed(self) -> None:
364+
"""The resolution chain accepts the whole family; claiming has to as well."""
365+
with TemporaryDirectory() as tmp:
366+
_count, medias = self._claim(self._table(tmp, "medias/wheel.jpg"))
367+
368+
self.assertEqual(medias["wheel"]["Source"], "user")
369+
self.assertEqual(medias["wheel"]["Path"], "wheel.jpg")
370+
371+
def test_a_spec_named_file_is_claimed(self) -> None:
372+
with TemporaryDirectory() as tmp:
373+
_count, medias = self._claim(
374+
self._table(tmp, f"medias/(Wheel) {FOLDER}.png"))
375+
376+
self.assertEqual(medias["wheel"]["Source"], "user")
377+
self.assertEqual(medias["wheel"]["Path"], f"(Wheel) {FOLDER}.png")
378+
379+
def test_a_file_at_the_folder_root_is_claimed(self) -> None:
380+
with TemporaryDirectory() as tmp:
381+
_count, medias = self._claim(self._table(tmp, "bg.png"))
382+
383+
self.assertEqual(medias["bg"]["Source"], "user")
384+
385+
def test_nothing_present_claims_nothing(self) -> None:
386+
with TemporaryDirectory() as tmp:
387+
count, medias = self._claim(self._table(tmp))
388+
389+
self.assertEqual(count, 0)
390+
self.assertEqual(medias, {})
391+
392+
def test_audio_is_still_left_alone(self) -> None:
393+
with TemporaryDirectory() as tmp:
394+
_count, medias = self._claim(self._table(tmp, "medias/audio.mp3"))
395+
396+
self.assertNotIn("audio", medias)
397+
398+
266399
class ImportSideTests(unittest.TestCase):
267400
def _table(self, tmp, *files):
268401
root = Path(tmp) / FOLDER

0 commit comments

Comments
 (0)