Updating map position in world via javascript extension cannot be undone.

[agoaj]

Describe the bug

Updating a map's location in a world via an extension doesn't update the undo stack, so it cannot be undone.

Steps to reproduce

Load a world file with at least one map.
Call world.setMapPos or setMapRect from an extension to move any map in the world.
try to undo the change, the map won't move back to it's previous location.

Expected behavior

Hitting undo should return the map to it's previous position.

OS version

Windows 11

Tiled version

1.11.2

[dogboydog]

Hi, I can't seem to reproduce. Are you doing it differently?

The code does handle these methods as undo commands, so I'm curious what could be causing this for you

setmappos.mov

[dogboydog]

I did notice that the editor's scroll position in the world isn't reset when you undo, so it might appear at a glance as if the undo doesn't work when it actually does. I wonder if that's what you're seeing?

[agoaj]

Had to go compile myself to track down the cause. The issues crops up if you move the map while inside a custom tool because of how the tool undostack works.
The tool I found this with was trying to make map resizing easier.

1. World Move Tool has it's own undo stack.
2. All calls to change world map rects get put on the World Move Tool's undo stack.
3. Calling from javascript while on another tool or within a new registered tool hides the World Move Tool's undo stack.
4. Hitting undo will undo from a different undo stack, making it appear as if it was never properly registered.
5. Switching back to the World Move Tool shows the missing moves in the undo stack and they can be undone correctly.

This ignores being grouped in with a macro. I'd like to combine resizing the tilemap(which goes on the undo stack as expected) with moving the world rect, but the different undo stacks prohibits that.

[dogboydog]

Ah okay interesting. Seems like a more complex issue to fix than I thought

[bjorn]

The world and the map are separate files and hence have their own undo stack. Tiled currently does not have a concept of "global" undo steps and I'm not in a hurry to introduce that due to the additional complexity.

That said, the map resizing handles you've implemented are something I'd really like to add to Tiled natively. In hindsight maybe I should have stored the world position inside the map file, but it's a bit hard to change that now since some people might rely on the fact that a map can be part of multiple world files (though Tiled does not support a map to be part of multiple loaded worlds...).

If we stick to the current design, we could try to find some way to have "linked" undo commands, which on undo will also trigger the undo for their linked command. Challenges with this are:

• We need to handle possible deletion of the linked undo command.
• Qt's undo history is like a stack and does not support undoing a command somewhere in the middle.

A simple solution might be to undo the two changes only when they're both still the last change.

Finally we need to make it clearer what the user is currently editing. Indeed when switching to the world tool, they're editing the world and the tool activates the undo stack of the world. This is a bit of a hack which is almost invisible to the user. Instead, world editing should probably be a mode which is visually more distinctive.