Remove useless sprite setting

[DevelopingTom]

Description:

Redundant animated sprite setting: the sprite frame width can be computed directly.

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

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

IngloriousTom

[coderabbitai]

Walkthrough

AnimatedSprite now computes its frameWidth from the image width and frameCount; constructor no longer accepts frameWidth. Sprite loader configs were updated to remove frameWidth and adjusted a frameCount (MiniSmokeAndFire 5→6).

Changes

Cohort / File(s)	Summary
AnimatedSprite Core src/client/graphics/AnimatedSprite.ts	Added private frameWidth field, removed frameWidth parameter from constructor, validate frameCount > 0, require image.width and image.height, compute frameWidth = image.width / frameCount, updated draw slicing to use computed frameWidth.
Sprite Configuration & Loader src/client/graphics/AnimatedSpriteLoader.ts	Removed frameWidth from AnimatedSpriteConfig and all instantiations; stopped passing frameWidth into AnimatedSprite. Adjusted MiniSmokeAndFire config: frameCount increased from 5 to 6.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A sprite once told its width by hand,
Now it counts its frames and makes a stand.
Fewer params, a tidy seam,
Pixels march in neat new streams ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Remove useless sprite setting' directly describes the main change: removing the redundant frameWidth parameter that can now be computed automatically.
Description check	✅ Passed	The description explains the rationale (frameWidth can be computed directly) and confirms all required checklist items are complete, relating clearly to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/client/graphics/AnimatedSprite.ts (1)

16-25: Good refactor: frameWidth is now computed internally.

This simplifies the API and removes redundant configuration. The duck-type check for both width and height is correct for CanvasImageSource.

One small concern: no guard against frameCount <= 0. If someone passes 0 or negative value, Math.floor(width / 0) yields Infinity, leading to broken rendering with no clear error.

Consider adding a simple guard:

🛡️ Optional defensive check

   constructor(
     private image: CanvasImageSource,
     private frameCount: number,
     private frameDuration: number, // in milliseconds
     private looping: boolean = false,
     private originX: number,
     private originY: number,
   ) {
+    if (frameCount <= 0) {
+      throw new Error("frameCount must be greater than 0.");
+    }
     if ("height" in image && "width" in image) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/client/graphics/AnimatedSprite.ts` around lines 16 - 25, The code in
AnimatedSprite that computes frameWidth/frameHeight from the provided image
lacks validation for frameCount, so passing 0 or a negative number yields
Infinity or NaN; update the constructor or setter that uses frameCount
(referencing variables/methods: frameCount, frameWidth, frameHeight, and the
image type check in AnimatedSprite) to validate that frameCount is a positive
integer (>0) before computing frameWidth and throw a clear Error (e.g.,
"frameCount must be a positive integer") if the check fails; perform this guard
immediately before the Math.floor(width / frameCount) calculation to prevent
invalid values propagating.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/client/graphics/AnimatedSprite.ts`:
- Around line 16-25: The code in AnimatedSprite that computes
frameWidth/frameHeight from the provided image lacks validation for frameCount,
so passing 0 or a negative number yields Infinity or NaN; update the constructor
or setter that uses frameCount (referencing variables/methods: frameCount,
frameWidth, frameHeight, and the image type check in AnimatedSprite) to validate
that frameCount is a positive integer (>0) before computing frameWidth and throw
a clear Error (e.g., "frameCount must be a positive integer") if the check
fails; perform this guard immediately before the Math.floor(width / frameCount)
calculation to prevent invalid values propagating.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cd51ff4a-8b10-4000-b758-2682fff77575

📥 Commits

Reviewing files that changed from the base of the PR and between 47ef5a0 and 0468dce.

📒 Files selected for processing (2)

• src/client/graphics/AnimatedSprite.ts
• src/client/graphics/AnimatedSpriteLoader.ts

[coderabbitai]

🧹 Nitpick comments (1)

src/client/graphics/AnimatedSprite.ts (1)

19-27: Frame width calculation looks solid.

Using Math.floor is a sensible choice — it ensures integer pixel values and gracefully handles sprite sheets where width is not perfectly divisible by frame count (some trailing pixels simply remain unused).

One small note: if an image's width is smaller than frameCount, you get a frameWidth of 0, which would produce invisible frames. This is unlikely in practice but worth knowing. If you want extra safety, you could add a check after computing frameWidth:

💡 Optional: guard against zero-width frames

      this.frameWidth = Math.floor(
        (image as HTMLImageElement | HTMLCanvasElement).width / frameCount,
      );
+     if (this.frameWidth <= 0) {
+       throw new Error("Computed frame width must be at least 1 pixel");
+     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/client/graphics/AnimatedSprite.ts` around lines 19 - 27, After computing
frameWidth in AnimatedSprite (using frameCount and the image width), add a guard
to detect zero or non-positive frameWidth (e.g., frameWidth <= 0) and handle it:
either throw a clear Error mentioning frameCount and image width or clamp to a
minimum of 1 to avoid invisible frames; place this check immediately after the
calculation that sets this.frameWidth so it runs when the image width is smaller
than frameCount (use symbols: AnimatedSprite, this.frameWidth, frameCount,
image).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/client/graphics/AnimatedSprite.ts`:
- Around line 19-27: After computing frameWidth in AnimatedSprite (using
frameCount and the image width), add a guard to detect zero or non-positive
frameWidth (e.g., frameWidth <= 0) and handle it: either throw a clear Error
mentioning frameCount and image width or clamp to a minimum of 1 to avoid
invisible frames; place this check immediately after the calculation that sets
this.frameWidth so it runs when the image width is smaller than frameCount (use
symbols: AnimatedSprite, this.frameWidth, frameCount, image).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9b5aa1d1-897c-4d2f-b433-9fe4a1355686

📥 Commits

Reviewing files that changed from the base of the PR and between 0468dce and 399401d.

📒 Files selected for processing (1)

• src/client/graphics/AnimatedSprite.ts

[evanpelle]

thanks!