fix: GraphicsGroup null/undefined guard to prevent crash

excaliburjs · Feb 9, 2024 · 3377b41 · 3377b41
1 parent 9449d94
commit 3377b41
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -203,6 +203,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 
+- Fixed issue where a null/undefined graphics group member graphic would cause a crash, now logs a warning.
 - Fixed issue where Actor built in components could not be extended because of the way the Actor based type was built.
   - Actors now use instance properties for built-ins instead of getters
   - With the ECS refactor you can now subtype built-in `Components` and `.get(Builtin)` will return the correct subtype.

diff --git a/sandbox/tests/graphics-group/heart.png b/sandbox/tests/graphics-group/heart.png
diff --git a/sandbox/tests/graphics-group/index.html b/sandbox/tests/graphics-group/index.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <title>Graphics Padding Test</title>
+</head>
+<body>
+  <script src="../../lib/excalibur.js"></script>
+  <script src="index.js"></script>
+</body>
+</html>
diff --git a/sandbox/tests/graphics-group/index.ts b/sandbox/tests/graphics-group/index.ts
@@ -0,0 +1,53 @@
+/// <reference path="../../lib/excalibur.d.ts" />
+
+var game = new ex.Engine({
+  width: 1000,
+  height: 1000,
+});
+
+var heartImage = new ex.ImageSource('./heart.png');
+
+var loader = new ex.Loader([heartImage])
+
+class MyActor2 extends ex.Actor {
+  constructor() {
+    super({
+      pos: ex.vec(200, 200)
+    });
+  }
+  onInitialize() {
+    this.graphics.add(
+      "interactive",
+      new ex.GraphicsGroup({
+        members: [
+          {
+            graphic: undefined,
+            offset: ex.vec(8, 8),
+          },
+          {
+            graphic: heartImage.toSprite(),
+            offset: ex.vec(8, -16),
+          },
+        ],
+      }),
+      {
+        anchor: ex.vec(0, 0),
+      }
+    );
+    this.graphics.add(
+      "noninteractive",
+      heartImage.toSprite(),
+      {
+        anchor: ex.vec(8, 8),
+      }
+    )
+  }
+
+  onPreUpdate(engine: ex.Engine<any>, delta: number): void {
+    this.graphics.use("interactive");
+  }
+}
+
+game.add(new MyActor2());
+
+game.start(loader)
diff --git a/src/engine/Graphics/GraphicsGroup.ts b/src/engine/Graphics/GraphicsGroup.ts
@@ -3,6 +3,7 @@ import { Graphic, GraphicOptions } from './Graphic';
 import { Animation, HasTick } from './Animation';
 import { ExcaliburGraphicsContext } from './Context/ExcaliburGraphicsContext';
 import { BoundingBox } from '../Collision/Index';
+import { Logger } from '../Util/Log';
 
 export interface GraphicsGroupingOptions {
   members: (GraphicsGrouping | Graphic)[];
@@ -14,6 +15,7 @@ export interface GraphicsGrouping {
 }
 
 export class GraphicsGroup extends Graphic implements HasTick {
+  private _logger = Logger.getInstance();
   public members: (GraphicsGrouping | Graphic)[] = [];
 
   constructor(options: GraphicsGroupingOptions & GraphicOptions) {
@@ -43,7 +45,11 @@ export class GraphicsGroup extends Graphic implements HasTick {
         bb = member.localBounds.combine(bb);
       } else {
         const { graphic, offset: pos } = member;
-        bb = graphic.localBounds.translate(pos).combine(bb);
+        if (graphic) {
+          bb = graphic.localBounds.translate(pos).combine(bb);
+        } else {
+          this._logger.warnOnce(`Graphics group member has an null or undefined graphic, member definition: ${JSON.stringify(member)}.`);
+        }
       }
     }
     return bb;
@@ -96,6 +102,9 @@ export class GraphicsGroup extends Graphic implements HasTick {
         graphic = member.graphic;
         member.offset.clone(pos);
       }
+      if (!graphic) {
+        continue;
+      }
       ex.save();
       ex.translate(x, y);
       graphic.draw(ex, pos.x, pos.y);

diff --git a/src/engine/Scene.ts b/src/engine/Scene.ts
@@ -418,7 +418,7 @@ implements CanInitialize, CanActivate<TActivationData>, CanDeactivate, CanUpdate
    */
   public update(engine: Engine, delta: number) {
     if (!this.isInitialized) {
-      throw new Error('Scene update called before it was initialized!');
+      throw new Error('Scene update called before it was initialized! Was there an error in actor or entity initialization?');
     }
     this._preupdate(engine, delta);