valters-tomsons / Spectabis

A modern PCSX2 Frontend.
https://spectabis.github.io/
GNU General Public License v3.0
116 stars 19 forks source link

Error Logging output to file + PCSX2 directory change bug fix #33

Closed Michanne closed 5 years ago

Michanne commented 5 years ago

Added error logging output to file capability Added exception handling for Audio, Video and Input plugin buttons Added snackbar on Main Window to display errors Fixed issue with PCSX2 directory in settings window not validating correctly

valters-tomsons commented 5 years ago

This looks good, thanks! Logging to file is greatly appreciated.

CyberFoxHax commented 5 years ago

"Fixed issue with PCSX2 directory in settings window not validating correctly" i fixed that bug in 0bf79dbc846f3d5061eb06de230913b82aa686bd you can't assume the executable's name == "pcsx2.exe"

Michanne commented 5 years ago

Maybe so, but for sure this bug was not fixed when I was testing. Maybe you fixed a different bug

The main issue is it is checking if path entered in the UI is a directory, then it is checking if that same entered text is a file. This throws errors if you enter either a valid directory or a valid file in the path text box.

I assume it is looking for the PCSX2 executable, so I coded it to look for it. If you know of some config that has the name of the executable it should be looking for, or even that File.Exists check is not required, you're more than welcome to make the edit.

It is only something I noted off-hand while adding the error logging

CyberFoxHax commented 5 years ago

I can see what happened. The old config only saved the folder part and throughout the application it was assumed that the .exe name was == "pcsx2.exe". Which means your config was lacking the complete path. This is not your fault, and you'll certainly not be only one with the problem.

rather than adding magic strings, surely there must be a better way to do this?

CyberFoxHax commented 5 years ago

Here's an idea. In the code the determines if we need to show the welcome screen.

var checkDir = emuDir;
if(File.Exist(checkDir) && checkDir.EndsWith(".exe")){ // check more??
    // proceed like normal
    return;
}
checkDir = Path.Combine(checkDir, "pcsx2.exe");
if(File.Exists(checkDir)){
    // update the path and proceed like nothing happened
    emuDir = checkDir;
    SaveSettings();
    return;
}

// all failed
// show the welcome screen to the user
new Welcome().Show()

We can do more checks if you'd like. For example Contains("pcsx2"), you could also md5 the .exe. But for me it suffices to trust that the .exe chosen by the user is the one intended. (It will also make it easier to add more emulators should that ever become a feature in the future)

If there's nothing more to add I'll make this by the end of the week.

Michanne commented 5 years ago

Nothing to add