search

mrchrisster / MiSTer_SAM

154 stars 23 forks source link

Console whitelists and configurable MiSTer_SAM repository #91

Closed InquisitiveCoder closed 3 years ago

InquisitiveCoder commented 3 years ago

This patch addresses issue #90 by introducing optional whitelist files for each console core. If the file exists and is not empty, only games from the list will be used.

The change is accomplished by adding tests to find such that ROM files that are not in the whitelist, and zip files that don't contain at least one ROM in the whitelist, are excluded from find's output. The whitelist is also used when selecting which random ROM to extract from the zip file.

A simpler approach would've been to check the whitelist right before the exclude list, but during testing I found this would result in a large number of skips and calls to next_core if the whitelist is small relative to the number of ROMs. Filtering the files during the find command ensures the script doesn't waste time selecting the wrong ROMs.

The exclusion list is still checked last, so if a file is mistakenly listed in both the whitelist and exclude list, the exclude list takes priority.

I also made a minor change to make the SAM repository configurable, since I wanted the script to use my fork of the code during testing.

EDIT: I found the old code hard to manage and test so I took the liberty of unifying the zip and non-zip logic. Rather than choosing from one of many code paths based on heuristics (i.e. number of zip files, number of ROM files, size of the zips), the new code considers both ROMs and zips in one call to find. This allows all of the scenarios the old code was trying to manage (one big zip, one ROM per zip, unzipped ROMs) to be handled uniformly.

I recognize this last change isn't strictly backwards compatible, so if you'd prefer I roll it back, let me know.

mrchrisster commented 3 years ago

I don't have time checking this but I'm happy to merge it if you feel it's ready? I see you're still working on it

mrchrisster commented 3 years ago

It's ok to combine the zip and non-zip logic

InquisitiveCoder commented 3 years ago

Thanks, I'm glad you didn't mind the rewrite. I think the new code will be easier to maintain.

I squeezed in one last small change right before your comment but it's good to go now. I've tested it with whitelist on/off, exclusion list on/off and zip files enabled/disabled and everything seems to be working correctly.

While I was working on this I found a minor bug related to how zip files are handled but I'll open a separate pull request for that instead of piling on unrelated commits to this one.

mrchrisster commented 3 years ago

Great, thanks for your work on this!