Skip to content
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

Change: Support decoding of NewGRF with out-of-order v2 container and 32bpp-only files. #29

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

PeterN
Copy link
Member

@PeterN PeterN commented Dec 12, 2023

GRFCodec assumes v2 container data is used sequentially, however this is not enforced by OpenTTD and some existing NewGRFs are able to use data out of order, or reuse it.

OpenTTD allows this by prescanning all v2 container data and storing the offset for each ID, and I have copied the basic principle from there. Note that, in the case of reusing data, GRFCodec is not aware of this and will happily output multiple copies of the same sprite data. This may be something to address in future but for now simply the ability to decode is nice.

Additionally, previously GRFCodec did not properly export 32bpp files as the condition for outputting the correct nfo line for the first sprite was only used on the line that writes 8bpp sprites. This was added to the 32bpp lines as well, so that 32bpp-only NewGRFs can be decoded.

Note that GRFCodec already does not enforce that an 8bpp is present when encoding, so it was possible for GRFCodec to create a file that it could not decode.

While the NFO specs mention that an 8bpp normal zoom level sprite is required first, this is not checked by GRFCodec, and importantly, it is not checked by OpenTTD either.

The NewGRF file at the following link was used when testing this:

https://bananas.openttd.org/package/newgrf/524f4231

@glx22 glx22 merged commit ca9551d into OpenTTD:master Dec 12, 2023
8 checks passed
@PeterN PeterN deleted the decode-out-of-order-container branch December 12, 2023 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants