Skip to content

Fix Tiger Mom buff cleanup logic#173

Merged
candour merged 2 commits into
mainfrom
fix-tiger-mom-buff-cleanup-15074498557543227381
May 30, 2026
Merged

Fix Tiger Mom buff cleanup logic#173
candour merged 2 commits into
mainfrom
fix-tiger-mom-buff-cleanup-15074498557543227381

Conversation

@candour
Copy link
Copy Markdown
Owner

@candour candour commented May 28, 2026

This PR fixes a bug where Tiger Mom buffs could persist indefinitely on enemies if the source Tiger Mom was grabbed by a Tray Return Uncle or if no projectiles hit any enemies in subsequent ticks.

I centralized the buff cleanup logic into a new cleanupEnemyBuffs method in MainViewModel.kt. This method ensures that:

  1. ARMOR buffs are removed if their source is dead, no longer targeting the enemy, or is currently grabbed (e.g., by a Tray Return Uncle).
  2. The Tiger Mom's own state (buffingTargetId, isStopped) is cleaned up if her target is gone or if she herself is grabbed.

The cleanup is now consistently performed at the end of the enemy processing pipeline and after projectile impacts, ensuring game state integrity even when combat is not active.

A new unit test TigerMomBuffCleanupTest was added to reproduce the issue and verify the fix.


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

Summary by CodeRabbit

  • Bug Fixes

    • Armor buffs granted by Tiger Mom are now reliably removed when Tiger Mom is grabbed, preventing lingering buffs on targets.
  • Tests

    • Added automated test verifying Tiger Mom’s armor buffs are cleared when she is grabbed (even with no projectiles).
  • Documentation

    • Added a bug-fix entry describing the Tiger Mom buff cleanup behavior.

Review Change Stack

- Created `cleanupEnemyBuffs` helper in `MainViewModel.kt` to ensure ARMOR buffs are removed if the source Tiger Mom is dead, no longer targeting, or grabbed.
- Applied cleanup at the end of both `handleEnemyPipeline` and `handleProjectiles`.
- Added `TigerMomBuffCleanupTest.kt` to verify fix.
- Updated `fixes.md` with FIX-016.

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
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 54f48a7e-0d79-4315-a140-7bc9bed9baa9

📥 Commits

Reviewing files that changed from the base of the PR and between 0e92bad and eb319bd.

📒 Files selected for processing (1)
  • app/src/main/java/com/messark/hawker/MainViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/src/main/java/com/messark/hawker/MainViewModel.kt

📝 Walkthrough

Walkthrough

Centralizes Tiger Mom ARMOR buff lifecycle into a new cleanupEnemyBuffs helper; handleEnemyPipeline now accumulates buff activations as buffActions, applies them, and returns the cleaned enemy list; projectile processing also calls the centralized cleanup. Adds a unit test and a FIX-016 entry.

Changes

Tiger Mom Buff Cleanup Centralization

Layer / File(s) Summary
Centralized buff cleanup function
app/src/main/java/com/messark/hawker/MainViewModel.kt
New cleanupEnemyBuffs helper validates ARMOR buffs against the current enemy list, removes buffs from missing/non-targeting/grabbed Tiger Moms, clears buffingTargetId when appropriate, and resets Tiger Mom stop fields when cleared.
Enemy pipeline and projectile handler integration
app/src/main/java/com/messark/hawker/MainViewModel.kt
handleEnemyPipeline accumulates newly activated Tiger Mom buffs in buffActions, applies them during the tick, removes prior ad-hoc special-case clearing, and performs final cleanup via cleanupEnemyBuffs; handleProjectiles uses cleanupEnemyBuffs instead of manual buff filtering.
Test validation and fix documentation
app/src/test/java/com/messark/hawker/TigerMomBuffCleanupTest.kt, fixes.md
New test asserts Tiger Mom's ARMOR buff is removed when she is grabbed even if no projectiles hit; fixes.md includes FIX-016 describing the change.

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% 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 clearly and specifically describes the main change: fixing Tiger Mom buff cleanup logic, which aligns with the primary objective of centralizing buff cleanup into a new cleanupEnemyBuffs method.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-tiger-mom-buff-cleanup-15074498557543227381

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
Copy link
Copy Markdown

Build Successful! 🚀

Download APK

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
app/src/test/java/com/messark/hawker/TigerMomBuffCleanupTest.kt (1)

95-102: ⚡ Quick win

Assert Tiger Mom state reset, not only target buff removal.

This test should also verify the source Tiger Mom clears buffingTargetId and unstops, matching the cleanup contract.

Suggested assertion additions
         val newState = viewModel.gameState.value
         val updatedTarget = newState.enemies.find { it.id == targetId }
+        val updatedMom = newState.enemies.find { it.id == momId }
         assertNotNull("Target enemy should still exist", updatedTarget)
+        assertNotNull("Tiger Mom should still exist", updatedMom)

         // This is expected to FAIL before the fix
         assertFalse("Buff should be removed when source is grabbed",
             updatedTarget!!.buffs.any { it.sourceId == momId && it.type == BuffType.ARMOR })
+        assertFalse("Tiger Mom should no longer be stopped while grabbed cleanup runs", updatedMom!!.isStopped)
+        org.junit.Assert.assertNull("Tiger Mom buff target should be cleared", updatedMom.buffingTargetId)

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1daa8c44-6818-4785-8cc5-66c24c0aacf4

📥 Commits

Reviewing files that changed from the base of the PR and between 6ae33d9 and 0e92bad.

📒 Files selected for processing (3)
  • app/src/main/java/com/messark/hawker/MainViewModel.kt
  • app/src/test/java/com/messark/hawker/TigerMomBuffCleanupTest.kt
  • fixes.md

Comment thread app/src/main/java/com/messark/hawker/MainViewModel.kt
Comment thread fixes.md
@candour candour enabled auto-merge (squash) May 30, 2026 12:03
@candour
Copy link
Copy Markdown
Owner Author

candour commented May 30, 2026

Handle dead targets explicitly when clearing buffingTargetId.**
shouldClear currently checks only missing target or grabbed source. If a target remains in-list but is flagged dead, Tiger Mom state can stay stale longer than intended.

@google-labs-jules
Copy link
Copy Markdown
Contributor

Handle dead targets explicitly when clearing buffingTargetId.**
shouldClear currently checks only missing target or grabbed source. If a target remains in-list but is flagged dead, Tiger Mom state can stay stale longer than intended.

Good catch. I've updated cleanupEnemyBuffs to explicitly check target.isDead in the shouldClear condition. This ensures the Tiger Mom's state is reset immediately if her target is defeated, even if the target is still present in the list before being pruned.

- Updated `cleanupEnemyBuffs` to explicitly clear `buffingTargetId` if the target is marked dead.
- This ensures Tiger Mom state is reset immediately upon target defeat.

Co-authored-by: candour <4670475+candour@users.noreply.github.com>
@candour candour merged commit 5fd06fa into main May 30, 2026
3 checks passed
@candour candour deleted the fix-tiger-mom-buff-cleanup-15074498557543227381 branch May 30, 2026 12:12
@github-actions
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