CD-ROM image listing from directories #229
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also incorporates the ".cue" and ".rom_loaded" checks. Intention is to re-use this during dynamic image scanning.
The goal here is to help support alternate image fetch strategies by making the function responsible for advancing the image, rather than callers manipulating image_index directly.
The initial code just looks for its presence, actual value will be re-read during image scanning.
It would make more sense to use getName() on the current image. However, during testing this always seemed to return '\0' as the filename, even without close() being invoked. Not certain if this is a bug or something I was doing wrong. If that can be sorted out this variable can be removed to save a chunk of SRAM.
scsiDiskResetImages() does a memset to 0 during reinit so default values need to be handled here.
This also uses some additional buffers to make the code easier to follow (hopefully) and help protect against excessively long paths.
|
The feedback thus far is much appreciated, thank you! |
|
I've tested this across several platforms and image switching (both from operating systems and external buttons) seems to be working properly. I did not test with huge numbers of images so that may expose some additional problems. Otherwise should be ready for review. |
PetteriAimonen
requested changes
Jun 2, 2023
PetteriAimonen
approved these changes
Jun 2, 2023
|
Excellent, thanks to both of you. Merged. :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposal to resolve #218 by adding an .ini file option called 'ImgDir' to read images out of a directory at runtime. This implementation follows the approach @PetteriAimonen outlined in the associated issue. It should also give a structure for supporting ini-free configuration in the future.
Basic image iteration works, including ejection events. However, there are parts (like directory naming) that are incomplete and testing has not yet been completed on different operating systems. If this approach seems workable, I'll continue to develop this and mark it as ready for merging once issues are fixed.