search

mika76 / mamesaver

Mamesaver is a mame emulated screensaver - get all the good ol' games playing their demo modes while you procrastinate and enjoy!
https://mika76.github.io/mamesaver/
MIT License
37 stars 10 forks source link

#39 Ignored failure to stop MAME when disposing #40

Closed nullpainter closed 6 years ago

nullpainter commented 6 years ago

@mika76 do you want to either merge your azure-pipelines branch into master, or disable this check for now in Appveyor?

mika76 commented 6 years ago

Maybe you should log it though even if you ignore? Or we might run into issues like our friend where the logs don't help...

nullpainter commented 6 years ago

I was loathe to as it's just noise, but you're probably right. I was happy enough not to because it's in Dispose() - but I guess we would care if it left orphaned MAME processes...

nullpainter commented 6 years ago

Actually, the issue is a bit fiddlier than that. Mid-way through disposing, MAME does start - but then tries to access objects which are disposed. I'll add a check against the cancellation token source; that should do it.

mika76 commented 6 years ago

Eh I think I've been bitten by empty try catches too many times. Also sometimes I would just leave a debug.writeline while developing, forget to log properly and then spend hours trying to figure out why something fails haha

Although like you say it's in dispose for a process. So it's probably pretty safe to really ignore...

nullpainter commented 6 years ago

Looking through the log entries again, the issue was that we were attempting to start MAME while we were shutting down, so I've added the check prior to this:

2018-09-14 15:06:24.938 +12:00 [INF] Stopping screensaver
2018-09-14 15:06:24.949 +12:00 [DBG] Releasing device context for \\.\DISPLAY1
2018-09-14 15:06:24.955 +12:00 [INF] Invoking MAME with arguments: gunbird2 -skip_gameinfo -nowindow -noswitchres -sleep -triplebuffer -sound none -screen "\\.\DISPLAY2" -artpath c:\code\tools\mame\artwork;C:\Users\matthew_painter\AppData\Local\Temp\Mamesaver\Layouts
2018-09-14 15:06:24.955 +12:00 [DBG] Releasing device context for \\.\DISPLAY3
2018-09-14 15:06:24.971 +12:00 [DBG] Releasing device context for \\.\DISPLAY2

I've also moved the message box error out of Program and into the initial sanity check in MameOrchestrator. This way, the user receives an error if MAME can't be run but if there is another of these kind of random errors after the screensaver correctly starts, the user isn't confronted with a dialog. I think this is a nicer user experience for a screensaver as it's both non-essential and not something which is being explicitly invoked by the user.

mika76 commented 6 years ago

Fantastic Matt, you're a machine 💪