feat(web): bake and wire LPC skins for all 12 council NPCs - #18
Conversation
Extend the LPC bake pipeline and runtime mapping so player + npc-1…12 use dedicated walk/idle atlases instead of Stardew fallbacks for council seats. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR expands LPC sprite assets to all players and council NPCs ChangesLPC visuals and map rendering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AssetCommand
participant BakeScript
participant AssetFiles
participant AssetManifest
participant RoomScene
AssetCommand->>BakeScript: run assets:sync:lpc-npcs
BakeScript->>AssetFiles: read player-1 and npc-1 through npc-12 sources
BakeScript->>AssetFiles: write generated LPC atlases
AssetManifest->>AssetFiles: register LPC spritesheets
RoomScene->>AssetManifest: verify LPC textures
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
apps/web/src/game/lpcNpc1Sheet.ts (1)
36-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNaming drift: generic-profile functions still carry the
Npc1-specific name.
lpcNpc1WalkCycleMs/lpcNpc1WalkLoopFrameRange(and the unchangedlpcNpc1FrameIndex/lpcNpc1NameplateY) now accept/default an arbitraryLpcNpcSpriteProfileand are invoked for every profile — including the player — fromentitySprites.ts(e.g.registerLpcNpcAnimsForProfile,createLpcNpcSprite). MeanwhilelpcNpcAnimKey/registerLpcNpcAnimswere properly generalized. The remainingNpc1-prefixed names are now misleading about scope.Consider renaming to generic names (e.g.
lpcWalkCycleMs,lpcWalkLoopFrameRange,lpcFrameIndex,lpcNameplateY) with thin@deprecatedNpc1-named wrappers, mirroring the pattern already used forlpcNpcAnimKey/lpcNpc1AnimKey.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/game/lpcNpc1Sheet.ts` around lines 36 - 56, Rename the profile-agnostic helpers lpcNpc1WalkCycleMs, lpcNpc1WalkLoopFrameRange, lpcNpc1FrameIndex, and lpcNpc1NameplateY to generic names such as lpcWalkCycleMs, lpcWalkLoopFrameRange, lpcFrameIndex, and lpcNameplateY, and update all callers including entitySprites.ts. Retain thin `@deprecated` Npc1-prefixed wrappers for compatibility, following the existing lpcNpcAnimKey/lpcNpc1AnimKey pattern.apps/web/src/game/assetManifest.ts (1)
97-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider reusing
LPC_COUNCIL_NPC_IDSto avoid hardcoding the NPC indices array.You can replace the inline array with the exported constant
LPC_COUNCIL_NPC_IDSfromlpcNpc1Sheet.tsto keep the council IDs defined in a single source of truth.♻️ Proposed refactor
First, add the import at the top of the file:
+import { LPC_COUNCIL_NPC_IDS } from "./lpcNpc1Sheet";Then replace the inline array:
- ...([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12] as const).map( + ...LPC_COUNCIL_NPC_IDS.map( (n): AssetSheetDef => ({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/game/assetManifest.ts` around lines 97 - 110, Update the NPC spritesheet mapping in the asset manifest to iterate over the exported LPC_COUNCIL_NPC_IDS constant from lpcNpc1Sheet.ts instead of the inline numeric array, adding the necessary import while preserving the existing key and URL generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/DEVELOPMENT-HISTORY.zh-CN.md`:
- Line 858: Update the zh-CN Phase 26.1 交付物 entry in
DEVELOPMENT-HISTORY.zh-CN.md to include the missing entitySprites LPC path
deliverable, matching the corresponding English entry while preserving the
existing listed assets and files.
---
Nitpick comments:
In `@apps/web/src/game/assetManifest.ts`:
- Around line 97-110: Update the NPC spritesheet mapping in the asset manifest
to iterate over the exported LPC_COUNCIL_NPC_IDS constant from lpcNpc1Sheet.ts
instead of the inline numeric array, adding the necessary import while
preserving the existing key and URL generation.
In `@apps/web/src/game/lpcNpc1Sheet.ts`:
- Around line 36-56: Rename the profile-agnostic helpers lpcNpc1WalkCycleMs,
lpcNpc1WalkLoopFrameRange, lpcNpc1FrameIndex, and lpcNpc1NameplateY to generic
names such as lpcWalkCycleMs, lpcWalkLoopFrameRange, lpcFrameIndex, and
lpcNameplateY, and update all callers including entitySprites.ts. Retain thin
`@deprecated` Npc1-prefixed wrappers for compatibility, following the existing
lpcNpcAnimKey/lpcNpc1AnimKey pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 31c9667b-64d3-4a50-91db-b8d24bdf84b2
⛔ Files ignored due to path filters (12)
apps/web/public/assets/sprites/lpc-npc-10.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-11.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-12.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-2.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-3.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-4.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-5.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-6.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-7.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-8.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-npc-9.pngis excluded by!**/*.pngapps/web/public/assets/sprites/lpc-player-1.pngis excluded by!**/*.png
📒 Files selected for processing (17)
AGENTS.mdapps/web/AGENTS.mdapps/web/public/assets/CREDITS.mdapps/web/src/game/HomeMapBackground.tsapps/web/src/game/RoomScene.tsapps/web/src/game/assetManifest.tsapps/web/src/game/entityLayout.test.tsapps/web/src/game/entityLayout.tsapps/web/src/game/entitySprites.tsapps/web/src/game/lpcNpc1Sheet.test.tsapps/web/src/game/lpcNpc1Sheet.tsdocs/BEGINNING-FIELDS.mddocs/DEVELOPMENT-HISTORY.mddocs/DEVELOPMENT-HISTORY.zh-CN.mddocs/ISSUE-LOG.mdpackage.jsonscripts/sync-npc-lpc-assets.mjs
Include entitySprites LPC path so bilingual DEVELOPMENT-HISTORY entries match. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
npc-assetforplayer-1+npc-1…npc-12(pnpm assets:sync:lpc-npcs)npcs.pngfallbackTest plan
pnpm assets:sync:lpc-npcs→ 13× 704×256 atlasespnpm --filter @aetherlife/web test(pre-push)pytest(pre-push)pnpm dev:stack→ hard refresh → confirm npc-1…12 skins on mappnpm verify:phase6:move-only+pnpm verify:phase13Made with Cursor
Summary by CodeRabbit
New Features
npc-1throughnpc-12.Documentation
Tests