Skip to content

fix(songs): detect added/removed files on Reload Songs/Courses#494

Draft
adstep wants to merge 1 commit into
pnn64:mainfrom
adstep:adstep/reload-detect-missing-files
Draft

fix(songs): detect added/removed files on Reload Songs/Courses#494
adstep wants to merge 1 commit into
pnn64:mainfrom
adstep:adstep/reload-detect-missing-files

Conversation

@adstep

@adstep adstep commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Why

With the default FastLoad=1, the song scan trusts the on-disk cache without verifying the song folder's directory hash. If a song folder is missing its music file at first scan, it gets cached with music_path: None, and the cache's path-existence check treats None as "present". So when the user later adds the music file and runs Reload Songs/Courses, the change is never detected. The song still won't preview or play until a folder is renamed (which changes the cache key) or the cache is wiped.

Approach

An explicit reload should always pick up on-disk changes, regardless of the FastLoad startup-speed toggle. So reload now forces cache freshness verification while startup stays fast:

  • process_song takes a force_fresh flag and computes verify_freshness = force_fresh || !fastload. Cache reads stay fast; they are just verified against the directory hash.
  • The flag is threaded through load_pack_scans, scan_and_load_songs_impl, and reload_song_dirs_impl.
  • A new reload_all_songs_with_progress_counts (force-fresh) backs both full-reload call sites (options/reload.rs and select_music.rs); startup (init.rs) keeps the fast path unchanged.

The directory-hash check is symmetric, so this also catches file removals and delete-and-re-add with the same folder name.

Performance

This was measured, not just estimated. Microbenchmark over 2000 synthetic song folders (5 files each, warm cache, single-threaded, Windows):

cache read per song
verify OFF (startup) ~263 us
verify ON (reload) ~706 us
delta ~443 us/song (~3x), ~0.44 s per 1000 songs

The extra cost is one canonicalize + read_dir + per-entry metadata per song. These are single-threaded numbers; the real reload parses in parallel, so wall-clock overhead is roughly delta / threads (~14 ms per 1000 songs on a 32-thread box). No extra chart parsing happens for unchanged songs, and startup is untouched. This is an acceptable trade for an occasional, progress-bar-driven action.

Notes for reviewers

  • Startup behavior is intentionally unchanged: restarting the game alone will not pick up a missing-then-added file; detection is deferred to an explicit reload. (Removing a previously-present asset is still caught at startup via the existing path-existence check.)
  • Two regression tests added in simfile/cache.rs cover the missing-then-added music case and a file-removal case.

Closes #493

Default FastLoad=1 made the song scan trust the cache without verifying the song folder's directory hash. A song missing its music file is cached with music_path: None, which the path-existence check treats as present, so adding the file later was never detected on reload. It only recovered after renaming a folder (new cache key) or wiping the cache.

An explicit Reload Songs/Courses now always verifies cache freshness, independent of the FastLoad startup toggle: process_song takes a force_fresh flag (verify_freshness = force_fresh || !fastload), threaded through load_pack_scans / scan_and_load_songs_impl / reload_song_dirs_impl. A new reload_all_songs_with_progress_counts (force-fresh) backs both full-reload call sites; startup stays on the fast path. The directory-hash check is symmetric, so additions and removals (including delete-and-re-add) are caught.

Closes pnn64#493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Reload Songs/Courses does not detect missing files in cached song folders

1 participant