Skip to content

Implement Line-of-Sight Blocking by Pillars - #135

Merged
candour merged 2 commits into
mainfrom
feature/los-blocking-pillars-15775259929042520796
May 9, 2026
Merged

Implement Line-of-Sight Blocking by Pillars#135
candour merged 2 commits into
mainfrom
feature/los-blocking-pillars-15775259929042520796

Conversation

@candour

@candour candour commented May 9, 2026

Copy link
Copy Markdown
Owner

This change introduces a line-of-sight (LOS) system where pillars (and future obstructions) block the targeting capability of hawker stalls.

Key technical highlights:

  • Refactored Tile System: TileType is now a sealed class, allowing for a clean hierarchy where PILLAR inherits from a new Obstruction class.
  • Geometric LOS Logic: Implemented a circle-segment intersection algorithm in MainViewModel that calculates whether a pillar's obstruction zone (defined as 0.25 units radius) blocks the path between a stall and an enemy.
  • Visual Feedback: When a user selects a stall that is affected by LOS (most stalls by default), the game board now renders a "shadow" corridor and boundary lines showing exactly which areas are unreachable due to obstructions.
  • Persistence Safety: A custom GSON TypeAdapter ensures 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

    • Added line-of-sight blocking mechanics: blockable stalls now respect obstacles when selecting targets.
    • Implemented visual indicators for blocked areas on the game board, showing obstruction zones.
    • Updated enemy pathing to account for obstacles and board edges during movement.
  • Improvements

    • Enhanced board tile-blocking logic for more consistent obstacle detection.

Review Change Stack

- 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>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@candour has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 44 minutes and 33 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: abc06209-4a67-4311-b703-c52a5f1c620f

📥 Commits

Reviewing files that changed from the base of the PR and between 0d26dfb and b4f0f11.

📒 Files selected for processing (1)
  • app/src/main/java/com/messark/hawker/ui/components/GameBoard.kt
📝 Walkthrough

Walkthrough

This PR introduces line-of-sight obstruction blocking for stalls: the data model refactors TileType to a sealed class hierarchy, adds an isBlockable property to stalls, implements targeting visibility checks via circle-segment intersection, updates path-finding logic for obstructions and edge tiles, visualizes blocked regions on the board, and adds JSON persistence for the refactored type system.

Changes

Stall Line-of-Sight Feature

Layer / File(s) Summary
Data Model
app/src/main/java/com/messark/hawker/model/GameModels.kt
TileType refactored from enum class to sealed class with singleton object instances (FLOOR, EDGE_*, PILLAR, GOAL_TABLE, START, END) and Obstruction sealed subtype; adds name-based toString() and companion object lookup helpers. Stall gains isBlockable: Boolean property (default true).
Registry Mapping
app/src/main/java/com/messark/hawker/registry/Registry.kt
StallDefinition adds isBlockable: Boolean property (default true); toStall() conversion passes isBlockable through to constructed Stall instance.
Core Game Logic
app/src/main/java/com/messark/hawker/MainViewModel.kt
Stall target selection applies isLineOfSightBlocked(...) filter for blockable stalls (backed by lineIntersectsCircle(...)); getBlockedCoordinates(...) treats obstructions, edge tiles, goal table, and occupied stalls as blocked; releaseEnemy(...) adjacency filter excludes TileType.Obstruction and edge tiles.
UI Visualization
app/src/main/java/com/messark/hawker/ui/components/GameBoard.kt
Renders LOS-blocked region for blockable stalls: computes angular boundaries from obstruction tiles, converts to screen coordinates via GridUtils.toScreenPrecise, draws filled dark polygon and red boundary rays.
Persistence
app/src/main/java/com/messark/hawker/utils/GameStateRepository.kt
TileTypeAdapter added for Gson serialization/deserialization of sealed-class TileType using name-based conversion; registered in GsonBuilder.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • candour/towerpower#88: Both PRs modify MainViewModel.releaseEnemy() adjacency logic to change which tile types are treated as non-walkable neighbors.
  • candour/towerpower#117: Both PRs modify stall placement/release logic in MainViewModel, specifically how START/GOAL_TABLE tiles are treated in releaseEnemy().
  • candour/towerpower#67: Both PRs modify Stall and StallDefinition types with new properties and update registry/board logic that depends on those types.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Implement Line-of-Sight Blocking by Pillars' accurately captures the main objective of the PR: introducing a line-of-sight system where pillars block stall targeting.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/los-blocking-pillars-15775259929042520796

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

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

@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

Build Successful! 🚀

Download APK

Note: This link will be removed when the PR is closed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Normalize Stall.isBlockable when loading older saves.

Line 66 restores nested Stall objects through Gson deserialization. Saves written before the isBlockable field was added omit it from JSON. Gson deserializes missing boolean fields to the JVM default false, not the Kotlin default true, causing stalls to lose line-of-sight blocking behavior on resume without any warning.

Normalize isBlockable at load time using StallRegistry.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 win

Use 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

📥 Commits

Reviewing files that changed from the base of the PR and between 8ada4ac and 0d26dfb.

📒 Files selected for processing (5)
  • app/src/main/java/com/messark/hawker/MainViewModel.kt
  • app/src/main/java/com/messark/hawker/model/GameModels.kt
  • app/src/main/java/com/messark/hawker/registry/Registry.kt
  • app/src/main/java/com/messark/hawker/ui/components/GameBoard.kt
  • app/src/main/java/com/messark/hawker/utils/GameStateRepository.kt

Comment thread app/src/main/java/com/messark/hawker/ui/components/GameBoard.kt Outdated
@candour
candour enabled auto-merge (squash) May 9, 2026 14:33
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@candour
candour merged commit ceef600 into main May 9, 2026
3 checks passed
@candour
candour deleted the feature/los-blocking-pillars-15775259929042520796 branch May 9, 2026 14:34
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

Build Successful! 🚀

Download APK

Note: This link will be removed when the PR is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant