Conversation
WalkthroughThis PR swaps many sprite assets for Halloween-themed variants, adds four new FX types (Tentacle, Shark, Bubble, Tornado), implements MoveSpriteFx for moving sprite effects, and adds a randomized FX event generator in FxLayer. Also updates some emoji and pastel theme color values. Changes
Sequence Diagram(s)sequenceDiagram
participant FxLayer
participant RandomEvent
participant SpriteFx
participant MoveSpriteFx
participant FadeFx
FxLayer->>FxLayer: tick() ++lastRandomEvent
alt lastRandomEvent > randomEventRate
FxLayer->>RandomEvent: randomEvent()
RandomEvent->>RandomEvent: pick randX, randY
alt tile is ocean (not shoreline)
RandomEvent->>SpriteFx: spawn Shark or Bubble (SpriteFx)
RandomEvent->>MoveSpriteFx: spawn Tornado (MoveSpriteFx→SpriteFx) -> target coords
RandomEvent->>FadeFx: spawn Tentacle (FadeFx→SpriteFx)
else not ocean
RandomEvent->>FadeFx: spawn MiniSmoke ghost (FadeFx→SpriteFx)
end
RandomEvent->>FxLayer: push FX into allFx
FxLayer->>FxLayer: reset lastRandomEvent = 0
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (2)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/client/graphics/SpriteLoader.ts (1)
157-166: Fix cache key: include theme’s spawn highlight color.colorizeCanvas uses spawnHighlightColor, but computeSpriteKey omits it. After a theme swap, cached canvases can be wrong. Include it in the key.
- function computeSpriteKey( - unit: UnitView, - territoryColor: Colord, - borderColor: Colord, - ): string { + function computeSpriteKey( + unit: UnitView, + territoryColor: Colord, + borderColor: Colord, + spawnHighlightColor: Colord, +): string { const owner = unit.owner(); const type = `${unit.type()}-${unit.trainType()}-${unit.isLoaded()}`; - const key = `${type}-${owner.id()}-${territoryColor.toRgbString()}-${borderColor.toRgbString()}`; + const key = `${type}-${owner.id()}-${territoryColor.toRgbString()}-${borderColor.toRgbString()}-${spawnHighlightColor.toRgbString()}`; return key; } export const getColoredSprite = ( unit: UnitView, theme: Theme, customTerritoryColor?: Colord, customBorderColor?: Colord, ): HTMLCanvasElement => { const territoryColor: Colord = customTerritoryColor ?? unit.owner().territoryColor(); const borderColor: Colord = customBorderColor ?? unit.owner().borderColor(); const spawnHighlightColor = theme.spawnHighlightColor(); - const key = computeSpriteKey(unit, territoryColor, borderColor); + const key = computeSpriteKey(unit, territoryColor, borderColor, spawnHighlightColor);Also applies to: 168-197
src/client/graphics/layers/FxLayer.ts (1)
382-390: Await sprite preload in init() or errors/logging won’t behave.Without await, try/catch won’t catch and “loaded successfully” logs too early.
async init() { this.redraw(); try { - this.animatedSpriteLoader.loadAllAnimatedSpriteImages(); - console.log("FX sprites loaded successfully"); + await this.animatedSpriteLoader.loadAllAnimatedSpriteImages(); + console.log("FX sprites loaded successfully"); } catch (err) { console.error("Failed to load FX sprites:", err); } }src/client/graphics/AnimatedSpriteLoader.ts (1)
247-260: Fix colored animation cache key to avoid wrong-tinted sprites after theme change.colorizeCanvas uses spawnHighlightColor, but the key is only fxType-owner. Include colors (or at least spawnHighlightColor).
- const territoryColor = owner.territoryColor(); - const borderColor = owner.borderColor(); - const spawnHighlightColor = theme.spawnHighlightColor(); - const key = `${fxType}-${owner.id()}`; + const territoryColor = owner.territoryColor(); + const borderColor = owner.borderColor(); + const spawnHighlightColor = theme.spawnHighlightColor(); + const key = `${fxType}-${owner.id()}-${territoryColor.toRgbString()}-${borderColor.toRgbString()}-${spawnHighlightColor.toRgbString()}`;
🧹 Nitpick comments (2)
src/client/graphics/SpriteLoader.ts (1)
55-66: Optional: prefer Image.decode() for faster, cleaner preload.decode() gives a promise and better error propagation. Keep onload/onerror as fallback for older browsers.
- await new Promise<void>((resolve, reject) => { - img.onload = () => resolve(); - img.onerror = (err) => reject(err); - }); + if ('decode' in img) { + await (img as HTMLImageElement).decode(); + } else { + await new Promise<void>((resolve, reject) => { + img.onload = () => resolve(); + img.onerror = (err) => reject(err); + }); + }src/client/graphics/fx/SpriteFx.ts (1)
81-83: Keep position updates behind a method (composition-friendly).Public fields work, but a setter is cleaner and safer. Consider adding setPosition(x, y) and reverting x/y to private in a follow-up.
// In SpriteFx setPosition(x: number, y: number) { this.x = x; this.y = y; } // In MoveSpriteFx.renderTick: this.fxToMove.setPosition( Math.floor(this.originX * (1 - t) + this.toX * t), Math.floor(this.originY * (1 - t) + this.toY * t), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (19)
resources/images/buildings/cityAlt1.pngis excluded by!**/*.pngresources/images/buildings/factoryAlt1.pngis excluded by!**/*.pngresources/images/buildings/fortAlt3.pngis excluded by!**/*.pngresources/images/buildings/port1.pngis excluded by!**/*.pngresources/images/buildings/silo1.pngis excluded by!**/*.pngresources/images/buildings/silo4.pngis excluded by!**/*.pngresources/sprites/halloween/bats.pngis excluded by!**/*.pngresources/sprites/halloween/bubble.pngis excluded by!**/*.pngresources/sprites/halloween/ghost.pngis excluded by!**/*.pngresources/sprites/halloween/miniPumpkin.pngis excluded by!**/*.pngresources/sprites/halloween/minifireGreen.pngis excluded by!**/*.pngresources/sprites/halloween/pumpkin.pngis excluded by!**/*.pngresources/sprites/halloween/sam_explosion.pngis excluded by!**/*.pngresources/sprites/halloween/shark.pngis excluded by!**/*.pngresources/sprites/halloween/skull.pngis excluded by!**/*.pngresources/sprites/halloween/skullNuke.pngis excluded by!**/*.pngresources/sprites/halloween/smokeAndFireGreen.pngis excluded by!**/*.pngresources/sprites/halloween/tentacle.pngis excluded by!**/*.pngresources/sprites/halloween/tornado.pngis excluded by!**/*.png
📒 Files selected for processing (9)
src/client/graphics/AnimatedSpriteLoader.ts(5 hunks)src/client/graphics/SpriteLoader.ts(2 hunks)src/client/graphics/fx/ConquestFx.ts(0 hunks)src/client/graphics/fx/Fx.ts(1 hunks)src/client/graphics/fx/SpriteFx.ts(2 hunks)src/client/graphics/layers/FxLayer.ts(3 hunks)src/core/Util.ts(1 hunks)src/core/configuration/PastelTheme.ts(2 hunks)src/core/execution/utils/BotBehavior.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/client/graphics/fx/ConquestFx.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/client/graphics/fx/SpriteFx.ts (1)
src/client/graphics/fx/Fx.ts (1)
Fx(1-3)
src/client/graphics/layers/FxLayer.ts (3)
src/client/graphics/layers/Layer.ts (1)
Layer(1-7)src/core/game/GameView.ts (1)
ref(658-660)src/client/graphics/fx/SpriteFx.ts (3)
SpriteFx(74-121)MoveSpriteFx(25-49)FadeFx(54-69)
🪛 GitHub Actions: 🧪 CI
src/client/graphics/AnimatedSpriteLoader.ts
[warning] 1-1: Code style issues found in the file. Run 'npx prettier --write' to fix.
[error] 1-1: Prettier formatting check failed. Run 'npx prettier --write' to fix code style issues in this file.
🔇 Additional comments (7)
src/core/configuration/PastelTheme.ts (1)
21-21: Color updates align with the autumn theme.The new shore, water, and shorelineWater colors create the intended Halloween ambiance. The purple-toned water (80, 76, 179) and tan shore (223, 187, 132) fit the autumn aesthetic.
Also applies to: 29-30
src/core/Util.ts (2)
295-295: Halloween emojis added correctly.The Halloween emojis 👻 and 🎃 are now in the emoji table and match the EMOJI_HECKLE update in BotBehavior.ts. The dynamic lookup using emojiId() will find these correctly.
292-292: No issues found with emoji value changes.The codebase uses dynamic emoji lookup via
indexOf()(inBotBehavior.ts) and loop-based iteration (inRadialMenuElements.ts), not hardcoded indices. The emoji values used in bot behavior (👍, ⛵, 🤝, 🎯, 🥱, 🤦♂️, 🥺, 💀, 🕊️, 👎, 👻, 🎃) are matched by content, not position. Changing the first row emojis does not break any functionality.src/core/execution/utils/BotBehavior.ts (1)
23-23: Halloween emoji update looks good.The EMOJI_HECKLE constant now uses Halloween-themed emojis. The dynamic lookup via emojiId() ensures the correct indices are found at runtime, and both 👻 and 🎃 exist in the updated emojiTable.
src/client/graphics/SpriteLoader.ts (1)
29-30: LGTM on Halloween sprite swap.AtomBomb → miniPumpkin and HydrogenBomb → pumpkin mapping is clear and isolated. No API churn.
src/client/graphics/fx/Fx.ts (1)
19-22: Enum additions look good.New FxType members are additive and consistent with loader configs.
src/client/graphics/AnimatedSpriteLoader.ts (1)
1-1: Verify formatting locally or check CI logs for specific Prettier failures.I could not run Prettier in the sandbox environment because dependencies weren't installed. While
prettier(3.5.3) andprettier-plugin-organize-imports(4.1.0) are defined inpackage.json, the sandbox lacks the npm install step.The file
AnimatedSpriteLoader.tsis visible, and imports appear organized (sprite paths grouped together, then framework imports). However, without successfully running Prettier, I cannot confirm the exact formatting violations or whether they've been resolved.To verify:
- Run
npm installlocally (or in CI logs) and executenpx prettier --check src/client/graphics/AnimatedSpriteLoader.tsto see the actual violations- Check your CI logs to confirm what Prettier reported
- Run
npx prettier --write src/client/graphics/AnimatedSpriteLoader.tsto auto-fix if needed
This reverts commit b69adf7.
This reverts commit b69adf7.
3 participants
Description:
Changed theme colors for an "autumn" ambiance:
Changed structures pixel art:
Change existing FX with new halloween-themed ones:
skull.mp4
Added new FX playing randomly on the map:
tentacle.mp4
Added a couple of new emojis, which are used by the bots when attacked:
👻🎃
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
IngloriousTom