-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update insert blank disk handling and improve invalid disk handling for saving #251
Conversation
Indeed, it looks like a functionality that was never fully implemented :-) drvA_format and drvB_format are not used anywhere at the moment. The way I personally handle this is that I have an I have some concerns with this PR though. I'll need to think a bit more about it, but here are my current thoughts: Concern 1It's a bit weird to have the "Format" button in the options. The options menu is meant to configure Caprice, not to do actions. On the other hand, I don't have a much better place to put it in the menu. It would also be weird in Load/Save as this is not really about loading/saving a file but about creating an emtpy disk that doesn't exist yet as a file. However, I think I'd still prefer it at this place. Having a new dedicated menu seems overkill. We could also have tabs in Load/Save (and potentially rename it) to make it less confusing. Another alternative would be to call dsk_format at startup, but it would mean it's not possible to have a CPC without a disk inserted. Not a big deal for most users, but still annoying for anybody who would want to test the case where no disk is present for some reason (I know I had to do that in the past). Finally, we could introduce it as keyboard shortcuts only but it's not ideal either. Concern 2The name of the button, "Format", is a bit weird when just above, we have a text saying "Insert blank disks as". We may want to reword one of them, either make the button "Insert" or make the text "Format disks as". I personally prefer the former (renaming the button to "Insert") because that's what we really want this button to do: to replace the disk in the drive. This naturally leads to the next concern: Concern 3When clicking "Insert", we should get an error message if there's already a disk in the drive. We may want to add a button "Eject" to allow removing the disk. But then we don't want to eject a disk if it's not been saved without warning the user. But then the user has to exit the options, go to Load/Save, save and then come back to the options. That's very weird (although probably not something that will happen very frequently) and suggest this whole Disk tab may be better placed in Load/Save. Or alternatively the whole Load/Save menu could be moved to this Disk tab! But the more I think about it, the less it makes sense to me to have the Disk tab in Options. None of this is saved in cap32.cfg. Currently, these formats are not used at all but if we were to use them, it makes sense to decide on it when doing the action, not when configuring Caprice. |
Note: if you can split this PR in 2 (extracting everything except CapriceOptions.h and CapriceOptions.cpp in a different PR) I can merge the rest as we think more about the correct change for the insertion of an empty disk. |
I agree with Concern 1. Perhaps change the Load/Save button to being renamed as Disks. It would then make more sense to change the Load/Save drop down and make the options "Insert Blank", "Load" and "Save" An "Eject" option could also be added into this dropdown as I don't really see any way to eject a disk in the user interface. |
I managed to mess your PR while trying to separate the error messages from the UI changes, sorry about that. In the end, I copied your change for the error messages in 595eb83 For the UI changes, I'd like to go with moving the "new disk" to the Load/Save menu, which may need some renaming, I'll try a few things. |
This change adds a Format button to the insert blank disk dialog in options.
Once the format button is clicked, it calls dsk_format for the appropriate disk with the select disk format.
This initialises the disk structure properly ensuring that later save disk works as expected.
A second part of this file is to improve error handling for trying to save when no disk was inserted.
If Save is called and the disk tracks are zero, no file will be written and an error message modal is displayed.
This should resolve #250