Change: Disallow using Action A to load sprites above the baseset unless reserved.

[PeterN]

Motivation / Problem

Using Action A above the baseset is error prone as the sprites are not fixed and can be moved around.

Any NewGRF doing so is likely to break in the future, so force it to break instead.

The NewGRF specifications have always been very clear on this, but it seems authors are not reading the spec. https://newgrf-specs.tt-wiki.net/wiki/ActionA

There are some authors not properly reading the spec, and using Action A when they should be using Action 5. These NewGRFs do appear to work for them, but there's no guarantee they will continue to do so.

Description

When loading sprites via Action A, check if the sprite ID is either in the permitted range (below SPR_OPENTTD_BASE), or if the NewGRF has reserved some SpriteIDs via GRM reservation.

If these conditions are satisfied, the sprites are loaded at the requested location.

Otherwise the sprites are loaded at the current location so that they have no effect, and a warning debug is output. It might be possible to skip the sprites, but it seems a bit tricky as we are mid-way into the Action A here so the usual way isn't available.

This effectively "breaks" the NewGRF now, instead of at some possible time in the future when authors have moved on to other things.

Limitations

I know of at least one NewGRF that is affected by this change, There may be others, but hopefully anything old enough to be affected and unmaintained will have been broken for a while already...

There may be some other conditions I haven't considered where Action A is appropriate, but I can't think of any.

NewGRFs known to be affected by this change

file	reason	Status
asasignals-v5p1	Using Action A to replace Action 5 sprites.	Author informed, has retracted NewGRF: https://github.com/j-n-dev/Asasignals
CZTR_Bridges-0.9.0	Assumes contiguous ranges, and reservation is too small.	Broken before this change, and unlikely to be resolved: https://www.tt-forums.net/viewtopic.php?p=1257714#p1257714

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

• The bug fix is important enough to be backported? (label: 'backport requested')
• This PR touches english.txt or translations? Check the guidelines
• This PR affects the save game format? (label 'savegame upgrade')
• This PR affects the GS/AI API? (label 'needs review: Script API')
  • ai_changelog.hpp, game_changelog.hpp need updating.
  • The compatibility wrappers (compat_*.nut) need updating.
• This PR affects the NewGRF API? (label 'needs review: NewGRF')
  • newgrf_debug_data.h may need updating.
  • PR must be added to API tracker

[PeterN]

Example of the GrfMsg that is output:

[2024-04-06 15:23:44] dbg: [grf:0] [41520001-asasignals-v5p1.tar/asasignals-v5p1/asasignals.grf:30] SpriteReplace: [Set 0] Changing 16 sprites, beginning with 5215, above limit of 4896 and not within reserved range, ignoring.

[2024-04-06 15:23:44] dbg: [grf:0] [CZTR_Bridges-0.9.0/CZTR_Bridges.grf:50] SpriteReplace: [Set 0] Changing 44 sprites, beginning with 6140, above limit of 4896 and not within reserved range, ignoring.

[PeterN]

As mentioned on Discord, this also does not allow using multiple reservations as if they are contiguous. That case is possible to handle by checking for the complete reserved range (instead of each range) if we want to be more lenient.