search

dwatteau / scummtr

Fan translation tools for LucasArts SCUMM games
MIT License
24 stars 4 forks source link

ScummFont corrupts some v1 fonts #15

Closed dwatteau closed 2 years ago

dwatteau commented 3 years ago

Tested with some games with V1 fonts, such as INDY3 PC VGA.

If you export a V1 font to .bmp, then modify it, and import it back to to the game, the new font is corrupted (in the game and in the new exported bitmap):

scummfont o 98.LFL 98.bmp
open -a preview 98.bmp # or anything that modifies its format a bit
bmptoppm 98.bmp |  ppmtobmp -windows -bpp 8 > fixed.bmp
scummfont i 98.LFL fixed.bmp
scummfont o 98.LFL fixed-but-now-corrupted.bmp

indy3-corrupted-v1-font

The corruption problem seems to be located somewhere in the saveFont() function.

Original scummfont.exe also has this bug (but then you need to deal with the -new file when reproducing the use-case above).

dwatteau commented 2 years ago

So, looking at the 98.LFL file after its modification, it appears that the width table (at offset 0x08) is corrupted.

In the original file:

Character width table: 4, 8, 6, 8, 6, 0, 0 [...]

after the scummfont changes:

Character width table: 8, 8, 8, 8, 8, 8, 8 [...] (all 8s)
dwatteau commented 2 years ago

PR #48 was merged to work around this.

Basically, the following line is maybe wrong:

https://github.com/dwatteau/scummtr/blob/f067b2f5235d0a1c9dfcfb658b37942e4b4e5e5a/src/ScummFont/scummfont.cpp#L400

…in that some image editors are free to reorder/simplify the palette, but the code appears not to be ready for this.

So, we just reject any file with a modified palette, for now (GIMP and old MS Paint releases are known to leave it as-is, as long as you work from the original BMP file).

Making ScummFont even more strict is not really nice for users, but silently corrupting the game resources is worse.

ScummFont probably needs to be rewritten to either have a much more robust BMP implementation, or maybe a much better format than BMP (BMP is a bit like CSV… it seems so simple that you may think you should make your own encoder/decoder, but you really shouldn't). But that's not for now.