scummvm/scummvm — pull request #300 by criezy (2013-01-24 00:11:25+00:00)
Title: Credits memory usage and non-ASCII character fix

There were two goals for this pull request: reduce memory usage for the credits display in Scummvm and fix display of non-ASCII characters when using a GUI translations that does not use the ISO-8859-1 charset (bug #3539986).

The first two commit removes about 15 kb of static data from the stack without increasing the heap usage. The last commit fixes bug #3539986

I opened a pull request instead of pushing directly to master because I am not sure moving the credits to a data file is a good idea (which is what the first commit is doing). This means that now there is a possibility that it will not find the credits data (which is why I added a fallback message pointing to our web site). This does reduce the executable size though, which maybe will be useful for some platforms.

Now a bit more details:

The first commit just moves the data around without changing the behaviour. This reduces the executable size by about 15 KB, but increases the heap size by that same amount when the About dialog is visible.

Note: in AboutDialog::reflowLayout() to compute the optimal dialog width it was using g_gui.getStringWidth(credits[i] + 5) That does not make much sense to me. I could understand if it was skipping the first two characters (since they define the formatting and are not shown) but why skip 5 characters? And that does not looks safe if the string is less than 5 character (e.g. "" or "C1"). So in the first commit I changed it to g_gui.getStringWidth(credits[i]) + 5, i.e. width of the string + a margin of 5 pixels, which seems more logical to me (and the second commit actually removes that code altogether). So there is actually a small change in behaviour with that first commit.

The second commit avoids duplication of data, which saves about 15 KB of heap memory but at the cost of removing the heuristic for the optimal dialog width. The dialog looks good to me without that heuristic in 1x, 2x and 3x mode anyway.

The Third commit just fixes an outdated comment.

The Fourth commit dumps both the Latin1 and the ASCII text in the credits data file when they differ. The About dialog will load either one or the other depending on the charset being used. This fixes bug #3539986 GUI: Names in credits wrong with some translations (http://sourceforge.net/tracker/?func=detail&aid=3539986&group_id=37116&atid=418820)

Things remaining to be done: update the platform dependant packaging scripts to install/copy the credits.dat file.

If you do not think moving the credits to a data file is a good idea, I will rewrite this code to keep the bug fix, comment update and heuristic fix but without moving the credits to a data file. I may also investigate a different way to do it. I do not like the idea of doing a Latin1 to ASCII conversion on the fly in the code because of the special cases which would need to be duplicated between credits.pl and about.cpp (i.e. ö giving either o or oe depending on the name). But since the bug is only occurring when translations is enabled we could maybe use the translation manager by having credits.pl output a credits.po and modifying create_translations to use it for languages that do not use the ISO-8859-1 charsets.