Refactor Combat Logic for Efficiency

[candour]

I have refactored the core combat logic in MainViewModel.kt to improve efficiency and maintainability.

Key improvements:

1. Performance: Introduced an enemyLookup map in handleProjectiles, changing target enemy lookups from $O(N)$ to $O(1)$. This is particularly beneficial when many projectiles are in flight.
2. Efficiency: Optimized handleStallFiring by pre-filtering for stalls that are actually ready to fire, reducing iterations over the entire hex grid.
3. Readability: Simplified complex logic branches and consolidated state updates to make the code more idiomatic and easier to follow.
4. Bug Fix: Corrected the enemy death threshold to ensure enemies with fractional health that rounds to zero (HP < 1.0) are correctly removed from the board, preventing "ghost" enemies.

Verified all changes with existing and updated unit tests. The project compiles successfully.

---

PR created automatically by Jules for task 10249510848776498304 started by @candour

Summary by CodeRabbit

• Refactor

  • Streamlined combat processing: faster stall firing and projectile resolution, optimized area-of-effect detection, consolidated state updates, and rate-limited haptic feedback for smoother gameplay.

• Chores

  • Added an entry to the bug/improvement log documenting the internal refactor and performance optimizations.

[google-labs-jules]

👋 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]

Warning

Rate limit exceeded

@candour has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 27 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: f9700d6b-e5c8-429f-97ff-3f54d17b1755

📥 Commits

Reviewing files that changed from the base of the PR and between 221e207 and 08d5801.

📒 Files selected for processing (1)

• app/src/main/java/com/messark/hawker/MainViewModel.kt

📝 Walkthrough

Walkthrough

MainViewModel's combat tick pipeline was refactored for performance: handleStallFiring now precomputes firing stalls and restructures the firing loop, while handleProjectiles uses a precomputed enemy lookup map, reorganizes hit detection into direct then AoE phases, consolidates impact processing, and centralizes haptic feedback emission with rate limiting.

Changes

Combat Tick Optimization

Layer / File(s)	Summary
Stall Firing Refactor app/src/main/java/com/messark/hawker/MainViewModel.kt	handleStallFiring refactored to precompute firingStalls, early-return when empty/no enemies, and restructure the per-stall firing loop while preserving target-mode selection, adjacency stat tracking, TRAY_RETURN_UNCLE grab/release updates, and temporary stat boost/restore behavior.
Enemy Lookup Infrastructure app/src/main/java/com/messark/hawker/MainViewModel.kt	Projectile-target resolution now uses a precomputed enemyLookup map indexed by enemy id instead of per-projectile searches through state.enemies.
Hit Detection & Collection app/src/main/java/com/messark/hawker/MainViewModel.kt	Hit collection reorganized into two phases: direct-target hits collected first (when target exists & not grabbed), then AoE hits collected by scanning enemies within aoeRadius, excluding grabbed enemies and avoiding direct-target double-counts.
Damage Aggregation & State Updates app/src/main/java/com/messark/hawker/MainViewModel.kt	Impact processing aggregates damage per enemy across projectiles, applies ARMOR buffs and source-stall modifiers, updates stall kill tracking in updatedHexes, computes gold/score and death vs. survival state, and prunes invalid buff targets; early-returns if no hits recorded.
Haptic Emission Consolidation app/src/main/java/com/messark/hawker/MainViewModel.kt	Haptic triggering centralized: set a single shouldTriggerHaptic flag and emit at most once per tick, rate-limited via lastHapticTimeMs.
Documentation fixes.md	Changelog entry (REF-003) documents the performance-focused refactors to stall firing and projectile handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• candour/towerpower#82: Also modifies MainViewModel stall firing/projectile logic including uniqueTargetIds and TRAY_RETURN_UNCLE held logic overlapping these changes.
• candour/towerpower#128: Changed calculateStatBoost signature/return details that are invoked by the refactored handleStallFiring in this PR.
• candour/towerpower#126: Adjusted calculateStatBoost to accept hex maps; this PR calls calculateStatBoost with state.hexes in the updated firing logic.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Refactor Combat Logic for Efficiency' is directly aligned with the main changes described in the PR: core refactoring of combat logic in MainViewModel.kt to improve performance and maintainability through optimizations like enemyLookup maps and pre-filtering of stalls.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 refactor-combat-logic-10249510848776498304

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Build Successful! 🚀

Download APK

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

app/src/main/java/com/messark/hawker/MainViewModel.kt (2)

625-639: 💤 Low value

Duplicate KDoc above handleStallFiring.

The new KDoc (lines 632-639) is stacked on top of the existing one (lines 625-631) for the same function. Kotlin will only treat the closest block as the function’s doc, but both will render in source/IDE tooltips depending on tooling. Drop the older block (or merge them) to avoid drift.

♻️ Suggested cleanup

