-
Notifications
You must be signed in to change notification settings - Fork 21
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
Windows: declare UTF-8 through manifest #96
Conversation
fc2ff51
to
f340a1b
Compare
Jenkins: test this please Thanks. Though we have no way to test the mingw path |
The relative paths to the manifest will break when you try to build a tool separate from e.g. archive checkout. For the "external" folder, I used symlinks, since they are supported on desktop platforms and Do not worry too much about autotools, it will be removed in the future anyway. Mingw building is unsupported, so can be fixed when actually used. |
Can you explain this in more detail please? I do notice two problems. My intention was to give the path to the manifest relative to the Modules/ConfigureWindows.cmake file. But
Putting those together, gives get_filename_component(_CONFIGURE_WINDOWS_BASE_DIR "${CMAKE_CURRENT_LIST_DIR}" REALPATH)
function(target_use_utf8_codepage_on_windows target)
set(RESOURCES_DIR "${_CONFIGURE_WINDOWS_BASE_DIR}/../resources")
... Does that look right? |
He means that all files you access from within a tool must be inside the tool directory. So e.g. for xyz2png the resource must be somewhere in the xyz2png directory. Iirc the command to create directory symlinks on Windows is So in every directory that references the resources you must do And in In case this doesn't work because symlinks on Windows are a bit weird: I can add the commit for you. |
Why? |
As c1 explained
The tool we use to create the final distribution archive resolves the symlinks and converts them back to "normal" files |
I'm not sure why you use symlinks at all then but I guess it doesn't matter here. So I should symlink |
yeah pls symlink resources in all folders. You can keep the mingw rc file. It doesn't hurt even if untested :) |
Should make tools work on non-ASCII paths.
f340a1b
to
fdfd3d7
Compare
Will CI run automatically or...? |
Thanks. Okay, how does that look? I can confirm it still works for me. Can anyone else test it? |
Sorry for the late reply, any suggested game file for testing this quickly? |
You can use the Japanese version of Yume Nikki from here https://rmarchiv.de/games/1089 2007-10-01 Vollversion - 0.10 just e.g. pass a Japanese filename to xyz2png etc. |
Tested latest pr build from https://ci.easyrpg.org/job/tools-win32-pr/211/ Tried xyzcrush on Windows 11 24H2. I guess globbing is broken (possibly unrelated different bug), but on a codepage 1252 is not showing characters at least. But converting with xyz2png works as expected and the output file now exists with same Japanese filename with extension .png. lmu2png also shows garbled characters without setting encoding at least. |
Yes, that's all correct. Globbing is the responsibility of each program on Windows, the shell doesn't do it. There's an .obj you can link to to supposedly do it for you though, |
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.
Looks good to me.
Fixes #61. Fixes #86.
Attaches a manifest to Windows .exes which makes
char*
s (narrow char strings) likeargv
be encoded in UTF-8, allowing tools to operate on files that can't be written in the system's narrow char encoding (eg. Japanese filenames on a Latin-1 system).This is basically copy-pasted from the way libjxl/libavif does it.
All the tools are changed except xyz-thumbnailer (I didn't look into that one).
For lmu2png specifically, I believe it also fixes a bug where a narrow char path and a UTF-8 string are concatenated. This implicitly assumes the narrow char encoding is UTF-8. Previously, Japanese games wouldn't work on Windows even when the system narrow char encoding was Shift JIS.
More points:
resources/
. Tell me if you want them somewhere else.