Refactor GameBoard Rendering to Layered Architecture#140
Conversation
Rewrote the GameBoard's rendering loop to use a structured, layered architecture (Background, Decal, World, Foreground). This improves performance by reducing object allocations and simplifying sorting to only the World layer entities. - Introduced RenderingContext to centralize shared drawing state. - Implemented row-based depth sorting for the World layer. - Optimized paint reuse and coordinate calculations. - Updated AGENTS.md and fixes.md. Co-authored-by: candour <4670475+candour@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors GameBoard rendering into four ordered layers (Background, Decals, World, Foreground), adds a RenderingContext for geometry/drawing, simplifies tap-to-axial mapping, and changes depth sorting so only the World layer is ordered by axial r. ChangesGameBoard Layered Rendering Refactor
Sequence Diagram(s)sequenceDiagram
participant User
participant Canvas
participant GameBoard
participant GridUtils
User->>Canvas: tap at offset
Canvas->>GameBoard: deliver touch offset
GameBoard->>GameBoard: compute fractional q,r (inverse projection)
GameBoard->>GridUtils: hexRound(q,r)
GridUtils-->>GameBoard: AxialCoordinate
GameBoard->>GameBoard: if cell exists -> onCellClick
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Build Successful! 🚀Note: This link will be removed when the PR is closed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/messark/hawker/ui/components/GameBoard.kt (2)
411-411: ⚡ Quick winAvoid per-frame allocation when sorting the World layer.
sortedByallocates a new list each frame. In-place sorting keeps the hot render path leaner.♻️ Proposed change
- worldLayer.sortedBy { it.r }.forEach { it.draw(this) } + worldLayer.sortBy { it.r } + worldLayer.forEach { it.draw(this) }
243-244: ⚡ Quick winCapture frame time once per draw pass.
Time is sampled repeatedly inside render lambdas. Using a single frame timestamp improves consistency and trims repeated system calls.
♻️ Proposed change
val ctx = RenderingContext( wPx = hexWidth.toPx(), hPx = hexHeight.toPx(), rowSpacingFactor = rowSpacingFactor, borderPx = 20.dp.toPx(), spriteSheet, stallsSheet, enemiesSheet, endTableSheet, upgradePaint, spritePaint ) + val frameTimeMs = System.currentTimeMillis() @@ - val p = (System.currentTimeMillis() % 1000) / 1000f + val p = (frameTimeMs % 1000L) / 1000f @@ - val p = ((System.currentTimeMillis() - effect.startTimeMs).toFloat() / effect.durationMs).coerceIn(0f, 1f) + val p = ((frameTimeMs - effect.startTimeMs).toFloat() / effect.durationMs).coerceIn(0f, 1f)Also applies to: 366-367
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 304278ae-ca8e-4896-935b-6cd69004a5b2
📒 Files selected for processing (3)
AGENTS.mdapp/src/main/java/com/messark/hawker/ui/components/GameBoard.ktfixes.md
|
Avoid per-frame allocation when sorting the World layer. sortedBy allocates a new list each frame. In-place sorting keeps the hot render path leaner. ♻️ Proposed change Capture frame time once per draw pass. Time is sampled repeatedly inside render lambdas. Using a single frame timestamp improves consistency and trims repeated system calls. |
Thanks for the feedback! I've implemented both optimizations. The frame timestamp is now captured once per draw pass via |
- Capture frame timestamp once per draw pass via RenderingContext. - Use in-place sortBy for World layer depth sorting to avoid per-frame allocations. - Replace System.currentTimeMillis() calls with cached timestamp. Co-authored-by: candour <4670475+candour@users.noreply.github.com>
Build Successful! 🚀Note: This link will be removed when the PR is closed. |
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d8444955-dfac-4ae7-9ed7-a81db7d18346
📒 Files selected for processing (1)
app/src/main/java/com/messark/hawker/ui/components/GameBoard.kt
- Switched Background, Decal, and Foreground layers to direct drawing to eliminate per-frame object allocations. - Implemented proper cube coordinate rounding for tap detection via GridUtils.hexRound. - Fixed pillar removal pulse animation by applying the scale factor to the hex path. - Optimized GAS_CLOUD effect drawing by moving it to the direct-drawn Decal layer. Co-authored-by: candour <4670475+candour@users.noreply.github.com>
Build Successful! 🚀Note: This link will be removed when the PR is closed. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Switched to direct drawing for most layers to eliminate per-frame allocations. - Implemented a WorldItem pool and reused active items list to avoid GC churn during depth sorting. - Reused Path objects and cached frame timestamps via RenderingContext. - Implemented mathematically correct cube rounding in GridUtils.hexRound for precise hexagonal grid interaction. - Fixed pillar removal animation pulse scale. - Ignored out-of-grid taps. Co-authored-by: candour <4670475+candour@users.noreply.github.com>
Build Successful! 🚀Note: This link will be removed when the PR is closed. |
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
1 participant
Refactored the GameBoard.kt component to utilize a layered rendering approach. This change improves both code maintainability and runtime performance by replacing a monolithic sorting block with specialized layers and a centralized RenderingContext. The World layer (stalls, enemies, pillars) is now sorted strictly by row, ensuring correct depth perception with minimal overhead. Static layers like Background and Decals are rendered without sorting. Foreground elements like UI and projectiles are always rendered on top.
PR created automatically by Jules for task 14384520409405927263 started by @candour
Summary by CodeRabbit
Refactor
Documentation