RPTools
[Bug]: onChangeSelection event in HTML5 can get triggered twice unnecessarily
Describe the Bug
If you change token selection to a single new unselected token by clicking on the token using the mouse left-hand button, the onChangeSelection event gets triggered twice: once on mouse left-hand button down and again on either mouse left-hand button up or mouse drag.
To Reproduce
- Create a blank campaign.
- Create some tokens, one being a lib token.
- Add the following macro to the lib token and run it:
[h, if(getSelected()==""): broadcast("NO TOKENS SELECTED") ; broadcast(getSelected())]
[h: oCS = macroLinkText(getMacroName() + "@" + getMacroLocation(), "none")]
[frame5("TEST"), code: {
<head>
<link rel='onChangeSelection' type='macro' href='[r: oCS]'>
</head>
<body>
[r: getSelected()]
</body>
}]
- Try selecting tokens as described in the bug description.
- Observe the triggering twice issue where the message is broadcasted on mouse down, and again on mouse up or mouse drag.
Expected Behaviour
onChangeSelection should only get fired the once in this scenario.
Screenshots
MapTool Info
1.15.2
Desktop
Windows 11
Additional Context
The name of the onChangeSelection event is a bit misleading, however the wiki does state that the event is fired when a token is selected (rather than when the token selection is actually changed!).
e.g. if you left mouse click on a single token that is also the sole currently selected token, onChangeSelection will also be triggered on left mouse up (i.e. unnecessarily as the selection has not effectively changed). Right mouse click on an already selected token does not trigger it. Similarly if no tokens are selected. and you left click somewhere else on the map (that is not a token), onChangeSelection is triggered but again the token selection has not effectively changed.
This is a documented issue with the onChangeSelection event, which may be called more than once on a change of selection and macro, so this needs to be taken into account. Or at least it used to be documented, but sometimes these warnings get removed from the Wiki. This is due to some internal tomfoolery that happens with how selection works and is never likely to be fixed for the current events
Is there a known work around for this? Or do I have to figure it out for myself? ;)
Is there a known work around for this? Or do I have to figure it out for myself? ;)
All onChangeSelection event handlers unfortionaly have to be written in a way that takes this into account
A bit of testing shows that the time between the two instances of the event varies somewhat, but is mostly within 100 ms. So I would think checking for a delta of say 200 ms, and ignoring calls within this should suffice.
A bit of testing shows that the time between the two instances of the event varies somewhat, but is mostly within 100 ms. So I would think checking for a delta of say 200 ms, and ignoring calls within this should suffice.
On your machine... it could vary wildly, but if your macros are only going to be running on the one machine then I guess its ok to use this logic in them
I would love if onChangeSelection could be replaced by a "real" event instead of a frame event. But that is probably a complete different discussion ;)
I would love if onChangeSelection could be replaced by a "real" event instead of a frame event. But that is probably a complete different discussion ;)
see #5274
A bit of testing shows that the time between the two instances of the event varies somewhat, but is mostly within 100 ms. So I would think checking for a delta of say 200 ms, and ignoring calls within this should suffice.
Care to share how are you are setting this up in your oCS?
Well, it relies on a method Skullman3194 posted in a thread (by me) on the Discord: https://discord.com/channels/296230822262865920/1303376781985841312
But basically it is a macro taking a (url encoded) json object which defines the parameters for the RunJSFunction function.
So:
<link rel="onChangeSelection" type="macro" href="macro://SendNotifyCallback@Lib:dovesoft.trinity-continuum/all/Impersonated?%7B%22name%22%3A%22TC%3A%20%C3%86on%20-%20Initiative%22%2C%22type%22%3A%22frame%22%2C%22func%22%3A%22refreshApp%22%2C%22thisArg%22%3A%22null%22%7D" />
The json object is actually:
{
"name": "TC: Æon - Initiative",
"type": "frame",
"func": "refreshApp",
"thisArg":"null"
}
The macro:
[h:name = json.get(macro.args, "name")]
[h:type = json.get(macro.args, "type")]
[h:func = json.get(macro.args, "func")]
[h:this = json.get(macro.args, "thisArg")]
[h:args = json.get(macro.args, "funcArgs")]
[h:funcArgs = if(json.type(args) == "ARRAY" , args, "[]")]
[h:frameVisible = (type == "frame" && isFrameVisible(name))]
[h:dialogVisible = (type == "dialog" && isDialogVisible(name))]
[h,if(frameVisible || dialogVisible): runJSFunction(name, type, func, this, funcArgs)]
And the js function in the frame (in this particular case):
const root = app.mount('#app');
let lastRefresh = Date.now();
async function refreshApp() {
try {
let thisRefresh = Date.now();
if (thisRefresh - lastRefresh > 200) {
lastRefresh = Date.now();
await root.process("refresh");
}
} catch (error) {
handleException("refreshApp", error);
}
}
app is the object representing a Vue application.
The particular issue in the OP is because we split selection logic between mouse press and mouse release. If we changed that to only modify selections in one place, it would avoid this issue. I would vote for only changing selection on mouse release.
More generally, we can fix redundant events by checking whether anything has changed before firing the event. There is a little bit of this done already, but we can make it universal by leveraging the selection history.
