From be66ffa5ef039b2dedb321d724031d0c5d72f092 Mon Sep 17 00:00:00 2001
From: adroitwhiz <adroitwhiz@protonmail.com>
Date: Tue, 16 Jul 2024 08:37:59 -0400
Subject: [PATCH 1/5] Fix lints and typechecking errors

---
 src/Project.ts  |  5 +++--
 src/Renderer.ts |  4 +---
 src/Sprite.ts   | 17 +++++++++--------
 src/Watcher.ts  |  2 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/Project.ts b/src/Project.ts
index 0d1e3e7..ebdfa19 100644
--- a/src/Project.ts
+++ b/src/Project.ts
@@ -133,10 +133,11 @@ export default class Project {
       let predicate;
       switch (trigger.trigger) {
         case Trigger.TIMER_GREATER_THAN:
-          predicate = this.timer > trigger.option("VALUE", target)!;
+          predicate = this.timer > (trigger.option("VALUE", target) as number);
           break;
         case Trigger.LOUDNESS_GREATER_THAN:
-          predicate = this.loudness > trigger.option("VALUE", target)!;
+          predicate =
+            this.loudness > (trigger.option("VALUE", target) as number);
           break;
         default:
           throw new Error(`Unimplemented trigger ${String(trigger.trigger)}`);
diff --git a/src/Renderer.ts b/src/Renderer.ts
index 7b116db..c79cae3 100644
--- a/src/Renderer.ts
+++ b/src/Renderer.ts
@@ -163,9 +163,7 @@ export default class Renderer {
   public _createFramebufferInfo(
     width: number,
     height: number,
-    filtering:
-      | WebGLRenderingContext["NEAREST"]
-      | WebGLRenderingContext["LINEAR"],
+    filtering: number,
     stencil = false
   ): FramebufferInfo {
     // Create an empty texture with this skin's dimensions.
diff --git a/src/Sprite.ts b/src/Sprite.ts
index 9ae6392..08a0a05 100644
--- a/src/Sprite.ts
+++ b/src/Sprite.ts
@@ -97,10 +97,11 @@ type InitialConditions = {
 };
 
 abstract class SpriteBase {
-  protected _project!: Project;
+  // TODO: make this protected and pass it in via the constructor
+  public _project!: Project;
 
   protected _costumeNumber: number;
-  protected _layerOrder: number;
+  public _layerOrder: number;
   public triggers: Trigger[];
   public watchers: Partial<Record<string, Watcher>>;
   protected costumes: Costume[];
@@ -701,7 +702,7 @@ export class Sprite extends SpriteBase {
     } while (t < 1);
   }
 
-  ifOnEdgeBounce() {
+  public ifOnEdgeBounce(): void {
     const nearestEdge = this.nearestEdge();
     if (!nearestEdge) return;
     const rad = this.scratchToRad(this.direction);
@@ -725,7 +726,7 @@ export class Sprite extends SpriteBase {
     this.positionInFence();
   }
 
-  positionInFence() {
+  private positionInFence(): void {
     // https://github.com/LLK/scratch-vm/blob/develop/src/sprites/rendered-target.js#L949
     const fence = this.stage.fence;
     const bounds = this._project.renderer.getTightBoundingBox(this);
@@ -869,7 +870,7 @@ export class Sprite extends SpriteBase {
     }
   }
 
-  nearestEdge() {
+  private nearestEdge(): symbol | null {
     const bounds = this._project.renderer.getTightBoundingBox(this);
     const { width: stageWidth, height: stageHeight } = this.stage;
     const distLeft = Math.max(0, stageWidth / 2 + bounds.left);
@@ -953,7 +954,7 @@ export class Sprite extends SpriteBase {
     BOTTOM: Symbol("BOTTOM"),
     LEFT: Symbol("LEFT"),
     RIGHT: Symbol("RIGHT"),
-    TOP: Symbol("TOP")
+    TOP: Symbol("TOP"),
   });
 }
 
@@ -971,7 +972,7 @@ export class Stage extends SpriteBase {
     right: number;
     top: number;
     bottom: number;
-  }
+  };
 
   public constructor(initialConditions: StageInitialConditions, vars = {}) {
     super(initialConditions, vars);
@@ -993,7 +994,7 @@ export class Stage extends SpriteBase {
       left: -this.width / 2,
       right: this.width / 2,
       top: this.height / 2,
-      bottom: -this.height / 2
+      bottom: -this.height / 2,
     };
 
     // For obsolete counter blocks.
diff --git a/src/Watcher.ts b/src/Watcher.ts
index 389d82f..2e15a38 100644
--- a/src/Watcher.ts
+++ b/src/Watcher.ts
@@ -29,7 +29,7 @@ type WatcherOptions = {
 export default class Watcher {
   public value: () => WatcherValue;
   public setValue: (value: number) => void;
-  private _previousValue: unknown | symbol;
+  private _previousValue: unknown;
   private color: Color;
   private _label!: string;
   private _x!: number;

From e1140942b00c77f32248f9d22ca0990a127bcbdb Mon Sep 17 00:00:00 2001
From: adroitwhiz <adroitwhiz@protonmail.com>
Date: Tue, 16 Jul 2024 09:10:52 -0400
Subject: [PATCH 2/5] Add todo for _layerOrder

---
 src/Sprite.ts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Sprite.ts b/src/Sprite.ts
index 08a0a05..809cd82 100644
--- a/src/Sprite.ts
+++ b/src/Sprite.ts
@@ -101,6 +101,7 @@ abstract class SpriteBase {
   public _project!: Project;
 
   protected _costumeNumber: number;
+  // TODO: remove this and just store the sprites in layer order, as Scratch does.
   public _layerOrder: number;
   public triggers: Trigger[];
   public watchers: Partial<Record<string, Watcher>>;

From 6f8e928341676c02762066595d95b6be1c4f7472 Mon Sep 17 00:00:00 2001
From: adroitwhiz <adroitwhiz@protonmail.com>
Date: Tue, 16 Jul 2024 09:14:34 -0400
Subject: [PATCH 3/5] Oops, actually lint all the files

---
 package.json                  | 2 +-
 src/renderer/Drawable.ts      | 4 +++-
 src/renderer/ShaderManager.ts | 7 +------
 src/renderer/Skin.ts          | 4 +---
 4 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/package.json b/package.json
index 1414194..da478a9 100644
--- a/package.json
+++ b/package.json
@@ -27,7 +27,7 @@
   "scripts": {
     "build": "rollup -c",
     "dev": "rollup -c --watch",
-    "lint": "eslint \"./src/**.ts\"",
+    "lint": "eslint \"./src/**/*.ts\"",
     "prepare": "npm run build"
   },
   "devDependencies": {
diff --git a/src/renderer/Drawable.ts b/src/renderer/Drawable.ts
index e1be5bd..96ee474 100644
--- a/src/renderer/Drawable.ts
+++ b/src/renderer/Drawable.ts
@@ -410,7 +410,9 @@ export default class Drawable {
   private _warnBadSize(description: string, treating: string): void {
     if (!this._warnedBadSize) {
       const { name } = this._sprite.constructor;
-      console.warn(`Expected a number, sprite ${name} size is ${description}. Treating as ${treating}.`);
+      console.warn(
+        `Expected a number, sprite ${name} size is ${description}. Treating as ${treating}.`
+      );
       this._warnedBadSize = true;
     }
   }
diff --git a/src/renderer/ShaderManager.ts b/src/renderer/ShaderManager.ts
index 8ed6849..89dd3d1 100644
--- a/src/renderer/ShaderManager.ts
+++ b/src/renderer/ShaderManager.ts
@@ -59,12 +59,7 @@ class ShaderManager {
   }
 
   // Creates and compiles a vertex or fragment shader from the given source code.
-  private _createShader(
-    source: string,
-    type:
-      | WebGLRenderingContext["FRAGMENT_SHADER"]
-      | WebGLRenderingContext["VERTEX_SHADER"]
-  ): WebGLShader {
+  private _createShader(source: string, type: number): WebGLShader {
     const gl = this.gl;
     const shader = gl.createShader(type);
     if (!shader) throw new Error("Could not create shader.");
diff --git a/src/renderer/Skin.ts b/src/renderer/Skin.ts
index 0f29528..17e3e0f 100644
--- a/src/renderer/Skin.ts
+++ b/src/renderer/Skin.ts
@@ -28,9 +28,7 @@ export default abstract class Skin {
   // Helper function to create a texture from an image and handle all the boilerplate.
   protected _makeTexture(
     image: HTMLImageElement | HTMLCanvasElement | null,
-    filtering:
-      | WebGLRenderingContext["NEAREST"]
-      | WebGLRenderingContext["LINEAR"]
+    filtering: number
   ): WebGLTexture {
     const gl = this.gl;
     const glTexture = gl.createTexture();

From 01911477d86d0d6f896acc11b2a52800c785d451 Mon Sep 17 00:00:00 2001
From: adroitwhiz <adroitwhiz@protonmail.com>
Date: Tue, 16 Jul 2024 10:23:35 -0400
Subject: [PATCH 4/5] Maintain list of targets in layer order

This matches how Scratch behaves, and means we don't have to mess around
with _layerOrder anymore.
---
 src/Project.ts  | 103 +++++++++++++++++++++++++++++++++++-------------
 src/Renderer.ts |  50 +++++++++++------------
 src/Sprite.ts   |  53 +++++++++++++------------
 3 files changed, 126 insertions(+), 80 deletions(-)

diff --git a/src/Project.ts b/src/Project.ts
index ebdfa19..a022fe3 100644
--- a/src/Project.ts
+++ b/src/Project.ts
@@ -3,7 +3,7 @@ import Renderer from "./Renderer";
 import Input from "./Input";
 import LoudnessHandler from "./Loudness";
 import Sound from "./Sound";
-import type { Stage, Sprite } from "./Sprite";
+import { Stage, Sprite } from "./Sprite";
 
 type TriggerWithTarget = {
   target: Sprite | Stage;
@@ -13,6 +13,13 @@ type TriggerWithTarget = {
 export default class Project {
   public stage: Stage;
   public sprites: Partial<Record<string, Sprite>>;
+  /**
+   * All rendered targets (the stage, sprites, and clones), in layer order.
+   * This is kept private so that nobody can improperly modify it. The only way
+   * to add or remove targets is via the appropriate methods, and iteration can
+   * be done with {@link forEachTarget}.
+   */
+  private targets: (Sprite | Stage)[];
   public renderer: Renderer;
   public input: Input;
 
@@ -34,12 +41,29 @@ export default class Project {
     this.stage = stage;
     this.sprites = sprites;
 
-    Object.freeze(sprites); // Prevent adding/removing sprites while project is running
+    this.targets = [
+      stage,
+      ...Object.values(this.sprites as Record<string, Sprite>),
+    ];
+    this.targets.sort((a, b) => {
+      // There should only ever be one stage, but it's best to maintain a total
+      // ordering to avoid weird sorting-algorithm stuff from happening if
+      // there's more than one
+      if (a instanceof Stage && !(b instanceof Stage)) {
+        return -1;
+      }
+      if (b instanceof Stage && !(a instanceof Stage)) {
+        return 1;
+      }
 
-    for (const sprite of this.spritesAndClones) {
-      sprite._project = this;
+      return a.getInitialLayerOrder() - b.getInitialLayerOrder();
+    });
+    for (const target of this.targets) {
+      target.clearInitialLayerOrder();
+      target._project = this;
     }
-    this.stage._project = this;
+
+    Object.freeze(sprites); // Prevent adding/removing sprites while project is running
 
     this.renderer = new Renderer(this, null);
     this.input = new Input(this.stage, this.renderer.stage, (key) => {
@@ -77,7 +101,7 @@ export default class Project {
         void Sound.audioContext.resume();
       }
 
-      let clickedSprite = this.renderer.pick(this.spritesAndClones, {
+      let clickedSprite = this.renderer.pick(this.targets, {
         x: this.input.mouse.x,
         y: this.input.mouse.y,
       });
@@ -113,8 +137,7 @@ export default class Project {
     triggerMatches: (tr: Trigger, target: Sprite | Stage) => boolean
   ): TriggerWithTarget[] {
     const matchingTriggers: TriggerWithTarget[] = [];
-    const targets = this.spritesAndStage;
-    for (const target of targets) {
+    for (const target of this.targets) {
       const matchingTargetTriggers = target.triggers.filter((tr) =>
         triggerMatches(tr, target)
       );
@@ -198,15 +221,13 @@ export default class Project {
       this.stopAllSounds();
       this.runningTriggers = [];
 
-      for (const spriteName in this.sprites) {
-        const sprite = this.sprites[spriteName]!;
-        sprite.clones = [];
-      }
+      this.filterSprites((sprite) => {
+        if (!sprite.isOriginal) return false;
 
-      for (const sprite of this.spritesAndStage) {
         sprite.effects.clear();
         sprite.audioEffects.clear();
-      }
+        return true;
+      });
     }
 
     const matchingTriggers = this._matchingTriggers((tr, target) =>
@@ -237,14 +258,41 @@ export default class Project {
     );
   }
 
-  public get spritesAndClones(): Sprite[] {
-    return Object.values(this.sprites)
-      .flatMap((sprite) => sprite!.andClones())
-      .sort((a, b) => a._layerOrder - b._layerOrder);
+  public addSprite(sprite: Sprite, behind?: Sprite): void {
+    if (behind) {
+      const currentIndex = this.targets.indexOf(behind);
+      this.targets.splice(currentIndex, 0, sprite);
+    } else {
+      this.targets.push(sprite);
+    }
+  }
+
+  public removeSprite(sprite: Sprite): void {
+    const index = this.targets.indexOf(sprite);
+    if (index === -1) return;
+
+    this.targets.splice(index, 1);
+    this.cleanupSprite(sprite);
+  }
+
+  public filterSprites(predicate: (sprite: Sprite) => boolean): void {
+    let nextKeptSpriteIndex = 0;
+    for (let i = 0; i < this.targets.length; i++) {
+      const target = this.targets[i];
+      if (target instanceof Stage || predicate(target)) {
+        this.targets[nextKeptSpriteIndex] = target;
+        nextKeptSpriteIndex++;
+      } else {
+        this.cleanupSprite(target);
+      }
+    }
+    this.targets.length = nextKeptSpriteIndex;
   }
 
-  public get spritesAndStage(): (Sprite | Stage)[] {
-    return [...this.spritesAndClones, this.stage];
+  private cleanupSprite(sprite: Sprite): void {
+    this.runningTriggers = this.runningTriggers.filter(
+      ({ target }) => target !== sprite
+    );
   }
 
   public changeSpriteLayer(
@@ -252,7 +300,7 @@ export default class Project {
     layerDelta: number,
     relativeToSprite = sprite
   ): void {
-    const spritesArray = this.spritesAndClones;
+    const spritesArray = this.targets;
 
     const originalIndex = spritesArray.indexOf(sprite);
     const relativeToIndex = spritesArray.indexOf(relativeToSprite);
@@ -264,17 +312,16 @@ export default class Project {
     // Remove sprite from originalIndex and insert at newIndex
     spritesArray.splice(originalIndex, 1);
     spritesArray.splice(newIndex, 0, sprite);
+  }
 
-    // spritesArray is sorted correctly, but to influence
-    // the actual order of the sprites we need to update
-    // each one's _layerOrder property.
-    spritesArray.forEach((sprite, index) => {
-      sprite._layerOrder = index + 1;
-    });
+  public forEachTarget(callback: (target: Sprite | Stage) => void): void {
+    for (const target of this.targets) {
+      callback(target);
+    }
   }
 
   public stopAllSounds(): void {
-    for (const target of this.spritesAndStage) {
+    for (const target of this.targets) {
       target.stopAllOfMySounds();
     }
   }
diff --git a/src/Renderer.ts b/src/Renderer.ts
index c79cae3..247d7ce 100644
--- a/src/Renderer.ts
+++ b/src/Renderer.ts
@@ -303,37 +303,33 @@ export default class Renderer {
         (filter && !filter(layer))
       );
 
-    // Stage
-    if (shouldIncludeLayer(this.project.stage)) {
-      this.renderSprite(this.project.stage, options);
-    }
+    this.project.forEachTarget((target) => {
+      // TODO: just make a `visible` getter for Stage to avoid this rigmarole
+      const visible = "visible" in target ? target.visible : true;
 
-    // Pen layer
-    if (shouldIncludeLayer(this._penSkin)) {
-      const penMatrix = Matrix.create();
-      Matrix.scale(
-        penMatrix,
-        penMatrix,
-        this._penSkin.width,
-        -this._penSkin.height
-      );
-      Matrix.translate(penMatrix, penMatrix, -0.5, -0.5);
+      if (shouldIncludeLayer(target) && visible) {
+        this.renderSprite(target, options);
+      }
 
-      this._renderSkin(
-        this._penSkin,
-        options.drawMode,
-        penMatrix,
-        1 /* scale */
-      );
-    }
+      // Draw the pen layer in front of the stage
+      if (target instanceof Stage && shouldIncludeLayer(this._penSkin)) {
+        const penMatrix = Matrix.create();
+        Matrix.scale(
+          penMatrix,
+          penMatrix,
+          this._penSkin.width,
+          -this._penSkin.height
+        );
+        Matrix.translate(penMatrix, penMatrix, -0.5, -0.5);
 
-    // Sprites + clones
-    for (const sprite of this.project.spritesAndClones) {
-      // Stage doesn't have "visible" defined, so check if it's strictly false
-      if (shouldIncludeLayer(sprite) && sprite.visible !== false) {
-        this.renderSprite(sprite, options);
+        this._renderSkin(
+          this._penSkin,
+          options.drawMode,
+          penMatrix,
+          1 /* scale */
+        );
       }
-    }
+    });
   }
 
   private _updateStageSize(): void {
diff --git a/src/Sprite.ts b/src/Sprite.ts
index 809cd82..c3fbf7a 100644
--- a/src/Sprite.ts
+++ b/src/Sprite.ts
@@ -102,7 +102,7 @@ abstract class SpriteBase {
 
   protected _costumeNumber: number;
   // TODO: remove this and just store the sprites in layer order, as Scratch does.
-  public _layerOrder: number;
+  private _initialLayerOrder: number | null;
   public triggers: Trigger[];
   public watchers: Partial<Record<string, Watcher>>;
   protected costumes: Costume[];
@@ -118,7 +118,7 @@ abstract class SpriteBase {
     // TODO: pass project in here, ideally
     const { costumeNumber, layerOrder = 0 } = initialConditions;
     this._costumeNumber = costumeNumber;
-    this._layerOrder = layerOrder;
+    this._initialLayerOrder = layerOrder;
 
     this.triggers = [];
     this.watchers = {};
@@ -136,6 +136,20 @@ abstract class SpriteBase {
     this._vars = vars;
   }
 
+  public getInitialLayerOrder(): number {
+    const order = this._initialLayerOrder;
+    if (order === null) {
+      throw new Error(
+        "getInitialLayerOrder should only be called once, when the project is initialized"
+      );
+    }
+    return order;
+  }
+
+  public clearInitialLayerOrder(): void {
+    this._initialLayerOrder = null;
+  }
+
   protected getSoundsPlayedByMe(): Sound[] {
     return this.sounds.filter((sound) => this.effectChain.isTargetOf(sound));
   }
@@ -523,8 +537,7 @@ export class Sprite extends SpriteBase {
   public size: number;
   public visible: boolean;
 
-  private parent: this | null;
-  public clones: this[];
+  private original: this;
 
   private _penDown: boolean;
   public penSize: number;
@@ -555,8 +568,7 @@ export class Sprite extends SpriteBase {
     this.size = size;
     this.visible = visible;
 
-    this.parent = null;
-    this.clones = [];
+    this.original = this;
 
     this._penDown = penDown || false;
     this.penSize = penSize || 1;
@@ -569,6 +581,10 @@ export class Sprite extends SpriteBase {
     };
   }
 
+  public get isOriginal(): boolean {
+    return this.original === this;
+  }
+
   public *askAndWait(question: string): Yielding<void> {
     if (this._speechBubble) {
       this.say("");
@@ -599,21 +615,16 @@ export class Sprite extends SpriteBase {
 
     // Clones inherit audio effects from the original sprite, for some reason.
     // Couldn't explain it, but that's the behavior in Scratch 3.0.
-    // eslint-disable-next-line @typescript-eslint/no-this-alias
-    let original = this;
-    while (original.parent) {
-      original = original.parent;
-    }
-    clone.effectChain = original.effectChain.clone({
+    clone.effectChain = this.original.effectChain.clone({
       getNonPatchSoundList: clone.getSoundsPlayedByMe.bind(clone),
     });
 
     // Make a new audioEffects interface which acts on the cloned effect chain.
     clone.audioEffects = new AudioEffectMap(clone.effectChain);
 
-    clone.clones = [];
-    clone.parent = this;
-    this.clones.push(clone);
+    clone.original = this.original;
+
+    this._project.addSprite(clone, this);
 
     // Trigger CLONE_START:
     const triggers = clone.triggers.filter((tr) =>
@@ -625,17 +636,9 @@ export class Sprite extends SpriteBase {
   }
 
   public deleteThisClone(): void {
-    if (this.parent === null) return;
-
-    this.parent.clones = this.parent.clones.filter((clone) => clone !== this);
-
-    this._project.runningTriggers = this._project.runningTriggers.filter(
-      ({ target }) => target !== this
-    );
-  }
+    if (this.isOriginal) return;
 
-  public andClones(): this[] {
-    return [this, ...this.clones.flatMap((clone) => clone.andClones())];
+    this._project.removeSprite(this);
   }
 
   public get direction(): number {

From 104c995ed5dc43c25d2721a010fefa090d8ec184 Mon Sep 17 00:00:00 2001
From: adroitwhiz <adroitwhiz@protonmail.com>
Date: Tue, 16 Jul 2024 10:24:41 -0400
Subject: [PATCH 5/5] Start triggers in top-down order

This matches how Scratch does it.
---
 src/Project.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/Project.ts b/src/Project.ts
index a022fe3..f2a2641 100644
--- a/src/Project.ts
+++ b/src/Project.ts
@@ -137,7 +137,9 @@ export default class Project {
     triggerMatches: (tr: Trigger, target: Sprite | Stage) => boolean
   ): TriggerWithTarget[] {
     const matchingTriggers: TriggerWithTarget[] = [];
-    for (const target of this.targets) {
+    // Iterate over targets in top-down order, as Scratch does
+    for (let i = this.targets.length - 1; i >= 0; i--) {
+      const target = this.targets[i];
       const matchingTargetTriggers = target.triggers.filter((tr) =>
         triggerMatches(tr, target)
       );