Refactor of JASC-PAL

[waterwheels]

Simplified the line-reading function to use fgets() which handles windows/unix line endings internally. Simplified the main palette-reading function, and consolidated all the error checking logic in it.

I added checks for as many errors I could think of, but happy to add more checks if anyone has suggestions.

Thanks to my friend Lito, who knows C much better than me, and workshopped the bones of this refactor with me.

[GriffinRichards]

Sorry for the delay. After more testing I'm still seeing some error messages worse than before. Example: if I have a palette with the invalid color entry 74 65 90 255 123, the error I got before would be

The line "74 65 90 25" is too long.

and the error I get now is

Invalid color format in color ""

[waterwheels]

No rush, I really appreciate you looking over this. I tried to replicate the issue, but adding your example line to a new test set palette didn't produce the same results. Could you share the palette you were testing with? Here's the full set of test palettes I've been using to check the changes I've made, now including 'five colours.pal'. I run them all with find . -type f -iname "*.pal" -exec echo "Encoding "{} \; -execdir "$gbafx" {} out.gbapal \; and they should each work or produce the specified error. (There's one more test palette called 'permission denied' but it seems pointless to share it since I need to allow access to zip it).

[waterwheels]

New test set with a palette with an out-of-range colour component

[GriffinRichards]

I tried to replicate the issue, but adding your example line to a new test set palette didn't produce the same results.

I changed this color https://github.com/pret/pokefirered/blob/master/graphics/battle_interface/healthbar.pal#L9 to 74 65 90 255 123 and then allowed gbagfx to run automatically as part of the build process (i.e. tools/gbagfx/gbagfx graphics/battle_interface/healthbar.pal graphics/battle_interface/healthbar.gbapal). That gives me the error Invalid color format in color ""

[sophxm]

I tried to replicate the issue, but adding your example line to a new test set palette didn't produce the same results.

I changed this color https://github.com/pret/pokefirered/blob/master/graphics/battle_interface/healthbar.pal#L9 to 74 65 90 255 123 and then allowed gbagfx to run automatically as part of the build process (i.e. tools/gbagfx/gbagfx graphics/battle_interface/healthbar.pal graphics/battle_interface/healthbar.gbapal). That gives me the error Invalid color format in color ""

@waterwheels This is because fgets stops reading after MAX_LINE_LENGTH, in this case making the line endings the next piece of input. Your five colours.pal test file uses unix line endings and 74 65 90 255 123\n fits in the buffer exactly, but 74 65 90 255 123\r\n does not (try changing 74 to 174 to recreate the bug with unix line endings, or just insert a blank line anyway).

It's probably better to avoid trying to sscanf it all at once and just parse it in steps, trying to create a format string that exclusively matches this ends up borderline-incomprehensible.

[waterwheels]

Thank you, that confirms my current line of testing. Increasing the max line length arbitrarily large 'solves' this issue, but that's not really a solution, so I'm working on having ReadJascPaletteLine() detect when it's filled up the line buffer without finding a newline and throw a more verbose error

[sophxm]

Thank you, that confirms my current line of testing. Increasing the max line length arbitrarily large 'solves' this issue, but that's not really a solution, so I'm working on having ReadJascPaletteLine() detect when it's filled up the line buffer without finding a newline and throw a more verbose error

Unfortunately sscanf will still happily accept bad input like 123 123 123a (which currently is just silently ignored), or 0 0 0, it's not really designed to ensure an exact match, which is what you generally want for parsing a format.