CUModSquad / Camelot-Unchained

UI repository for Camelot Unchained, a tri-realm RvR MMORPG.
Mozilla Public License 2.0
7 stars 25 forks source link

Cancelling `CancellablePromise` doesn't cancel promise #278

Open Mehuge opened 5 years ago

Mehuge commented 5 years ago

Found while investigating an issue with keybinds. If you click a key to bind, and say no or close the dialog, it calls this.listenerPromise.cancel() however the next key press still fulfills the promise, causing a prompt to bind the key.

I added some debugs to KeybindRow.tsx to demonstrate this:

[Log] startBind 0 (hud.js, line 81740)
[Log] listenForKeyBindingAsync undefined (hud.js, line 81749)
[Log] GAME TASK: NEW ACTIVE TASK 3722305007 cancel=undefined (hud.js, line 21171)
[Log] GAME TASK: ACTIVE TASK {"id":3722305007} (hud.js, line 21181)
[Log] this.cancel (hud.js, line 81777)
[Log] cancel keybind listener (hud.js, line 81780)
[Log] GAME TASK: CANCEL TASK 3722305007 (hud.js, line 21188)
[Log] GAME TASK: taskComplete | {"id":3722305007,"statusCode":2,"value":{"__Type":"Binding","name":"/","value":191}} (hud.js, line 21207)
[Log] GAME TASK: RESOLVE TASK {"id":3722305007,"cancelled":true} (hud.js, line 21213)
[Error] CancellablePromise: resolved after being cancelled
    (anonymous function) (hud.js:21226)
[Log] this.cancel (hud.js, line 81777)
[Error] Key bind failed {"cancelled":true,"errorMessage":"promise has been cancelled"}
    (anonymous function) (hud.js:81756)
    promiseReactionJob

Not sure how listenForKeyBindingAsync is intended to work, but would cancelling the promise cancel the capture of the key press anyway? Don't we need to also tell the game client to stop listening?

Mehuge commented 5 years ago

Annotated trace

[Log] startBind 0 (hud.js, line 81740)
[Log] listenForKeyBindingAsync undefined (hud.js, line 81749)

User clicks key to bind, client calls game.listenForKeyBindingAsync()

[Log] GAME TASK: NEW ACTIVE TASK 3722305007 cancel=undefined (hud.js, line 21171)
[Log] GAME TASK: ACTIVE TASK {"id":3722305007} (hud.js, line 21181)

Library gets handle id (task id) from API and wraps a promise around the resolve/reject functions.

[Log] this.cancel (hud.js, line 81777)
[Log] cancel keybind listener (hud.js, line 81780)

User clicks X to cancel keybind request. This calls cancel() on the promise.

[Log] GAME TASK: CANCEL TASK 3722305007 (hud.js, line 21188)

Library does engine.trigger('CancelTask', handle.id) to cancel the task.

[Log] GAME TASK: taskComplete | {"id":3722305007,"statusCode":2,"value":{"__Type":"Binding","name":"/","value":191}} (hud.js, line 21207)

User presses a key, so the key bind listener (which should have stopped listening) erroneously triggers a taskComplete event in the client.

[Log] GAME TASK: RESOLVE TASK {"id":3722305007,"cancelled":true} (hud.js, line 21213)
[Error] CancellablePromise: resolved after being cancelled
    (anonymous function) (hud.js:21226)

The taskComplete event handler (now recognising its a cancelled promise - new code I added) rejects it.

[Log] this.cancel (hud.js, line 81777)
[Error] Key bind failed {"cancelled":true,"errorMessage":"promise has been cancelled"}
    (anonymous function) (hud.js:81756)
    promiseReactionJob

The KeyBindRow.tsx module handles the rejection by cancelling the promise again and logging a console error for key bind failed.

Mehuge commented 5 years ago

Not sure how the Game API is supposed to work, but I would guess that sending the CancelTask event should have told the keybind listener code to stop, thus preventing the subsequent capture of a key and resolving of the promise.