fix: compute DiceRoll success count from fresh roll, not stale result (Closes #16) #39

shahondin1624 · 2026-03-13T12:22:52+01:00

shahondin1624 commented

2026-03-13 12:22:52 +01:00

Summary

Fixed bug in DiceRoll.roll(): numberOfSuccesses was computed from the stale this.result (empty list) instead of the freshly rolled values, causing it to always be 0 on first roll
Root cause: Kotlin's copy() evaluates all arguments using this state before creating the new object, so result.countSuccesses() referenced the old empty list
Fix: Capture the new roll in a local variable newResult and use it for both result and numberOfSuccesses in the copy() call

Test plan

6 new unit tests in DiceTest.kt all passing (verified via ./gradlew :sharedUI:jsTest)
rollProducesCorrectSuccessCount -- verifies success count matches actual dice >= 5
rollWithCustomThresholdProducesCorrectSuccessCount -- verifies with threshold 4
rollResultHasCorrectNumberOfDice -- verifies result list size
countSuccessesExtensionFunction -- direct test of the extension function
freshRollDoesNotUseStaleResult -- specifically tests the bug scenario (empty initial result)
consecutiveRollsProduceIndependentResults -- verifies multiple independent rolls

Closes #16

Generated with Claude Code

## Summary - **Fixed bug in `DiceRoll.roll()`**: `numberOfSuccesses` was computed from the stale `this.result` (empty list) instead of the freshly rolled values, causing it to always be 0 on first roll - **Root cause**: Kotlin's `copy()` evaluates all arguments using `this` state before creating the new object, so `result.countSuccesses()` referenced the old empty list - **Fix**: Capture the new roll in a local variable `newResult` and use it for both `result` and `numberOfSuccesses` in the `copy()` call ## Test plan - [x] 6 new unit tests in `DiceTest.kt` all passing (verified via `./gradlew :sharedUI:jsTest`) - [x] `rollProducesCorrectSuccessCount` -- verifies success count matches actual dice >= 5 - [x] `rollWithCustomThresholdProducesCorrectSuccessCount` -- verifies with threshold 4 - [x] `rollResultHasCorrectNumberOfDice` -- verifies result list size - [x] `countSuccessesExtensionFunction` -- direct test of the extension function - [x] `freshRollDoesNotUseStaleResult` -- specifically tests the bug scenario (empty initial result) - [x] `consecutiveRollsProduceIndependentResults` -- verifies multiple independent rolls Closes #16 Generated with [Claude Code](https://claude.ai/code)

shahondin1624 added 1 commit 2026-03-13 12:22:52 +01:00

fix: compute DiceRoll success count from fresh roll, not stale result (Closes #16 ) 1b75bc0b69

The DiceRoll.roll() method was counting successes from `this.result`
(the stale/empty list from before rolling) instead of the newly rolled
values. Fixed by capturing the new roll in a local variable before
passing it to copy(). Added 6 unit tests covering the fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

shahondin1624 merged commit d08cc83348 into main

2026-03-13 12:22:57 +01:00

shahondin1624 deleted branch feature/issue-16-fix-diceroll-success-counting-bug

2026-03-13 12:22:58 +01:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: shahondin1624/ShadowrunCharSheet#39