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

.plans/issue-16-fix-diceroll-bug.md

@@ -0,0 +1,27 @@
+# Plan: Fix DiceRoll Success Counting Bug (Issue #16)
+
+## Summary
+The `DiceRoll.roll()` method computes `numberOfSuccesses` from the stale `this.result` list instead of the freshly rolled values. The fix is to capture the new roll in a local variable before passing it to `copy()`, so both `result` and `numberOfSuccesses` reference the same fresh data.
+
+## Steps
+
+### Step 1: Fix `Dice.kt`
+**File**: `sharedUI/src/commonMain/kotlin/org/shahondin1624/lib/functions/Dice.kt`
+
+Change the `roll()` method to:
+1. Roll dice into a local `val newResult`
+2. Count successes from `newResult`
+3. Pass both to `copy()`
+
+### Step 2: Write unit test
+**File**: `sharedUI/src/commonTest/kotlin/org/shahondin1624/DiceTest.kt`
+
+Create a new test file with:
+- Test that after `roll()`, `numberOfSuccesses` equals the count of values >= threshold in `result`
+- Test with default threshold (5) and custom threshold
+- Test that `result` list has correct size (matches `numberOfDice`)
+- Test the `countSuccesses` extension function directly
+
+## AC Verification Checklist
+1. [ ] `numberOfSuccesses` is computed from newly rolled values (not stale result list)
+2. [ ] Unit test verifying correct success count after `.roll()`

sharedUI/src/commonMain/kotlin/org/shahondin1624/lib/functions/Dice.kt

@@ -15,8 +15,11 @@ data class DiceRoll(
     val result: List<Int> = listOf(),
     val numberOfSuccesses: Int = -1
 ) {
-    fun roll(numberForSuccessOrHigher: Int = 5): DiceRoll = this.copy(
+    fun roll(numberForSuccessOrHigher: Int = 5): DiceRoll {
-        result = rollXDice(sides = this.numberOfSides, numberOfDice = this.numberOfDice),
+        val newResult = rollXDice(sides = this.numberOfSides, numberOfDice = this.numberOfDice)
-        numberOfSuccesses = result.countSuccesses(numberForSuccessOrHigher)
+        return this.copy(
-    )
+            result = newResult,
+            numberOfSuccesses = newResult.countSuccesses(numberForSuccessOrHigher)
+        )
+    }
 }

sharedUI/src/commonTest/kotlin/org/shahondin1624/DiceTest.kt

@@ -0,0 +1,89 @@
+package org.shahondin1624
+
+import org.shahondin1624.lib.functions.DiceRoll
+import org.shahondin1624.lib.functions.countSuccesses
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class DiceTest {
+
+    @Test
+    fun rollProducesCorrectSuccessCount() {
+        val diceRoll = DiceRoll(numberOfDice = 10).roll()
+        val expectedSuccesses = diceRoll.result.count { it >= 5 }
+        assertEquals(
+            expectedSuccesses,
+            diceRoll.numberOfSuccesses,
+            "numberOfSuccesses should match the count of dice >= 5 in result"
+        )
+    }
+
+    @Test
+    fun rollWithCustomThresholdProducesCorrectSuccessCount() {
+        val diceRoll = DiceRoll(numberOfDice = 10).roll(numberForSuccessOrHigher = 4)
+        val expectedSuccesses = diceRoll.result.count { it >= 4 }
+        assertEquals(
+            expectedSuccesses,
+            diceRoll.numberOfSuccesses,
+            "numberOfSuccesses should match the count of dice >= 4 in result"
+        )
+    }
+
+    @Test
+    fun rollResultHasCorrectNumberOfDice() {
+        val numberOfDice = 6
+        val diceRoll = DiceRoll(numberOfDice = numberOfDice).roll()
+        assertEquals(
+            numberOfDice,
+            diceRoll.result.size,
+            "result list size should equal numberOfDice"
+        )
+    }
+
+    @Test
+    fun countSuccessesExtensionFunction() {
+        val rolls = listOf(1, 2, 3, 4, 5, 6)
+        assertEquals(2, rolls.countSuccesses(5), "Only 5 and 6 should be successes with threshold 5")
+        assertEquals(3, rolls.countSuccesses(4), "4, 5, and 6 should be successes with threshold 4")
+        assertEquals(1, rolls.countSuccesses(6), "Only 6 should be a success with threshold 6")
+    }
+
+    @Test
+    fun freshRollDoesNotUseStaleResult() {
+        // Create a DiceRoll with an empty result (default), then roll.
+        // Before the fix, numberOfSuccesses would always be 0 because it counted
+        // successes from the stale empty result list.
+        val initial = DiceRoll(numberOfDice = 20)
+        assertEquals(listOf<Int>(), initial.result, "Initial result should be empty")
+        assertEquals(-1, initial.numberOfSuccesses, "Initial numberOfSuccesses should be -1")
+
+        val rolled = initial.roll()
+        assertEquals(20, rolled.result.size, "Rolled result should have 20 dice")
+
+        val manualCount = rolled.result.count { it >= 5 }
+        assertEquals(
+            manualCount,
+            rolled.numberOfSuccesses,
+            "numberOfSuccesses must reflect the new roll, not the stale empty list"
+        )
+    }
+
+    @Test
+    fun consecutiveRollsProduceIndependentResults() {
+        val diceRoll = DiceRoll(numberOfDice = 10)
+        val firstRoll = diceRoll.roll()
+        val secondRoll = diceRoll.roll()
+
+        // Both rolls should independently have correct success counts
+        assertEquals(
+            firstRoll.result.count { it >= 5 },
+            firstRoll.numberOfSuccesses,
+            "First roll success count should be correct"
+        )
+        assertEquals(
+            secondRoll.result.count { it >= 5 },
+            secondRoll.numberOfSuccesses,
+            "Second roll success count should be correct"
+        )
+    }
+}