From 1c6c4c0512719d467b5d27463efcaec7af6fb584 Mon Sep 17 00:00:00 2001
From: Rouillard <jon@rouillard.org>
Date: Sat, 10 May 2025 15:12:48 +0200
Subject: [PATCH] Clear the sprites in a first loop before drawing (#692)

## Description:
Split the clearRect from the drawImage in a first loop so that moving
units don't erase other units. There should not be a perfomance
lowering.
There is still an issue of unit erasing other units when the other unit
is not updated (unit didn't move) but it's a rare case and the fix would
be to update all units regardless of their activity, so I think this
might be a good compromise, performance wise.

before:

![image](https://github.com/user-attachments/assets/c0f500ca-2465-4986-9755-0d5f07a17bd7)

after:

![image](https://github.com/user-attachments/assets/ebaef73e-9bf1-485f-8a79-da96c949e4f5)


## Please complete the following:

- [x] I have added screenshots for all UI updates
- [x] I confirm I have thoroughly tested these changes and take full
responsibility for any bugs introduced
- [x] I understand that submitting code with bugs that could have been
caught through manual testing blocks releases and new features for all
contributors

## Please put your Discord username so you can be contacted if a bug or
regression is found:

Vivacious Box
---
 src/client/graphics/layers/UnitLayer.ts | 58 ++++++++++++++++---------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/src/client/graphics/layers/UnitLayer.ts b/src/client/graphics/layers/UnitLayer.ts
index afbd8fe7f..b071fd9cc 100644
--- a/src/client/graphics/layers/UnitLayer.ts
+++ b/src/client/graphics/layers/UnitLayer.ts
@@ -15,7 +15,11 @@ import { MoveWarshipIntentEvent } from "../../Transport";
 import { TransformHandler } from "../TransformHandler";
 import { Layer } from "./Layer";
 
-import { getColoredSprite, loadAllSprites } from "../SpriteLoader";
+import {
+  getColoredSprite,
+  isSpriteReady,
+  loadAllSprites,
+} from "../SpriteLoader";
 
 enum Relationship {
   Self,
@@ -65,9 +69,8 @@ export class UnitLayer implements Layer {
     if (this.myPlayer == null) {
       this.myPlayer = this.game.playerByClientID(this.clientID);
     }
-    this.game.updatesSinceLastTick()?.[GameUpdateType.Unit]?.forEach((unit) => {
-      this.onUnitEvent(this.game.unit(unit.id));
-    });
+
+    this.updateUnitsSprites();
   }
 
   init() {
@@ -194,11 +197,9 @@ export class UnitLayer implements Layer {
     this.canvas.height = this.game.height();
     this.transportShipTrailCanvas.width = this.game.width();
     this.transportShipTrailCanvas.height = this.game.height();
-    this.game
-      ?.updatesSinceLastTick()
-      ?.[GameUpdateType.Unit]?.forEach((unit) => {
-        this.onUnitEvent(this.game.unit(unit.id));
-      });
+
+    this.updateUnitsSprites();
+
     this.boatToTrail.forEach((trail, unit) => {
       for (const t of trail) {
         this.paintCell(
@@ -213,6 +214,34 @@ export class UnitLayer implements Layer {
     });
   }
 
+  private updateUnitsSprites() {
+    const unitsToUpdate = this.game
+      .updatesSinceLastTick()
+      ?.[GameUpdateType.Unit]?.map((unit) => this.game.unit(unit.id));
+    unitsToUpdate
+      ?.filter((UnitView) => isSpriteReady(UnitView.type()))
+      .forEach((unitView) => {
+        this.clearUnitCells(unitView);
+      });
+    unitsToUpdate?.forEach((unitView) => {
+      this.onUnitEvent(unitView);
+    });
+  }
+
+  private clearUnitCells(unit: UnitView) {
+    const sprite = getColoredSprite(unit, this.theme);
+    const clearsize = sprite.width + 1;
+
+    const lastX = this.game.x(unit.lastTile());
+    const lastY = this.game.y(unit.lastTile());
+    this.context.clearRect(
+      lastX - clearsize / 2,
+      lastY - clearsize / 2,
+      clearsize,
+      clearsize,
+    );
+  }
+
   private relationship(unit: UnitView): Relationship {
     if (this.myPlayer == null) {
       return Relationship.Enemy;
@@ -419,8 +448,6 @@ export class UnitLayer implements Layer {
   drawSprite(unit: UnitView, customTerritoryColor?: Colord) {
     const x = this.game.x(unit.tile());
     const y = this.game.y(unit.tile());
-    const lastX = this.game.x(unit.lastTile());
-    const lastY = this.game.y(unit.lastTile());
 
     let alternateViewColor = null;
 
@@ -457,15 +484,6 @@ export class UnitLayer implements Layer {
       alternateViewColor,
     );
 
-    const clearsize = sprite.width + 1;
-
-    this.context.clearRect(
-      lastX - clearsize / 2,
-      lastY - clearsize / 2,
-      clearsize,
-      clearsize,
-    );
-
     if (unit.isActive()) {
       this.context.drawImage(
         sprite,