alucryd / oxyromon

Rusty ROM OrgaNizer
Other
120 stars 13 forks source link

DAT update fails with "Error while finding romfile with" #140

Closed docbobo closed 3 months ago

docbobo commented 3 months ago

I am trying to update some DAT Files, but that always fails with

thread 'main' panicked at src/database.rs:2509:25:
Error while finding romfile with id 11425
stack backtrace:
   0: rust_begin_unwind
             at /rustc/7cf61ebde7b22796c69757901dd346d0fe70bd97/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/7cf61ebde7b22796c69757901dd346d0fe70bd97/library/core/src/panicking.rs:72:14
   2: oxyromon::database::find_romfile_by_id::{{closure}}
   3: oxyromon::import_dats::reimport_orphan_romfiles::{{closure}}
   4: oxyromon::import_dats::import_dat::{{closure}}
   5: oxyromon::download_dats::download_redump_dat::{{closure}}
   6: oxyromon::main::{{closure}}
   7: tokio::runtime::park::CachedParkThread::block_on
   8: tokio::runtime::context::runtime::enter_runtime
   9: tokio::runtime::runtime::Runtime::block_on
  10: oxyromon::main

I tried deleting the particular ROM, followed by a oyxromon purge-roms -o which seems to remove the entry. Rerunning the update command then results in another ROM triggering the panic.

Is update working at all with the latest version?

docbobo commented 3 months ago

Okay, this seems to be fixed by adding a orphan_romfile_ids.dedup(); in import_dats.rs#L568 - for me orphan_romfile_ids was containing multiple instances of the same id which would then result in the above panic.

alucryd commented 3 months ago

Thanks for both reports :) I will backport your fixes in the develop branch and add unit tests to cover these cases, thank you very much!

alucryd commented 3 months ago

The error was a bit more widespread, for instance it would also choke on reimporting orphan CHD with multiple tracks (for the same reason, duplicated orphan ids), and even CHD files that are still valid but have been renamed (for a different reason).

Just had the issue with ff9 usa which was renamed over at redump. The current fix is still not ideal because now it will trash the CHD when the CUE is invalid, but at least it doesn't crash anymore. I would argue that people need to download the updated CUE anyway, they might as well reimport both files together so I'll leave that on the back-burner for now.

In any case, reimporting archives with multiple files and CHDs with multiple tracks should be fixed in https://github.com/alucryd/oxyromon/commit/63e9f32e63337fd3471e9011e7be013a7023c54a

alucryd commented 3 months ago

Actually the remaining issue was a misdiagnosis on my end, I forgot I had converted these using CHD's parent feature. Before I can support reimporting these kind of CHDs, I need to support importing them in the first place. That's an edge case for now, I'll revisit it later if CHD parents become popular.