Make adjustment to tileset column count changes a manual action

[bjorn]

After a discussion with eishiya we agreed that the process of adjusting a map or tileset to a change in the column count of a tileset image (which can happen when the image width changed, mostly) should really be manually triggered.

Currently, Tiled tries to be helpful by automatically triggering this change, just giving the user a single chance to decide whether it is desired or not using a modal dialog. Often the user will not be able to make an informed decision at that time, and another problem is that the action may need to be repeated for other maps that were not open at the time, which is something that's currently not possible without undoing and redoing the change to the tileset image with those maps loaded.

In issue #2863 the goal is to remove the need for adjusting tile indices entirely, but at least for reasons of compatibility we should still support the current workflow and fix this behavior.

Suggested change:

• Introduce an explicit action that can be triggered, both from UI and from scripts, to adjust the tile indices in a certain asset based on an old and a new column count (in the UI, it might be more helpful to have the user enter the old and new image width). This action would use the AdjustTileIndexes undo command for maps and the AdjustTileMetaData undo command for tilesets (both commands will need to be changed to not read the column count change from the tileset).

• Turn the modal dialog into a non-modal warning. This way, the user can also see whether the map is messed up, which would allow making a more informed decision.

• Ideally, the warning should apply per-asset. So, when one map is fixed up and another broken map is loaded, Tiled should show this warning again (currently, since the expected column count is stored on the tileset, the warning would not show again).

[eishiya]

Essay ahead!
tl;dr: I think fixing broken maps should be a tileset-focused action rather than a map-focused action, and batching needs to be a built-in option because it's almost always needed.

---

Ideally, the warning should apply per-asset. So, when one map is fixed up and another broken map is loaded, Tiled should show this warning again (currently, since the expected column count is stored on the tileset, the warning would not show again).

I'd rather not see things like column count stored in the map, so if this is stored at all, I hope it's part of the session, same as how Tiled stores the zoom level and selected layer. This could result in false positives when collaborating, if the updated maps and tilesets are sent, but not the updated session. Hopefully users are smart enough to see that their maps aren't broken, and communicate about things like changes to the tileset.

Solo developers usually know when they're changing their tilesets, so any warning wouldn't tell them anything they don't already know. So, I think the focus should be on creating a powerful action to adjust their existing maps, and not on warning/prompting them about it.

The suggested changes in the OP sound like they're approaching this as a map action, modifying the currently selected map based on the tileset(s)'s old and new column counts. While I can see the logic in this and it's consistent with how it's done currently, it doesn't feel intuitive to me. I feel this is fundamentally a tileset action. I've changed my tileset, I know I want to adjust some maps now, and I might not know which maps. That is what I want Tiled to handle for me. As such, I feel that batching should be built into it.
I don't think scripting should be required for to batch this, because this action would be applied to many maps most of the time it's used, but the action isn't needed very frequently, but scripts are best in the opposite scenario - things that aren't needed most of the time, but are needed frequently when they're needed at all.

I think the action should let users input the old tileset parameters (column count, or image width, tile width, margin, spacing so that Tiled can calculate the column count), pre-populating those fields if their values are known (e.g. if we stored the original values when first opening the tileset). Tiled should not expect that it'll always know these values, since tilesets and maps may have been modified outside of (this user's) Tiled, or may be newly open in Tiled in an already broken state. If the values aren't known, it can pre-populate with the current values just to reduce the typing the user needs to do.

The batching options should let the user choose between fixing open maps, all maps in the project, and (maybe) manually selecting a bunch of maps. Tiled should automatically ignore any maps that don't use the tileset. If Tiled stores the old column count for each map, then it can ignore any maps that are already correct.

The tileset-first approach does mean that the user will need to run the action for each modified tileset, it won't be able to handle multiple modified scenarios in a single go. In my experience, this is fine, because there are typically fewer modified tilesets than there are maps using them, and I think people who use a large number of tilesets often do so in part because it reduces/eliminates the need to ever adjust the tileset images. Plus, the map-first approach really shouldn't group multiple tileset changes together anyway (and currently doesn't, AFAIK).

The map-first approach has the advantage of making it easier for a user to visually inspect their maps before applying their changes, at least, if the warning isn't a modal. Idealy, I think when batching, users should be given the option to inspect each map before letting Tiled "fix" it (as well as an option to just fix all maps without letting them inspect each one), but I can imagine it would be challenging to code it in such a way that users can pan, zoom, and hide/unhide layers, while still having the "Fix this map?" prompt on top.

The tileset-first approach, especially with batching, presents more UI challenges, but I think it would be much more useful in practical scenarios.

Lastly, a note on scripting: since my proposed action would have a dialog to go with it, tiled.trigger() would just pop up the dialog, same as e.g. tiled.trigger('ResizeMap'). If scripting is to make any meaningful use of this feature, it'll probably need to be accessible as a function that can take some parameters, e.g. tiled.adjustTileReferences(tileset, oldColumnCount, mapList, silentMode = false), or tilemap.adjustTileReferences(tileset, oldColumnCount) in a loop (a map-first approach does work nicely for scripting, I'll give you that xP). If we go with tilemap.adjustTileReferences(tileset, oldColumnCount), then the GUI action could be a fancy wrapper for this that takes care of picking the tilemaps to apply it to, and prompting the user as needed.