Skip to content
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

DONT MERGE - GUI: U32: Transforms from Win32 Ansi encoding to U32 in dialogs #2928

Closed
wants to merge 1 commit into from

Conversation

@ZvikaZ
Copy link
Contributor

@ZvikaZ ZvikaZ commented Apr 7, 2021

Windows returns paths encoded in local code page. The dialogs use
U32Strings. This commit ensures that the game path is explicitly
transformed to and from U32String when passed to or from the GUI dialog,
otherwise, a wrong implicit conversion is done.

Note:
This PR tries to solve the same problem as PR #2914 , but with (hopefully) simpler approach.
@SupSuper , @sluicebox , as far as I understand, this should be safe for the Win98 build. Please approve my assumption.

Windows returns paths encoded in local code page. The dialogs use
U32Strings. This commit ensures that the game path is explicitly
transformed to and from U32String when passed to or from the GUI dialog,
otherwise, a wrong implicit conversion is done.
@criezy
Copy link
Member

@criezy criezy commented Apr 7, 2021

I don't really like this approach. I think ideally the system encoding would be handled in the backend code and the fs code in common and gui code would expect Common::String to be UTF-8.

And here you only handle the game path. But what about the theme path and extra path for example? Also what about games using non-ASCII file names if we need to support that one day? How would the engine know which encoding it needs to use when creating a file stream if we cannot assume it is UTF-8.

@ZvikaZ ZvikaZ changed the title GUI: U32: Transforms from Win32 Ansi encoding to U32 in dialogs DONT MERGE - GUI: U32: Transforms from Win32 Ansi encoding to U32 in dialogs Apr 7, 2021
@ZvikaZ
Copy link
Contributor Author

@ZvikaZ ZvikaZ commented Apr 7, 2021

I don't really like this approach. I think ideally the system encoding would be handled in the backend code and the fs code in common and gui code would expect Common::String to be UTF-8.

And here you only handle the game path. But what about the theme path and extra path for example? Also what about games using non-ASCII file names if we need to support that one day? How would the engine know which encoding it needs to use when creating a file stream if we cannot assume it is UTF-8.

OK, thanks for noting that.
I'm working on a third PR ;-)

@bluegr
Copy link
Member

@bluegr bluegr commented Apr 9, 2021

I agree that this is definitely not the right way to go - we are essentially handling specific ANSI strings as Unicode. This is basically a hack, and it's limited to specific configuration paths.

The other two PRs follow approaches which do not suffer from this problem, so this one can be closed.

Closing

@bluegr bluegr closed this Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants