Implement Line-of-Sight Blocking by Pillars - #135
Conversation
- Refactored `TileType` to a sealed class with an `Obstruction` subclass for extensibility. - Added `isBlockable` property to `Stall` and `StallDefinition` to control LOS behavior. - Implemented geometric line-of-sight checking in `MainViewModel` to prevent stalls from targeting enemies behind obstructions. - Added visual indicators in `GameBoard.kt` to show blocked range areas when a stall is selected. - Updated `GameStateRepository` with a GSON adapter for `TileType` to maintain save state compatibility. 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 (1)
📝 WalkthroughWalkthroughThis PR introduces line-of-sight obstruction blocking for stalls: the data model refactors ChangesStall Line-of-Sight Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/messark/hawker/utils/GameStateRepository.kt (1)
66-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize
Stall.isBlockablewhen loading older saves.Line 66 restores nested
Stallobjects through Gson deserialization. Saves written before theisBlockablefield was added omit it from JSON. Gson deserializes missing boolean fields to the JVM defaultfalse, not the Kotlin defaulttrue, causing stalls to lose line-of-sight blocking behavior on resume without any warning.Normalize
isBlockableat load time usingStallRegistry.get(stallType).isBlockable:Suggested fix
+import com.messark.hawker.registry.StallRegistry ... fun loadGameState(): GameState? { if (!file.exists()) return null return try { val persistentState = gson.fromJson(file.readText(), PersistentGameState::class.java) + val restoredHexes = persistentState.hexes + .map { hex -> + hex.copy( + stall = hex.stall?.let { stall -> + stall.copy( + isBlockable = StallRegistry.get(stall.stallType).isBlockable + ) + } + ) + } + .associateBy { it.coordinate } + GameState( currentScreen = AppScreen.GAME, health = persistentState.health, gold = persistentState.gold, - hexes = persistentState.hexes.associateBy { it.coordinate }, + hexes = restoredHexes, startPosition = persistentState.startPosition, endPosition = persistentState.endPosition, currentWave = persistentState.currentWave, score = persistentState.score, kitchelinStars = persistentState.kitchelinStars,
🧹 Nitpick comments (1)
app/src/main/java/com/messark/hawker/ui/components/GameBoard.kt (1)
415-430: ⚡ Quick winUse theme colors for the LOS overlay.
The new shadow and boundary colors are hardcoded, so this overlay will not adapt with the rest of the board in light/dark themes.
Suggested fix
+import androidx.compose.material3.MaterialTheme ... + val losShadowColor = MaterialTheme.colorScheme.scrim.copy(alpha = 0.2f) + val losBoundaryColor = MaterialTheme.colorScheme.error.copy(alpha = 0.5f) ... - drawPath( - path = shadowPath, - color = Color.Black.copy(alpha = 0.2f) - ) + drawPath( + path = shadowPath, + color = losShadowColor + ) drawLine( - color = Color.Red.copy(alpha = 0.5f), + color = losBoundaryColor, start = p1Start, end = p1End, strokeWidth = 2.dp.toPx() ) drawLine( - color = Color.Red.copy(alpha = 0.5f), + color = losBoundaryColor, start = p2Start, end = p2End, strokeWidth = 2.dp.toPx() )As per coding guidelines, Avoid hardcoding colors in UI components; always prefer
MaterialTheme.colorScheme(e.g.,onSurface,surface,primary) to support light and dark themes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 09ea8d82-99fd-4b9b-b3c2-ab63bbe10d7f
📒 Files selected for processing (5)
app/src/main/java/com/messark/hawker/MainViewModel.ktapp/src/main/java/com/messark/hawker/model/GameModels.ktapp/src/main/java/com/messark/hawker/registry/Registry.ktapp/src/main/java/com/messark/hawker/ui/components/GameBoard.ktapp/src/main/java/com/messark/hawker/utils/GameStateRepository.kt
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Build Successful! 🚀Note: This link will be removed when the PR is closed. |
This change introduces a line-of-sight (LOS) system where pillars (and future obstructions) block the targeting capability of hawker stalls.
Key technical highlights:
TileTypeis now a sealed class, allowing for a clean hierarchy wherePILLARinherits from a newObstructionclass.MainViewModelthat calculates whether a pillar's obstruction zone (defined as 0.25 units radius) blocks the path between a stall and an enemy.TypeAdapterensures that the new sealed class structure is serialized correctly and remains backward-compatible with existing save files.PR created automatically by Jules for task 15775259929042520796 started by @candour
Summary by CodeRabbit
New Features
Improvements