[UI/UX] [Bug] Fix ModifierSelectPhase animation delay (#6121)

* Rework promise handling to ensure no races

* Add delay to ensure pokeball opening animation can be seen

* Remove leftover debug statements.

Co-authored-by: NightKev <34855794+DayKev@users.noreply.github.com>

* Add tween bouncing pokeball to tweens that must complete for promise to resolve

* Fix typo in tsdoc

Co-authored-by: Bertie690 <136088738+Bertie690@users.noreply.github.com>

---------

Co-authored-by: NightKev <34855794+DayKev@users.noreply.github.com>
Co-authored-by: Bertie690 <136088738+Bertie690@users.noreply.github.com>

src/ui/modifier-select-ui-handler.ts

@@ -273,12 +273,23 @@ export class ModifierSelectUiHandler extends AwaitableUiHandler {
     // causing errors if reroll is selected
     this.awaitingActionInput = false;
 
-    // TODO: Replace with `Promise.withResolvers` when possible.
+    const { promise: tweenPromise, resolve: tweenResolve } = Promise.withResolvers<void>();
-    let tweenResolve: () => void;
-    const tweenPromise = new Promise<void>(resolve => (tweenResolve = resolve));
     let i = 0;
 
-    // TODO: Rework this bespoke logic for animating the modifier options.
+    // #region: animation
+    /** Holds promises that resolve once each reward's *upgrade animation* has finished playing */
+    const rewardAnimPromises: Promise<void>[] = [];
+    /** Holds promises that resolves once *all* animations for a reward have finished playing */
+    const rewardAnimAllSettledPromises: Promise<void>[] = [];
+
+    /*
+     * A counter here is used instead of a loop to "stagger" the apperance of each reward,
+     * using `sine.easeIn` to speed up the appearance of the rewards as each animation progresses.
+     *
+     * The `onComplete` callback for this tween is set to resolve once the upgrade animations
+     * for each reward has finished playing, allowing for the next set of animations to
+     * start to appear.
+     */
     globalScene.tweens.addCounter({
       ease: "Sine.easeIn",
       duration: 1250,
@@ -288,30 +299,35 @@ export class ModifierSelectUiHandler extends AwaitableUiHandler {
         const index = Math.floor(value * typeOptions.length);
         if (index > i && index <= typeOptions.length) {
           const option = this.options[i];
-          option?.show(
+          if (option) {
+            rewardAnimPromises.push(
+              option.show(
                 Math.floor((1 - value) * 1250) * 0.325 + 2000 * maxUpgradeCount,
                 -(maxUpgradeCount - typeOptions[i].upgradeCount),
+                rewardAnimAllSettledPromises,
+              ),
             );
+          }
           i++;
         }
       },
       onComplete: () => {
-        tweenResolve();
+        Promise.allSettled(rewardAnimPromises).then(() => tweenResolve());
       },
     });
 
-    let shopResolve: () => void;
+    /** Holds promises that resolve once each shop item has finished animating */
-    const shopPromise = new Promise<void>(resolve => (shopResolve = resolve));
+    const shopAnimPromises: Promise<void>[] = [];
-    tweenPromise.then(() => {
+    globalScene.time.delayedCall(1000 + maxUpgradeCount * 2000, () => {
-      globalScene.time.delayedCall(1000, () => {
       for (const shopOption of this.shopOptionsRows.flat()) {
-          shopOption.show(0, 0);
+        // It is safe to skip awaiting the `show` method here,
+        // as the promise it returns is also part of the promise appended to `shopAnimPromises`,
+        // which is awaited later on.
+        shopOption.show(0, 0, shopAnimPromises, false);
       }
-        shopResolve();
-      });
     });
 
-    shopPromise.then(() => {
+    tweenPromise.then(() => {
       globalScene.time.delayedCall(500, () => {
         if (partyHasHeldItem) {
           this.transferButtonContainer.setAlpha(0);
@@ -344,6 +360,11 @@ export class ModifierSelectUiHandler extends AwaitableUiHandler {
           duration: 250,
         });
 
+        // Ensure that the reward animations have completed before allowing input to proceed.
+        // Required to ensure that the user cannot interact with the UI before the animations
+        // have completed, (which, among other things, would allow the GameObjects to be destroyed
+        // before the animations have completed, causing errors).
+        Promise.allSettled([...shopAnimPromises, ...rewardAnimAllSettledPromises]).then(() => {
           const updateCursorTarget = () => {
             if (globalScene.shopCursorTarget === ShopCursorTarget.CHECK_TEAM) {
               this.setRowCursor(0);
@@ -368,6 +389,9 @@ export class ModifierSelectUiHandler extends AwaitableUiHandler {
           });
         });
       });
+    });
+
+    // #endregion: animation
 
     return true;
   }
@@ -820,14 +844,45 @@ class ModifierOption extends Phaser.GameObjects.Container {
     }
   }
 
-  show(remainingDuration: number, upgradeCountOffset: number) {
+  /**
-    if (!this.modifierTypeOption.cost) {
+   * Start the tweens responsible for animating the option's appearance
+   *
+   * @privateremarks
+   * This method is unusual. It "returns" (one via the actual return, one by via appending to the `promiseHolder`
+   * parameter) two promises. The promise returned by the method resolves once the option's appearance animations have
+   * completed, and is meant to allow callers to synchronize with the completion of the option's appearance animations.
+   * The promise appended to `promiseHolder` resolves once *all* animations started by this method have completed,
+   * and should be used by callers to ensure that all animations have completed before proceeding.
+   *
+   * @param remainingDuration - The duration in milliseconds that the animation can play for
+   * @param upgradeCountOffset - The offset to apply to the upgrade count for options whose rarity is being upgraded
+   * @param promiseHolder - A promise that resolves once all tweens started by this method have completed will be pushed to this array.
+   * @param isReward - Whether the option being shown is a reward, meaning it should show pokeball and upgrade animations.
+   * @returns A promise that resolves once the *option's apperance animations* have completed. This promise will resolve _before_ all
+   *   promises that are initiated in this method complete. Instead, the `promiseHolder` array will contain a new promise
+   *   that will resolve once all animations have completed.
+   *
+   */
+  async show(
+    remainingDuration: number,
+    upgradeCountOffset: number,
+    promiseHolder: Promise<void>[],
+    isReward = true,
+  ): Promise<void> {
+    /** Promises for the pokeball and upgrade animations */
+    const animPromises: Promise<void>[] = [];
+    if (isReward) {
+      const { promise: bouncePromise, resolve: resolveBounce } = Promise.withResolvers<void>();
       globalScene.tweens.add({
         targets: this.pb,
         y: 0,
         duration: 1250,
         ease: "Bounce.Out",
+        onComplete: () => {
+          resolveBounce();
+        },
       });
+      animPromises.push(bouncePromise);
 
       let lastValue = 1;
       let bounceCount = 0;
@@ -857,7 +912,9 @@ class ModifierOption extends Phaser.GameObjects.Container {
 
       // TODO: Figure out proper delay between chains and then convert this into a single tween chain
       // rather than starting multiple tween chains.
+
       for (let u = 0; u < this.modifierTypeOption.upgradeCount; u++) {
+        const { resolve, promise } = Promise.withResolvers<void>();
         globalScene.tweens.chain({
           tweens: [
             {
@@ -883,65 +940,99 @@ class ModifierOption extends Phaser.GameObjects.Container {
               ease: "Sine.easeOut",
               onComplete: () => {
                 this.pbTint.setVisible(false);
+                resolve();
               },
             },
           ],
         });
+        animPromises.push(promise);
       }
     }
 
+    const finalPromises: Promise<void>[] = [];
     globalScene.time.delayedCall(remainingDuration + 2000, () => {
-      if (!globalScene) {
+      if (isReward) {
-        return;
-      }
-
-      if (!this.modifierTypeOption.cost) {
         this.pb.setTexture("pb", `${this.getPbAtlasKey(0)}_open`);
         globalScene.playSound("se/pb_rel");
 
+        const { resolve: pbResolve, promise: pbPromise } = Promise.withResolvers<void>();
+
         globalScene.tweens.add({
           targets: this.pb,
           duration: 500,
-          delay: 250,
           ease: "Sine.easeIn",
           alpha: 0,
-          onComplete: () => this.pb.destroy(),
+          onComplete: () => {
+            Promise.allSettled(animPromises).then(() => this.pb.destroy());
+            pbResolve();
+          },
         });
+        finalPromises.push(pbPromise);
       }
 
+      /** Delay for the rest of the tweens to ensure they show after the pokeball animation begins to appear */
+      const delay = isReward ? 250 : 0;
+
+      const { resolve: itemResolve, promise: itemPromise } = Promise.withResolvers<void>();
       globalScene.tweens.add({
         targets: this.itemContainer,
+        delay,
         duration: 500,
         ease: "Elastic.Out",
         scale: 2,
         alpha: 1,
+        onComplete: () => {
+          itemResolve();
+        },
       });
-      if (!this.modifierTypeOption.cost) {
+      finalPromises.push(itemPromise);
+
+      if (isReward) {
+        const { resolve: itemTintResolve, promise: itemTintPromise } = Promise.withResolvers<void>();
         globalScene.tweens.add({
           targets: this.itemTint,
           alpha: 0,
+          delay,
           duration: 500,
           ease: "Sine.easeIn",
-          onComplete: () => this.itemTint.destroy(),
+          onComplete: () => {
+            this.itemTint.destroy();
+            itemTintResolve();
+          },
         });
+        finalPromises.push(itemTintPromise);
       }
+
+      const { resolve: itemTextResolve, promise: itemTextPromise } = Promise.withResolvers<void>();
       globalScene.tweens.add({
         targets: this.itemText,
+        delay,
         duration: 500,
         alpha: 1,
         y: 25,
         ease: "Cubic.easeInOut",
+        onComplete: () => itemTextResolve(),
       });
+      finalPromises.push(itemTextPromise);
+
       if (this.itemCostText) {
+        const { resolve: itemCostResolve, promise: itemCostPromise } = Promise.withResolvers<void>();
         globalScene.tweens.add({
           targets: this.itemCostText,
+          delay,
           duration: 500,
           alpha: 1,
           y: 35,
           ease: "Cubic.easeInOut",
+          onComplete: () => itemCostResolve(),
         });
+        finalPromises.push(itemCostPromise);
       }
     });
+    // The `.then` suppresses the return type for the Promise.allSettled so that it returns void.
+    promiseHolder.push(Promise.allSettled([...animPromises, ...finalPromises]).then());
+
+    await Promise.allSettled(animPromises);
   }
 
   getPbAtlasKey(tierOffset = 0) {