Enable loading FDS files with error handling #35
Conversation
Also exit the application, as any further operation will cause access violation errors.
|
We cannot exit the application on this error, or on any other error. If the application gets stuck in an invalid state when the game fails to load, it must be fixed. |
|
@leiradel Would you prefer leaving the application in an error state that will crash it on a later operation? I don't think that's a better situation. Like I said, exiting following an error message is the most graceful way to provide feedback and avoid later crashes, until these access violations can be avoided - which can be done as part of a later patch. Yes, this is a stopgap measure, and I can add a comment in the code to reflect that if you would like, but this is preferable to a silent failure or crash. Note that these access violations are not introduced by this PR, they already occur now following an attempt to load an FDS file, with or without a system present. This PR eliminates the issue and enables FDS support when a system is present, and provides feedback when it is absent. |
|
No, I'm saying that we should never exit the application because of a bug. The bug should be fixed, but it doesn't need to be fixed in this PR. If you can open an issue describing how to replicate the bug, it'll be awesome. This PR is good since it adds support for more games for NES, but we need to get rid of the |
|
To be fair, you don't need to change code to use |
|
Oops, I just saw that you've already made this change. Sorry about that. |
| // Assume that the FDS system is missing | ||
| _logger.debug(TAG "Game load failure (Nintendo)"); | ||
|
|
||
| MessageBox(g_mainWindow, "Game load error. Are you missing a system file?", "Core Error", MB_OK); |
There was a problem hiding this comment.
recommend showing a generic message for all systems. it's useful feedback, and there may be other reasons this fails for NES.
There was a problem hiding this comment.
That seems fine, but I want to keep this message, since a generic error message is no more helpful than crashing when a file is missing. I don't know if it is possible to differentiate between different types of errors in libretro, so the alternative would be to detect the system file ourselves before falling back to a generic error message. In any case, errors during core operations should be added at all levels, not just the loading stage, so I think another PR with better error handling would be more adequate.
Continued from #32.
FDS files cannot use the same parsing strategy as regular NES files. This solves the problem by falling back onto the default loading strategy if NES header parsing fails.
This also fixes #33, by displaying an error message when a game fails to load for
System::kNintendo, and exiting the application, as this is the most graceful way to handle this failure currently (all other methods tested result in access violation errors down the line).