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
Fix for issue #746 #764
Fix for issue #746 #764
Conversation
src/CxbxKrnl/HLEIntercept.cpp
Outdated
std::string filename = sstream.str(); | ||
setlocale(LC_ALL, ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is a requirement? setlocale
is also called below and does not make another call later on in the function for revert to empty string. Just wondering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure, I put it just in case but probably yes, it can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed: it works as well
src/Common/Xbe.cpp
Outdated
for (auto it = s.begin(); it < s.end(); ++it) | ||
{ | ||
bool found = illegalChars.find(*it) != std::string::npos; | ||
if (found) { *it = ' '; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of space, I think it should be underline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if I do that for Tenchu you will get Tenchu_ Return from Darkness and I think that's odd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may look odd, it also tells us this title has a invalid character in it. I think Windows change it to underline by default. I could be wrong... I had seen it happen several times, just don't remember where I saw it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I can confirm the illegal character convert into underline with chrome browser by saving a webpage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I think we can change all spaces to underline chars
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then it will look more odd. 😨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't leave the illegal characters in the filename, if I do that WritePrivateProfileString will stop at the first illegal char. For example, for Tenchu you will get just Tenchu and the remaining part will be truncated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. i did like you said
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With underline of illegal character | With your suggestion with all spaces |
---|---|
Tenchu_ Return from Darkness |
Tenchu__Return_from_Darkness |
WritePrivateProfileString accept spaces...
It's best to replace only the illegal characters for this function.
LGTM |
I tested games with UNICODE characters (The Return of the King™), with illegal characters (Tenchu: Return from Darkness has a colon) and multi-xbe titles (DEAD OR ALIVE ULTIMATE). The generated cache files are the same (except for the date which is normal) and the clear cache options work as well.
I also added myself to the list of contributing devs: if you don't want, tell me and I will remove it
cache-files.zip