[Bug]: Token moves momentarily applied before onTokenMove

[kwvanderlinde]

Describe the Bug

onTokenMove allows setting the tokens.denyMove = 1 to prevent a token from moving. But it looks like this is applied after the token is moved. If onTokenMove is in any way slow, other clients can observe the token in its new position only for it to snap back to its previous location if denied.

To Reproduce

1. Start a server and open this campaign: onTokenMove.cmpgn.renamed-to.zip
   • Lib:Wolf has an onTokenMove that uses input() to force it to take a long time. This is just to make the issue more apparent by controlling how much time onTokenMove will take.
2. In the campaign panel, click the "Elf Position" button. It should report (0, 0)
3. Open a second MT instance and connect to the server.
4. On the host instance, drag the "Elf" token anywhere else on the map and drop it. An input dialog should come up - leave it open for now.
5. On the connected client, notice the token image is at the final location. Clicking the "Elf Position" button will now reports the final position, not (0, 0).
6. On the host instance, answer the input dialog and watch the Elf token go back to where it started.
7. On the connnected client, watch the Elf token go back to where it started, and "Elf Position" will now report (0, 0) again.

Expected Behaviour

The token position should not be changed until after onTokenMove runs, and only if tokens.denyMove is not set.

Screenshots

No response

MapTool Info

1.15.2

Desktop

Linux Mint 22

Additional Context

No response

[kwvanderlinde]

I realize this is not as cut-and-dry as I make it sound above. onTokenMove has a deep problem in that it pulls double duty as a pre-move check and a post-move reaction. If used in the former way, the current behaviour is surprising and even problematic as in this bug report. If used in the latter way, onTokenMove actually needs the move to already be committed before it runs.

[cwisniew]

Yes onToken move is broken, when it was implemented the implementor didn't follow what was being done for the initiative change macros. It's been flagged as broken in this way and "never going to be fixed" for a long time.

Really we need to replace the whole current event system with something that works better and works across threads (which is the tricky part) if we ever want to have a future where new macro language doesn't run on swing thread