[Bug]: changes via javascript token.setProperty is not propagated to clients

[SherbanWisperwald]

Describe the Bug

I am setting a property on a token while the server is running and a client is connected via a javascript-UDF. The property is set on server side correctly, impersonating the token and calling [getProperty("test")] returns "newValue". But on the client it returns whatever old value it had before.

To Reproduce

1. Create the following two macros on a "Lib:js" Token, with URL access enabled:
   setTest macro on Lib:js:

"use strict";
function setTest(tokenId) {
	let token = MapTool.tokens.getTokenByID(tokenId);
	token.setProperty("test", "newValue");
}

MTScript.registerMacro("setTest", setTest);

init macro on Lib:js:

[h: js.createNS("jsFunctions")]
[h: js.evalURI("jsFunctions", "lib://js/macro/setTest?cachelib=false")]

2. Click on the init Button on Lib:js to register the setTest javascript-UDF.
3. impersonate a TestToken on the server and call [setProperty("test", "oldValue")]. Impersonate the same TestToken on the client. Now [getProperty("test")] called on server and on client returns "oldValue" in both cases, as expected.
4. call on the server [js.setTest(currentToken())].
5. call [getProperty("test")] on the server returns "newValue" (correct), a call to [getProperty("test")] on the client returns "oldValue", which is the bug.

Additional Info:
Writing the javascript as a workaround like this, i.e. calling a macro for setting the property, works as expected:

"use strict";
function setTest(tokenId) {
    let token = MapTool.tokens.getTokenByID(tokenId);
    MTScript.setVariable("value", "newValue");
    MTScript.setVariable("tokenId", tokenId);
    MTScript.evalMacro("[h: setProperty('test', value, tokenId)]");
}

Hint from Azhrei on in the discord channel:
Inside the Java code, changing a token property requires calling zone.putToken(token) on the token. It may be that the MapTool.tokens API in JS doesn't automatically do that — I don't know, just offering up ideas.

Expected Behaviour

Token property changes in javascript on the server should be propagated to the clients, i.e. in step 5 [getProperty("test")] should return "newValue" on server and client.

Screenshots

No response

MapTool Info

Version 1.13.2, New Install, Gentoo Linux using the debian package.

Desktop

Gentoo Linux: Linux omikron 6.1.41-gentoo #1 SMP Tue Jul 25 19:39:42 CEST 2023 x86_64 Intel(R) Core(TM) i7-5930K CPU @ 3.50GHz GenuineIntel GNU/Linux

Additional Context

==== MapTool Information ====
MapTool Version: 1.13.2
MapTool Home...: /home/stephan/.maptool-rptools
MapTool Install: /home/stephan/Anwendungen/maptool_1.13.2-amd64/data/opt/maptool/lib/app
Max mem avail..: 15 GB
Max mem used...: 560 MB
Custom Property: -DMAPTOOL_LOGDIR=/home/stephan/.maptool-rptools/logs
Custom Property: -DMAPTOOL_DATADIR=.maptool-rptools

==== Java Information ====
Java Home......: /home/stephan/Anwendungen/maptool_1.13.2-amd64/data/opt/maptool/lib/runtime
Java Vendor....: Eclipse Adoptium
Java Version...: 17.0.1
Java Parameters:
-Djpackage.app-version=1.0
-Xss8M
-Dsun.java2d.d3d=false
-Dsentry.environment=Production
-Dfile.encoding=UTF-8
-Dpolyglot.engine.WarnInterpreterOnly=false
-XX:+ShowCodeDetailsInExceptionMessages
--add-opens=java.desktop/java.awt=ALL-UNNAMED
--add-opens=java.desktop/java.awt.geom=ALL-UNNAMED
--add-opens=java.desktop/sun.awt.geom=ALL-UNNAMED
--add-opens=java.base/java.util=ALL-UNNAMED
--add-opens=javafx.web/javafx.scene.web=ALL-UNNAMED
--add-opens=javafx.web/com.sun.webkit=ALL-UNNAMED
--add-opens=javafx.web/com.sun.webkit.dom=ALL-UNNAMED
--add-opens=java.desktop/javax.swing=ALL-UNNAMED
--add-opens=java.desktop/sun.awt.shell=ALL-UNNAMED
--add-opens=java.desktop/com.sun.java.swing.plaf.windows=ALL-UNNAMED
-Djpackage.app-path=/home/stephan/Anwendungen/maptool_1.13.2-amd64/data/opt/maptool/bin/MapTool

