Question: Help with a possible async issue

[caewok]

Confirm you have read the above
I am the libRuler developer. I am working on a fork of Drag Ruler that uses libRuler, and I am having an issue that may be related to libWrapper and async, but I am not sure. This is bit involved for a discord question and is rather libWrapper-specific, so I thought I would try here instead.

** Setup **
Foundry 0.8.9
libWrapper 1.10.5.0
Drag Ruler fork: https://github.com/caewok/foundryvtt-drag-ruler/tree/caewok-libruler
libRuler: https://github.com/caewok/fvtt-lib-ruler

Describe the question
When dragging one or more tokens, I am seeing instances where the token moves places it should not. Sometimes, the token ends up at a different location than expected (usually seen with groups, where the group starts in a tight square and ends up spread out). Sometimes, the token drifts and then jumps back to a position. This, to me, suggests a problem with how async is being used to update token positions.

I reviewed issue #7 and have tried various versions of the following code, but this problem still persists. So I was hoping that you might have some insight into what might be going wrong here.

Code flow
I think what should be happening is:

• Drag Ruler moveToken calls the wrapped moveToken and awaits -->
• libRuler moveToken calls animateToken and awaits -->
• dragRuler wrapped animateToken runs. For each selected token:
  • The wrapped animateToken is called.
  • libRuler animateToken awaits Foundry base functions token.document.update and token.animateMovement.
• dragRuler collects the promises for each selected token and awaits the result.

Additional context
Sorry, this will be somewhat involved in order to explain relevant the code steps...

libRuler overrides Ruler.prototype.moveToken:

libWrapper.register(MODULE_ID, 'Ruler.prototype.moveToken', libRulerMoveToken, 'OVERRIDE');

The key part of the override for this issue, I think, is where the libRuler version iterates through the ruler segments (lines between ruler waypoints), and calls a new method, animateToken:

for ( let [i, r] of rays.entries() ) {
      // Break the movement if the game is paused
      if ( !wasPaused && game.paused ) break;
      
      // Break the movement if Token is no longer located at the prior destination (some other change override this)
      if ( priorDest && ((token.data.x !== priorDest.x) || (token.data.y !== priorDest.y)) ) break;

      priorDest = await this.animateToken(token, r, dx, dy, i + 1); // increment by 1 b/c first segment is 1.
    }   

libRuler adds to the new method to the Ruler class :

Object.defineProperty(Ruler.prototype, "animateToken", {
  value: libRulerAnimateToken,
  writable: true,
  configurable: true
});

export async function libRulerAnimateToken(token, ray, dx, dy, segment_num) {
  log(`Animating token for segment_num ${segment_num}`);
  
  // Adjust the ray based on token size
  const dest = canvas.grid.getTopLeft(ray.B.x, ray.B.y);
  const path = new Ray({x: token.data.x, y: token.data.y}, {x: dest[0] + dx, y: dest[1] + dy});
  
  // Commit the movement and update the final resolved destination coordinates
  const priorDest = duplicate(path.B); // resolve issue #3; get prior dest before update.
  await token.document.update(path.B);
  path.B.x = token.data.x;
  path.B.y = token.data.y;
  
  // Update the path which may have changed during the update, and animate it
  await token.animateMovement(path);
  
  return priorDest;
}

Now, Drag Ruler wraps moveToken and animateToken:

libWrapper.register(settingsKey, "Ruler.prototype.moveToken", dragRulerMoveToken, "MIXED");
libWrapper.register(settingsKey, "Ruler.prototype.animateToken", dragRulerAnimateToken, "WRAPPER");

Drag Ruler wraps moveToken because until the dragged token is let go, no actual movement should happen. The relevant portion is:

async function dragRulerMoveToken(wrapped) {
	if(!this.isDragRuler || this._state === Ruler.STATES.MOVING) {
		const p1 = wrapped();
		const res = await p1;
		return res;
	}
      ...

Drag Ruler wraps animateToken so it can handle adjustments for dragging multiple tokens. The relevant portion is:

async function dragRulerAnimateToken(wrapped, token, ray, dx, dy, segment_num) { 
...
  const promises = [];
  for(const entity of selectedEntities) {
    //const top_left = canvas.grid.getTopLeft(entity.center.x, entity.center.y);
    const offset = { x: entity.center.x - token.center.x, y: entity.center.y - token.center.y };
    const offsetRay = new Ray(ray.A, { x: ray.B.x + offset.x, y: ray.B.y + offset.y });

    // don't await here b/c this makes the tokens all move one-by-one
    promises.push(wrapped(entity, offsetRay, dx, dy, segment_num)); // usually works, but can occassionally fail to keep multiple tokens in original arrangement
    //promises.push(wrapped(entity, ray, dx + offset.x, dy + offset.y, segment_num)); // Fails to keep tokens in original arrangement much of the time
  }

  // use Promise.all to await to avoid confusing move errors if moving repeatedly very quickly
  promises.push(wrapped(token, ray, dx, dy, segment_num));
  const res = await Promise.allSettled(promises);

  // need to return the path from the movement for the token
  return res[res.length -1].value;  // Promise.allSettled returns an array of [{status: , value: }]; Promise.all returns an array
}

[ruipin]

I doubt this is directly related to libWrapper - all libWrapper does is detect a Promise, and if it is one it then delays the cleanup with a then call. Essentially, all Promise-related code is delegated to the callee/caller.

Every single unit/integration test in the suite will run both with async and not-async code paths (and randomised settlement delays), so this code should be well exercised.

The relevant libWrapper code can be found in https://github.com/ruipin/fvtt-lib-wrapper/blob/master/src/lib/wrapper.js#L480-L496

My immediate suspicion is the fact that Drag Ruler seems to be calling wrapped multiple times inside its animateToken wrapper, and then waiting for the Promises to settle in parallel:

• Depending on how more modules are wrapping this method, any single one of them could be causing issues.
• Even ignoring other modules, I believe the JS spec does not enforce a Promise settlement order. As such, a browser could resolve these Promises in any order. Does awaiting them individually resolve the issue?
• Even if the settlement order is fixed, if the user moves "repeatedly very quickly" there could easily be two or more animations occurring in parallel from all the Promises created by each call of animateToken.

If it is indeed the browser breaking the Promise resolution order for whatever reason, you might need to detect an on-going animation, and automatically settle/cancel the previous Promises (or ensure your new Promises are a then of the previous ones).

See for example how Token-Animation-Tools does the animation cancellation: https://github.com/ruipin/fvtt-token-animation-tools/blob/master/token-animation-tools.js#L168-L193

That said, I would suggest a few things that could allow us to remove libWrapper from the equation:

• Disable all modules except for these two (and libWrapper), to ensure there aren't other modules causing issues. Does the issue still occur?
• Enable libWrapper "High Performance Mode", since this triggers a different (less complex) code path. Does the issue still occur?
• Test the libWrapper Shim (with libWrapper disabled) to remove libWrapper from the equation entirely. Does the issue still occur?

[ruipin]

Closing since I believe this has been answered, and there have been no further replies in over two months. Feel free to re-open if you wish to continue the discussion.