manuelVo / foundryvtt-drag-ruler

A Foundry VTT module that shows a ruler when dragging tokens so you can see how far you've dragged them
MIT License
40 stars 52 forks source link

Requested change to how `onEntityLeftDragMove` and `onEntityDragLeftDrop` interact with the ruler #204

Open caewok opened 2 years ago

caewok commented 2 years ago

I am working on better compatibility between modules like Elevation Ruler and Drag Ruler. I am running into a bit of a show-stopper with how onEntityLeftDragMove and onEntityDragLeftDrop work (in main.js).

hookDragHandlers wraps _onDragLeftMove and _onDragLeftDrop using libWrapper. That is fine, but then onEntityLeftDragMove and onEntityDragLeftDrop both use the call method to override ruler functions (_onMouseMove and moveTokens, respectively). The issue for me is that because the call is to an internal DragRuler function, there is basically no way to stop or modify this chain of events.

function onEntityLeftDragMove(wrapped, event) {
    wrapped(event);
    const ruler = canvas.controls.ruler
    if (ruler.isDragRuler)
        onMouseMove.call(ruler, event) // <-- this call
}
function onEntityDragLeftDrop(event) {
...
    if (ruler._state === Ruler.STATES.STARTING)
        onMouseMove.call(ruler, event); // <-- this call
    ruler._state = Ruler.STATES.MOVING
    moveEntities.call(ruler, ruler.draggedEntity, selectedTokens); // <-- this call
}

The solution, as I see it, is relatively straightforward. The other functions of this type call a ruler method, which I can then wrap or modify because the DragRulerRuler class is available at window.Ruler. But if you could migrate onMouseMove and moveEntities into the DragRulerRuler class, this problem would go away. This would also be consistent with other libWrapper wrapping functions in main.js, which usually call ruler.METHOD directly. Thus, if we had DragRulerRuler.prototype.onMouseMove and DragRulerRuler.prototype.moveEntities, then the above code becomes:

function onEntityLeftDragMove(wrapped, event) {
    wrapped(event);
    const ruler = canvas.controls.ruler
    if (ruler.isDragRuler)
        ruler.onMouseMove(event) // <-- this call
}
function onEntityDragLeftDrop(event) {
...
    if (ruler._state === Ruler.STATES.STARTING)
        ruler.onMouseMove(event); // <-- this call
    ruler._state = Ruler.STATES.MOVING
    ruler.moveEntities(ruler.draggedEntity, selectedTokens); // <-- this call
}

Thanks for considering this! Let me know if you would like me to open a PR to accomplish this.

manuelVo commented 2 years ago

Making _onMouseMove and moveTokens override is a fair request. Unfortunately, I'm myself currently in a very stuck situation, where I have a huge backlog of changes that awaits releasing, but is currently blocked by a change in Enhanced Terrain Layer that needs to happen first. I'm currently working on resolving that, but the next release of Drag Ruler could be awhile for that reason (I suppose no further releases for v9 will happen and instead the next Drag Ruler release will happen after v10, given my current speed of progress), but I'll definitely look at making this happen.