==== OS Information ====
OS Name........: Linux
OS Version.....: 6.1.41-gentoo
OS Architecture: amd64
PATH...........: /usr/local/sbin:/usr/local/bin:/usr/bin:/opt/bin:/usr/lib/llvm/16/bin:/usr/lib/llvm/15/bin:/opt/cuda/bin
Number of Procs: 12

==== User Information ====
User Name: stephan
User Home: /home/stephan
User Dir.: /home/stephan/Anwendungen/maptool_1.13.2-amd64/data/opt/maptool/bin

==== Network Interfaces ====
Display Name..: eno1
Interface Name: eno1
Address...: 192.168.14.6

Display Name..: lo
Interface Name: lo
Address...: 127.0.0.1

Host Address...: 192.168.14.6
Default Gateway: 192.168.14.1

==== Locale Information ====
Country.: Germany
Language: German
Locale..: German (Germany)
Variant.:

==== Encoding Information ====
Default Locale: de_DE
Default Charset: UTF-8
file.encoding: UTF-8
sun.jnu.encoding: UTF-8
Default Encoding: UTF8

==== Display Information ====
Number of Displays: 3
Display 1: 5120x1080(-1)
Display 2: 1280x1024(-1)
Display 3: 1920x1080(-1)

==== Internet Gateway Devices ====
No IGDs Found!

[SherbanWisperwald]

The bug occurs also with MapTool 1.14.0 Alpha 3 (Pre Release).

[Azhrei]

I don't see this reported yet; thanks for the bug report. It's been discussed on the Discord a couple times already.

In the JSAPIToken class, I believe this line (or something like it) should be inserted after line 132:

putToken(this.token);

but I haven't had time to test it.

[SherbanWisperwald]

Is the token property synchronization with the clients already possible with another JS call?
From a performance point of view, it might be better to have a dedicated call for synchronization, so that multiple properties could be changed/set and then the synchronization is called only once.
Is putToken available as JS call? If not, maybe this would be good solution to make it available.

[Azhrei]

No, putToken() is not available from JS. I wouldn't consider it wise to leave that up to the user writing JS code anyway — they'd likely forget at least once and then spend many hours chasing down the problem.

I thought about setting it up so that returning from the GraalVM interpreter would cause a putToken() call on any tokens with pending updates. That would batch all of the updates at once. But that doesn't buy us anything, since the amount of network traffic is the same. In fact, it might make things worse given that the packet transmit time is not amortized over the life of the JS code.

So that means either putToken() could be exposed and setProperty() could call it, or the Java implementation of setProperty() could call it directly. I would lean towards the latter, as I can't think of a use case where calling setProperty() makes sense without also updating the clients immediately, particularly since waiting to do them all at once isn't helpful.

(The history of putToken() is a little dark. In the original days, it serialized the entire token and sent it across the network. Large tokens, meaning with lots of properties or image attachments, could take significant time when used on a slow upload link and many clients. However, the current implementation only sends fields that have changed, reducing the cost of calling the function. And it still doesn't handle simultaneous/conflicting updates to a specific field (such as a token property), but fixing that means adding UI support code throughout the app. I.o.w., a big investment in time.)

[SherbanWisperwald]

The same problem occurs for setNotes and setName, too.
setX and setY are working fine.

[ColdAnkles]

I've had a crack at fixing this and unless I've done something strange, a simple putToken() call didn't fix it.
See: https://github.com/ColdAnkles/maptool/tree/JS_setPropertyFix

Further debugging getting into the server->client message handling seems to show that when doing the MTScript 'setProperty()' a message is sent from client to server informing of the property change. A second message for the chat output (since [setProperty("test", "oldValue")] not hidden) appears as well - corroborated by the chat box getting the name and image of the token with a blank message.

Doing the JS 'setProperty()' (with putToken() in place) only has that second chat box message sent and received.

This is where my knowledge is failing, as I can't figure out where the message is ultimately coming from and why the JS one isn't generating a message.

All assuming I've made no mistakes in my debugging.

[ColdAnkles]

I found a solution - copied what setX() was doing and ran MapTool.serverCommand().updateTokenProperty(token, Token.Update.setProperty, name, value.toString());

I don't know if it's a good solution, but it seems to work...