Unicode rendering #47
Conversation
There was a problem hiding this comment.
Overall this is an extremely massive patch that is undocumented, seems to have irrelevant changes, and adds both a software dependency and even a third-party content dependency. That’s pretty much three strikes right away.
A proper system for localization is a good idea, but this isn’t a good start. It would be good to start with text data first, then unicode support with the existing bitmap (knowing that basically all the chars would be ‘?’ at first), then we can start modifying the stock content to support the new text data. Stapling a random font and shoving it in by force with a megacommit doesn’t help the code’s current quality.
|
For what it's worth, over 2/3 of tinyxml is documentation and tests. |
|
utfcpp I mean, not tinyxml... |
|
If it’s not needed to build we can remove it (aside from the license). This still seems like a very large dependency for what is essentially a single function. |
|
Still have some dot files, but WOW that repo must be pretty bloated. 3000+ lines to almost less than 500 is quite dramatic, no? |
|
Right, so that covers the dependency issue... I am definitely more okay with a dependency that’s ~400 lines, rather than 10 times that(!!!). I do still think the changes need to be designed differently, however - try to aim for the absolute minimum C++ standard possible (if you can get away with it, write it as basically C). We have to support some very old compilers, so there’s no room to be overly fancy. The char list in particular is pretty extreme - if there’s a way to determine the font information at load time instead of every lookup, and with a... slightly less hardcoded charset, that will make maintaining the font a lot easier in the long run. |
|
Possible route towards fixing the mega array: should we have a font.txt next to the PNG to provide the list of supported chars? This would allow for a default charset with the current font.png (when no txt is found) and allow for custom fonts, including the one you’ve made and other font updates in the future. (Sidenote: We’ll probably not include the custom font in the repo, that can be a content mod until Terry decides to update the font officially.) |
|
That sounds good to me. |
|
My keyboard broke midway through this commit and my only spare is super difficult to type on. I'm gonna work on this some more tomorrow. |
There was a problem hiding this comment.
Still a bit to do (and a boatload to test) but this is already a huge improvement.
@JayFoxRox does this at least compile and not crash when the mod is absent? If not, we’ll need to figure out a safe way to support this (maybe a text interface with UTF8 and ASCII backends?).
Not sure which mod you mean (mod[ification] = changes in this PR?). Before commenting earlier I didn't try compiling this PR. I actually did compile it now and found another issue (with utfcpp): dependency on C++ exceptions. Our toolchain does not currently support those: There doesn't seem to be a way to disable them with utfcpp? This explanation about my ongoing port is offtopic, so I'll fold this (click here for more information)My port (to original Xbox) is currently blocked by issues in our SDL2 port and a lot of stupid changes required for PhysFS. It still compiles though. But even the VVVVVV master revision does not work correctly on original Xbox (crashes in sound initialization), yet. As we don't have a debugger in our toolchain it will take some time to understand the crashes I'm getting. See JayFoxRox#1. However, I know that our original Xbox toolchain doesn't (currently?) support unicode in winapi (selfmade winapi abstraction for Xbox NT kernel) and also doesn't support I assume similar issues, weak UTF / unicode and exception support, for other homebrew SDKs for smaller platforms (PS2? PSP? Event badges?). At least original Xbox has a modern compiler (LLVM clang) - I heard that some PS2 SDKs use an ancient gcc (not sure if Sony SDK or homebrew SDK). I don't really understand why VVVVVV has to process strings itself anyway - most of it are probably constant texts or very simple formatting operations. So I'm not sure if the code would really need to understand UTF / Unicode - one could just map symbols to glyphs from data files. Localization is obviously a great feature that I'd also want to use in the future. A lightweight or optional implementation of Unicode is totally fine with me. Alternatively, you could also make the decision to drop support for niche platforms to focus on code quality / simplicity - which is also fine if it is communicated correctly, although it would be a bit disappointing. |
|
If we can remove the exceptions that’d be great - we don’t even handle them anyway so it’d be nice to make those optional (and submit the changes upstream of course). I think the problem with data processing is the editor, we may have to worry about custom scripts and levels. Maybe not though, if it’s easy to read/write the correct format (presumably we’d write this format for the static data anyhow). |
|
The main purpose of this was so custom levels could be written in foreign languages. |
|
@JayFoxRox Does your toolchain just not have throwing std functions, or do they abort instead of throwing? Should I avoid using them? |
|
I'm not catching the exceptions, for what it's worth. |
|
Also, for what it's worth, no UTF-8 strings ever get passed to libc or std functions that handle them specially. std::string is encoding-agnostic to any 8-bit encoding (which is why utfcpp is necessary in the first place). |
| #include <cstdlib> | ||
|
|
||
| //#define UTF_CPP_THROW(X) throw (X); | ||
| #define UTF_CPP_THROW(X) (printf("UTF-8 error: %s", (X).what()), exit(1)) |
There was a problem hiding this comment.
In theory I don’t mind this, but we should ask the utfcpp maintainer about this first. We may also meed to define UTF_CPP_CPLUSPLUS to fix some of the exception keywords and includes in both files. @JayFoxRox, would you be able to help with this?
There was a problem hiding this comment.
Unfortunately I can't help with this; I have many projects to maintain myself.
|
Thanks for taking the time to respect demands of limited platforms 👍
It wouldn't even build, because the compiler will depend on features in our libc/libcxx/compiler-rt. Small teams (like us hobbyists) don't have the capacity to support these features easily (it's easy to have a "functional" port of a libc/libcxx, but harder to have complete port which depends on proper stack unwinding and other platform / ABI dependent features). Basically there are some non-essential C++ features that are "harder" to port; so for custom platforms these typically aren't implemented or very low priority. We also still lack RTTI (XboxDev/nxdk#237). This usually isn't an issue as software from scratch can respect those limitations. Games typically don't use these features either - especially those which are suitable for even running on 20 year old hardware (low memory, slow CPU, limited resolution, no full blown OS, ..). If I were to remove As our toolchain improves we'll probably add support for these features.
Exactly. Ideally software that doesn't use these features shouldn't require them in the first place for portability sake (not just for our toolchain, but also other potential stakeholders which depend on non-FOSS toolchains which are impossible to update).
Sounds good, that shouldn't be an issue then 👍 |
desktop_version/src/KeyPoll.cpp
Outdated
| @@ -93,7 +93,7 @@ void KeyPoll::Poll() | |||
|
|
|||
| if (textentrymode) | |||
| { | |||
| if (evt.key.keysym.sym == SDLK_BACKSPACE) | |||
| if (evt.key.keysym.sym == SDLK_BACKSPACE && keybuffer.size() > 0) | |||
There was a problem hiding this comment.
Please pardon me, I am just curious. Would it be better to use here keybuffer.empty instead of keybuffer.size?
|
@voct Probably, yes. |
|
What's the situation on this right now? I'm really looking forward to UTF-8 support in VVVVVV. |
|
We need to write (and ideally upstream) some patches to make the C++ exceptions optional. That's the only serious blocker I remember, tbh. |
|
Those are already in the PR. I haven't tried to upstream them, though. |
|
Just rebased. |
This reverts commit ef0022c.
|
As a reminder, we're still not hosting the custom fonts - that will only be in the CE repo. Will let the buildbots do the rest of the talking... |
|
@flibitijibibo In VVVVVV-CE, I use FriBidi to handle bidirectional text. FriBidi is a very heavy library, but I wasn't able to find something more lightweight. Do you have any suggestions on this? |
|
I wasn't aware of #118 when I removed stelpjo, but it turns out they were in CONTRIBUTORS.txt twice after rebasing anyway. |
|
Looks like Graphics.cpp got pretty thrashed in the last update - the only way I can really see all the mismatches now is to Ctrl+F exactly four spaces (GitHub please stop ruining my whitespace the irony is not funny) and Highlight All. Still a lot of mixing and matching going on for some reason. All the other files seem okay. |
|
@flibitijibibo I reindented every function I could recall changing. Should I just reindent the whole file with tabs? |
|
I'm not sure what was already indented with spaces, so I've only reindented what was mentioned in the review. |
|
Squashed into upstream, thanks for all the work on this! |
Legal Stuff:
By submitting this pull request, I confirm that...
example, a 2.3 update on Steam for Windows/macOS/Linux)
CONTRIBUTORSfile for all of said releases, butwill NOT be compensated for these changes
Changes:
If font.png is more than 128px tall, it is assumed to be the .png version of the Space Station font. Compatibility with the original font is preserved.
All characters outside ASCII have a width of 8.
The Space Station font (font.png) was created by Matt 'Stelpjo' Aaldenberg.
utfcpp is by @nemtrif. It does not require attribution in binary releases.