-    /**
-     * Checks all stalls to see if they are ready to fire and creates projectiles/puddles.
-     *
-     * `@param` state Current game state.
-     * `@param` currentTimeMs Current game time.
-     * `@return` Updated state with new projectiles/puddles.
-     */
     /**
      * Handles the firing logic for all stalls on the board.
      * Efficiently filters for active stalls and determines targets based on their targeting mode.
      *
      * `@param` state Current game state.
      * `@param` currentTimeMs Current game time in milliseconds.
      * `@return` Updated game state with new projectiles, puddles, and stall states.
      */
     private fun handleStallFiring(state: GameState, currentTimeMs: Long): GameState {

---

857-866: ⚡ Quick win

Reuse enemyLookup for buff source resolution.

The post-damage buff cleanup still does a linear state.enemies.find per buff per surviving enemy (O(buffs · enemies) per tick), which goes against the O(1) lookup goal of this PR. The already-built enemyLookup map (line 740) gives the same result with a single map probe.

♻️ Proposed refactor

         }.map { e ->
             // Clean up buffs: source must be alive, still targeting this enemy, and not grabbed
             val validBuffs = e.buffs.filter { buff ->
-                val source = state.enemies.find { it.id == buff.sourceId }
+                val source = enemyLookup[buff.sourceId]
                 source != null && source.buffingTargetId == e.id && !source.isGrabbed
             }
             if (validBuffs.size != e.buffs.size) {
                 e.copy(buffs = validBuffs)
             } else e
         }

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5110c5a0-7239-4bc8-bc10-9d7477334a9b

📥 Commits

Reviewing files that changed from the base of the PR and between cc4a70e and 449f9eb.

📒 Files selected for processing (2)

• app/src/main/java/com/messark/hawker/MainViewModel.kt
• fixes.md

[candour]

Stall kill-credit threshold doesn’t match the new death threshold.

The death/reward check at line 845 was updated to currentHealth < 1.0f (REF-003), but the per-projectile kill attribution at line 832 still uses currentHealth <= 0. When an enemy dies with fractional residual HP in (0, 1) — easily reachable via ARMOR buffs (line 814) or stallDef.applyDamageModifiers producing non-integer damage — the player is awarded gold/score and the enemy is removed, but the firing stall’s kills counter and uniqueTargetIds are never incremented. Stall stats will silently undercount kills for any non-integer damage path.

Use the same threshold as the post-loop death check (or factor it into a shared isDead predicate so the two cannot drift again).

[candour]

Duplicate KDoc above handleStallFiring.

The new KDoc (lines 632-639) is stacked on top of the existing one (lines 625-631) for the same function. Kotlin will only treat the closest block as the function’s doc, but both will render in source/IDE tooltips depending on tooling. Drop the older block (or merge them) to avoid drift.

Reuse enemyLookup for buff source resolution.

The post-damage buff cleanup still does a linear state.enemies.find per buff per surviving enemy (O(buffs · enemies) per tick), which goes against the O(1) lookup goal of this PR. The already-built enemyLookup map (line 740) gives the same result with a single map probe.

[google-labs-jules]

Stall kill-credit threshold doesn’t match the new death threshold.

The death/reward check at line 845 was updated to currentHealth < 1.0f (REF-003), but the per-projectile kill attribution at line 832 still uses currentHealth <= 0. When an enemy dies with fractional residual HP in (0, 1) — easily reachable via ARMOR buffs (line 814) or stallDef.applyDamageModifiers producing non-integer damage — the player is awarded gold/score and the enemy is removed, but the firing stall’s kills counter and uniqueTargetIds are never incremented. Stall stats will silently undercount kills for any non-integer damage path.

Use the same threshold as the post-loop death check (or factor it into a shared isDead predicate so the two cannot drift again).

Good catch! I've updated the kill-credit threshold in handleProjectiles to currentHealth < 1.0f to perfectly align with the death threshold. This ensures stalls correctly receive credit for kills involving fractional damage. Thanks for pointing that out!

[github-actions]

Build Successful! 🚀

Download APK

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

[candour]

Duplicate KDoc above handleStallFiring.

The new KDoc (lines 632-639) is stacked on top of the existing one (lines 625-631) for the same function. Kotlin will only treat the closest block as the function’s doc, but both will render in source/IDE tooltips depending on tooling. Drop the older block (or merge them) to avoid drift.

Reuse enemyLookup for buff source resolution.

The post-damage buff cleanup still does a linear state.enemies.find per buff per surviving enemy (O(buffs · enemies) per tick), which goes against the O(1) lookup goal of this PR. The already-built enemyLookup map (line 740) gives the same result with a single map probe.

[google-labs-jules]

Duplicate KDoc above handleStallFiring.

The new KDoc (lines 632-639) is stacked on top of the existing one (lines 625-631) for the same function. Kotlin will only treat the closest block as the function’s doc, but both will render in source/IDE tooltips depending on tooling. Drop the older block (or merge them) to avoid drift.

Reuse enemyLookup for buff source resolution.

The post-damage buff cleanup still does a linear state.enemies.find per buff per surviving enemy (O(buffs · enemies) per tick), which goes against the O(1) lookup goal of this PR. The already-built enemyLookup map (line 740) gives the same result with a single map probe.

Fixed! I've removed the duplicate KDoc block above handleStallFiring. I've also updated the buff cleanup logic in handleProjectiles to use the enemyLookup map, ensuring we maintain $O(1)$ lookups and avoid the $O(N)$ linear search per buff. Thanks for the thorough review!

[github-actions]

Build Successful! 🚀

Download APK

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