Fix point picking override

app/javascript/controllers/player/loop_manager.js

@@ -1,11 +1,13 @@
+import { debug } from "controllers/util";
+
 /** Class driving the execution of a playback loop */
 export default class LoopManager {
   /** @property {YoutubePlayer} */
   #player;
   /** @property {number} intervalId - The id to clear the interval */
   #intervalId;
   /** @property {number} times - The number of times the lopo has repeated */
-  #times = 1;
+  #times = 0;
   /** @property {boolean} aborted - Whether the loop has been canceled or not */
   #aborted = false;
   /** @property {AbortController} abortController - A controller to abort the
@@ -33,9 +35,9 @@ export default class LoopManager {
    * @param {?number} max - An optional maximum number of repetitions for the
    * loop
    */
-  loop(from, to, max = null) {
+  async loop(from, to, max = null) {
     // We need to stop any previous loop before we start a new one
-    this.clear()
+    await this.clear()
 
     this.#player.play(from)
 
@@ -61,8 +63,8 @@ export default class LoopManager {
           }
         })
 
-        if(max !== null && max !== undefined && this.#times > max) {
-          this.#times = 1
+        if(max !== null && max !== undefined && this.#times >= max) {
+          this.#times = 0
           clearInterval(this.#intervalId)
           resolve()
         }
@@ -75,17 +77,21 @@ export default class LoopManager {
    *
    * If there's a loop active we use the abort controller to stop it.
    */
-  clear() {
+  async clear() {
     if(this.#intervalId !== null && this.#intervalId !== undefined) {
-      // this.#player.pause()
-      this.#times = 1
-      if(this.#abortController && !this.#abortController.signal.aborted) {
+      debug("Interval id of this operation", this.#intervalId)
+      this.#player.pause()
+      this.#times = 0
+      if(this.#abortController !== null && this.#abortController !== undefined && !this.#abortController.signal.aborted) {
+        debug("aborting")
         this.#abortController.abort("LoopManager: Cancelled manually")
+      } else {
+        debug("Already aborted")
       }
-
       this.#intervalId = null
+      this.#aborted = false
     } else {
-      console.log("LoopManager: Tried to clear a non-existing loop")
+      debug("No interval")
     }
   }
 }

app/javascript/controllers/player/state.js

@@ -1,3 +1,5 @@
+import { debug } from "controllers/util"
+
 /** Abstract class for a PlayerState */
 class PlayerState {
   /**
@@ -44,7 +46,7 @@ export class ReadyState extends PlayerState {
   }
 
   reset() {
-    console.log("ReadyState: Nothing to reset")
+    debug("ReadyState: Nothing to reset")
   }
 }
 
@@ -75,8 +77,11 @@ export class EditingState extends PlayerState {
 
 export class PickingPointState extends PlayerState {
   async loop(from, to) {
+    debug("Looping 3 times", { state: this.constructor.name, from, to })
     await this.context.loopManager.loop(from, to, 3)
+    debug("Done point looping. Doing editing loop:", this.context.editState)
     this.context.state = this.context.editingState
+    this.context.editState = { ...this.context.editState, setting: null }
     this.context.loop(this.context.editState.start, this.context.editState.end)
   }
 

test/javascript/controllers/player/loop_manager_test.js

@@ -32,6 +32,13 @@ describe("LoopManager", () => {
 
       loopManager.loop(1, 5, 3)
 
+      // Force the event loop to move before we clear to allow the code in the
+      // loop promise to run and set the intervalId used for the check of clear
+      // FIXME: Extremely hacky and I don't completely understand why this works
+      // nor have the time. For now and for this test I'll leave it until I read
+      // more about it.
+      await new Promise(jest.requireActual("timers").setImmediate);
+
       jest.runAllTimers()
 
       expect(mockPlay).toHaveBeenCalledTimes(4)
@@ -44,9 +51,15 @@ describe("LoopManager", () => {
 
       const loop = loopManager.loop(1, 5)
 
-      jest.advanceTimersByTime(2500)
-      loopManager.clear()
-      jest.advanceTimersByTime(1000)
+      // Force the event loop to move before we clear to allow the code in the
+      // loop promise to run and set the intervalId used for the check of clear
+      // FIXME: Extremely hacky and I don't completely understand why this works
+      // nor have the time. For now and for this test I'll leave it until I read
+      // more about it.
+      await new Promise(jest.requireActual("timers").setImmediate);
+
+      jest.advanceTimersByTime(500)
+      await loopManager.clear()
 
       await expect(loop).rejects.toMatch("LoopManager: Cancelled manually")
     })