VnManager icon indicating copy to clipboard operation
VnManager copied to clipboard

Should Arguments be exempt from ContainsIllegalCharacters?

Open edgar1016 opened this issue 3 years ago • 4 comments

I was testing adding none localized visual novels that require Japanese locale. I use the program locale emulator to run these games. Locale emulator allows for running the LEProc.exe with the argument -run "path-to-exe" to load Japanese games with a shortcut.

Currently using periods is considered invalid in the Arguments text box for example -run "C:\Users\edgar\Desktop\Kara no Shoujo - FULL VOICE HD SIZE EDITION\Launcher.exe" would be invalid because it has a period in Launcher.exe.

https://github.com/micah686/VnManager/blob/14ffa2218fe9753910d986d5e2a8e1e6613eb31e/src/VnManager/Helpers/ValidationHelpers.cs#L47

So I was wondering if maybe this line should be removed or maybe add some sort of integration where the LEProc.exe path is stored and the software can run it with the argument containing the game executable's path. This could be toggled on an individual basis like the Enable Custom Arguments toggle maybe it could be something like Use Locale Emulator. I also don't like the idea of just adding a period to https://github.com/micah686/VnManager/blob/14ffa2218fe9753910d986d5e2a8e1e6613eb31e/src/VnManager/Helpers/ValidationHelpers.cs#L66 This line also keeps me from adding games with Japanese characters in its executable obviously. Also there's can't be more than once game added with the same executable path so adding game using LEProc.exe is a no go for now.

edgar1016 avatar Mar 03 '21 09:03 edgar1016

I was actually waiting for something like this, where I figure out what characters shouldn't be allowed. I'll work on editing the allowed characters. As for the LE path, I'm also planning to have it so that it will allow games with the same executable with different arguments.

micah686 avatar Mar 03 '21 17:03 micah686

Ok, this has been fixed in 3c5c7cd62af6ba1be1064b004ccbcbccb6ba6b38. I removed the check for duplicate exe files in the database, and removed the Illegal characters check for Arguments. I might add back in a check at a later point in time for one of these two functions to validate that the path is valid, but I'll wait on that.: GetInvalidFileNameChars() GetInvalidPathChars()

micah686 avatar Mar 04 '21 06:03 micah686

Thanks much appreciated. I was messing with those too but I found that GetInvalidPathChars() doesn't allow for periods and GetInvalidFileNameChars() doesn't allow "/" I think. I have a solution in my head where we can keep the previous code and I could create a toggle that makes the play button run LEProc.exe and the argument would be something like "-run \"" + SelectedGame.ExePath + ""\";. I'm still learning C# I'm mostly familiar with Java but I'll work on this overtime as well.

edgar1016 avatar Mar 04 '21 07:03 edgar1016

My thought was to add back some of the old code, but make these changes:

  • Only allow the same exe file multiple times if the arguments are different.
  • Use the GetInvalid....Chars(), but then strip out certain characters. So we could keep things like '/', but strip out obviously bad unicode characters.

micah686 avatar Mar 07 '21 08:03 micah686