Refactor tax counters into cast counts with separate ID space and user activated create/delete

tests/command_zone_tests/counter_command_auth_test.cpp

@@ -18,156 +18,35 @@ namespace
 {
 constexpr int UserCounterId = CounterIds::FirstUserId;
 
-Server_Counter makeCounter(int id, int count, bool active = true)
+Server_Counter makeCounter(int id, int count)
 {
-    Server_Counter c(id, "c", color(), 20, count);
-    (void)c.setActive(active);
-    return c;
+    return Server_Counter(id, "c", color(), 20, count);
 }
 } // namespace
 
 TEST(EvaluateDelCounter, RejectsWhenGameNotStarted)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
-    EXPECT_EQ(
-        Server_Player::evaluateDelCounter(/*gameStarted=*/false, /*playerConceded=*/false, UserCounterId, &counter),
-        Response::RespGameNotStarted);
+    EXPECT_EQ(Server_Player::evaluateDelCounter(/*gameStarted=*/false, /*playerConceded=*/false, &counter),
+              Response::RespGameNotStarted);
 }
 
 TEST(EvaluateDelCounter, RejectsWhenPlayerConceded)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
-    EXPECT_EQ(Server_Player::evaluateDelCounter(/*gameStarted=*/true, /*playerConceded=*/true, UserCounterId, &counter),
+    EXPECT_EQ(Server_Player::evaluateDelCounter(/*gameStarted=*/true, /*playerConceded=*/true, &counter),
               Response::RespContextError);
 }
 
-TEST(EvaluateDelCounter, RejectsTaxCounters)
-{
-    Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0);
-    EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, CounterIds::TaxCounter1, &tax1),
-              Response::RespFunctionNotAllowed);
-
-    Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0);
-    EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, CounterIds::TaxCounter2, &tax2),
-              Response::RespFunctionNotAllowed);
-}
-
 TEST(EvaluateDelCounter, RejectsMissingCounter)
 {
-    EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, UserCounterId, nullptr), Response::RespNameNotFound);
+    EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, nullptr), Response::RespNameNotFound);
 }
 
 TEST(EvaluateDelCounter, AllowsDeletingUserCounter)
 {
     Server_Counter counter = makeCounter(UserCounterId, 7);
-    EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, UserCounterId, &counter), Response::RespOk);
-}
-
-TEST(EvaluateDelCounter, GameNotStartedTakesPrecedenceOverTaxGuard)
-{
-    Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0);
-    EXPECT_EQ(Server_Player::evaluateDelCounter(false, false, CounterIds::TaxCounter1, &tax1),
-              Response::RespGameNotStarted);
-}
-
-TEST(EvaluateSetCounterActive, RejectsWhenGameNotStarted)
-{
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(/*gameStarted=*/false, /*playerConceded=*/false,
-                                                      /*commandZoneEnabled=*/true, CounterIds::TaxCounter1, &counter,
-                                                      /*requestedActive=*/true, nullptr, nullptr),
-              Response::RespGameNotStarted);
-}
-
-TEST(EvaluateSetCounterActive, RejectsWhenPlayerConceded)
-{
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, /*playerConceded=*/true, true, CounterIds::TaxCounter1,
-                                                      &counter, true, nullptr, nullptr),
-              Response::RespContextError);
-}
-
-TEST(EvaluateSetCounterActive, RejectsNonTaxCounter)
-{
-    Server_Counter counter = makeCounter(UserCounterId, 0);
-    EXPECT_EQ(
-        Server_Player::evaluateSetCounterActive(true, false, true, UserCounterId, &counter, true, nullptr, nullptr),
-        Response::RespFunctionNotAllowed);
-}
-
-TEST(EvaluateSetCounterActive, RejectsWhenCommandZoneDisabled)
-{
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, /*commandZoneEnabled=*/false,
-                                                      CounterIds::TaxCounter1, &counter, true, nullptr, nullptr),
-              Response::RespContextError);
-}
-
-TEST(EvaluateSetCounterActive, RejectsMissingCounter)
-{
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, nullptr, true,
-                                                      nullptr, nullptr),
-              Response::RespNameNotFound);
-}
-
-TEST(EvaluateSetCounterActive, RejectsDisablingWhenTaxAccumulated)
-{
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 3);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &counter,
-                                                      /*requestedActive=*/false, nullptr, nullptr),
-              Response::RespContextError);
-}
-
-TEST(EvaluateSetCounterActive, AllowsEnablingWithAccumulatedTax)
-{
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 3);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &counter,
-                                                      /*requestedActive=*/true, nullptr, nullptr),
-              Response::RespOk);
-}
-
-TEST(EvaluateSetCounterActive, AllowsDisablingWhenCounterIsZero)
-{
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &counter,
-                                                      /*requestedActive=*/false, nullptr, nullptr),
-              Response::RespOk);
-}
-
-TEST(EvaluateSetCounterActive, AllowsEnabling2ndTaxWhen1stIsActive)
-{
-    Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0, true);
-    Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0, false);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter2, &tax2,
-                                                      /*requestedActive=*/true, &tax1, nullptr),
-              Response::RespOk);
-}
-
-TEST(EvaluateSetCounterActive, RejectsEnabling2ndTaxWhen1stIsInactive)
-{
-    Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0, false);
-    Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0, false);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter2, &tax2,
-                                                      /*requestedActive=*/true, &tax1, nullptr),
-              Response::RespContextError);
-}
-
-TEST(EvaluateSetCounterActive, AllowsDisabling1stTaxWhen2ndIsInactive)
-{
-    Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0, true);
-    Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0, false);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &tax1,
-                                                      /*requestedActive=*/false, nullptr, &tax2),
-              Response::RespOk);
-}
-
-TEST(EvaluateSetCounterActive, RejectsDisabling1stTaxWhen2ndIsActive)
-{
-    Server_Counter tax1 = makeCounter(CounterIds::TaxCounter1, 0, true);
-    Server_Counter tax2 = makeCounter(CounterIds::TaxCounter2, 0, true);
-    EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::TaxCounter1, &tax1,
-                                                      /*requestedActive=*/false, nullptr, &tax2),
-              Response::RespContextError);
+    EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, &counter), Response::RespOk);
 }
 
 TEST(EvaluateCreateCounter, RejectsWhenGameNotStarted)
@@ -181,20 +60,6 @@ TEST(EvaluateCreateCounter, RejectsWhenPlayerConceded)
     EXPECT_EQ(Server_Player::evaluateCreateCounter(true, /*playerConceded=*/true, "test"), Response::RespContextError);
 }
 
-TEST(EvaluateCreateCounter, RejectsTaxCounterNames)
-{
-    EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter1),
-              Response::RespFunctionNotAllowed);
-    EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter2),
-              Response::RespFunctionNotAllowed);
-    EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter3),
-              Response::RespFunctionNotAllowed);
-    EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter4),
-              Response::RespFunctionNotAllowed);
-    EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::TaxCounter5),
-              Response::RespFunctionNotAllowed);
-}
-
 TEST(EvaluateCreateCounter, AllowsUserCounterName)
 {
     EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, "poison"), Response::RespOk);
@@ -203,61 +68,151 @@ TEST(EvaluateCreateCounter, AllowsUserCounterName)
 TEST(EvaluateModifyCounter, RejectsWhenGameNotStarted)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
-    EXPECT_EQ(Server_Player::evaluateModifyCounter(/*gameStarted=*/false, /*playerConceded=*/false, true, UserCounterId,
-                                                   &counter),
+    EXPECT_EQ(Server_Player::evaluateModifyCounter(/*gameStarted=*/false, /*playerConceded=*/false, &counter),
               Response::RespGameNotStarted);
 }
 
 TEST(EvaluateModifyCounter, RejectsWhenPlayerConceded)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
-    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, /*playerConceded=*/true, true, UserCounterId, &counter),
+    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, /*playerConceded=*/true, &counter),
               Response::RespContextError);
 }
 
 TEST(EvaluateModifyCounter, RejectsMissingCounter)
 {
-    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, true, UserCounterId, nullptr),
-              Response::RespNameNotFound);
+    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, nullptr), Response::RespNameNotFound);
 }
 
 TEST(EvaluateModifyCounter, AllowsUserCounter)
 {
     Server_Counter counter = makeCounter(UserCounterId, 5);
-    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, true, UserCounterId, &counter), Response::RespOk);
+    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, &counter), Response::RespOk);
 }
 
-TEST(EvaluateModifyCounter, RejectsTaxCounterWhenCommandZoneDisabled)
+// Cast count evaluation tests
+
+TEST(EvaluateCreateCastCount, RejectsWhenGameNotStarted)
 {
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0);
-    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/false, CounterIds::TaxCounter1,
-                                                   &counter),
+    EXPECT_EQ(Server_Player::evaluateCreateCastCount(/*gameStarted=*/false, /*playerConceded=*/false,
+                                                     /*commandZoneEnabled=*/true, 1, /*exists=*/false,
+                                                     /*predecessorExists=*/true),
+              Response::RespGameNotStarted);
+}
+
+TEST(EvaluateCreateCastCount, RejectsWhenPlayerConceded)
+{
+    EXPECT_EQ(Server_Player::evaluateCreateCastCount(true, /*playerConceded=*/true, true, 1, false, true),
               Response::RespContextError);
 }
 
-TEST(EvaluateModifyCounter, RejectsInactiveTaxCounter)
+TEST(EvaluateCreateCastCount, RejectsWhenCommandZoneDisabled)
 {
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter2, 0, false);
-    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::TaxCounter2,
-                                                   &counter),
+    EXPECT_EQ(Server_Player::evaluateCreateCastCount(true, false, /*commandZoneEnabled=*/false, 1, false, true),
               Response::RespContextError);
 }
 
-TEST(EvaluateModifyCounter, AllowsActiveTaxCounter)
+TEST(EvaluateCreateCastCount, RejectsInvalidIndex)
 {
-    Server_Counter counter = makeCounter(CounterIds::TaxCounter1, 0, true);
-    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::TaxCounter1,
-                                                   &counter),
+    EXPECT_EQ(Server_Player::evaluateCreateCastCount(true, false, true, 0, false, false), Response::RespContextError);
+    EXPECT_EQ(Server_Player::evaluateCreateCastCount(true, false, true, 6, false, false), Response::RespContextError);
+}
+
+TEST(EvaluateCreateCastCount, RejectsExistingIndex)
+{
+    EXPECT_EQ(Server_Player::evaluateCreateCastCount(true, false, true, 1, /*exists=*/true, true),
+              Response::RespContextError);
+}
+
+TEST(EvaluateCreateCastCount, RejectsGapInSequence)
+{
+    EXPECT_EQ(
+        Server_Player::evaluateCreateCastCount(true, false, true, 3, /*exists=*/false, /*predecessorExists=*/false),
+        Response::RespContextError);
+}
+
+TEST(EvaluateCreateCastCount, AllowsNextInSequence)
+{
+    EXPECT_EQ(
+        Server_Player::evaluateCreateCastCount(true, false, true, 2, /*exists=*/false, /*predecessorExists=*/true),
+        Response::RespOk);
+}
+
+TEST(EvaluateDeleteCastCount, RejectsWhenGameNotStarted)
+{
+    EXPECT_EQ(Server_Player::evaluateDeleteCastCount(/*gameStarted=*/false, /*playerConceded=*/false,
+                                                     /*commandZoneEnabled=*/true, 1, /*exists=*/true, /*value=*/0,
+                                                     /*successorExists=*/false),
+              Response::RespGameNotStarted);
+}
+
+TEST(EvaluateDeleteCastCount, RejectsWhenPlayerConceded)
+{
+    EXPECT_EQ(Server_Player::evaluateDeleteCastCount(true, /*playerConceded=*/true, true, 1, true, 0, false),
+              Response::RespContextError);
+}
+
+TEST(EvaluateDeleteCastCount, RejectsWhenCommandZoneDisabled)
+{
+    EXPECT_EQ(Server_Player::evaluateDeleteCastCount(true, false, /*commandZoneEnabled=*/false, 1, true, 0, false),
+              Response::RespContextError);
+}
+
+TEST(EvaluateDeleteCastCount, RejectsMissingIndex)
+{
+    EXPECT_EQ(Server_Player::evaluateDeleteCastCount(true, false, true, 1, /*exists=*/false, 0, false),
+              Response::RespNameNotFound);
+}
+
+TEST(EvaluateDeleteCastCount, RejectsNonZeroValue)
+{
+    EXPECT_EQ(Server_Player::evaluateDeleteCastCount(true, false, true, 1, true, /*value=*/5, false),
+              Response::RespContextError);
+}
+
+TEST(EvaluateDeleteCastCount, RejectsNonHighestIndex)
+{
+    EXPECT_EQ(Server_Player::evaluateDeleteCastCount(true, false, true, 1, true, 0, /*successorExists=*/true),
+              Response::RespContextError);
+}
+
+TEST(EvaluateDeleteCastCount, AllowsHighestWithZeroValue)
+{
+    EXPECT_EQ(Server_Player::evaluateDeleteCastCount(true, false, true, 2, /*exists=*/true, /*value=*/0,
+                                                     /*successorExists=*/false),
               Response::RespOk);
 }
 
-TEST(EvaluateModifyCounter, RejectsMissingTaxCounter)
+TEST(EvaluateModifyCastCount, RejectsWhenGameNotStarted)
 {
-    EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, /*commandZoneEnabled=*/true, CounterIds::TaxCounter1,
-                                                   nullptr),
+    EXPECT_EQ(Server_Player::evaluateModifyCastCount(/*gameStarted=*/false, /*playerConceded=*/false,
+                                                     /*commandZoneEnabled=*/true, 1, /*exists=*/true),
+              Response::RespGameNotStarted);
+}
+
+TEST(EvaluateModifyCastCount, RejectsWhenPlayerConceded)
+{
+    EXPECT_EQ(Server_Player::evaluateModifyCastCount(true, /*playerConceded=*/true, true, 1, true),
+              Response::RespContextError);
+}
+
+TEST(EvaluateModifyCastCount, RejectsWhenCommandZoneDisabled)
+{
+    EXPECT_EQ(Server_Player::evaluateModifyCastCount(true, false, /*commandZoneEnabled=*/false, 1, true),
+              Response::RespContextError);
+}
+
+TEST(EvaluateModifyCastCount, RejectsMissingIndex)
+{
+    EXPECT_EQ(Server_Player::evaluateModifyCastCount(true, false, true, 1, /*exists=*/false),
               Response::RespNameNotFound);
 }
 
+TEST(EvaluateModifyCastCount, AllowsExistingIndex)
+{
+    EXPECT_EQ(Server_Player::evaluateModifyCastCount(true, false, true, 1, /*exists=*/true), Response::RespOk);
+}
+
 int main(int argc, char **argv)
 {
     ::testing::InitGoogleTest(&argc, argv);

tests/command_zone_tests/new_counter_id_test.cpp

@@ -49,14 +49,6 @@ TEST(NewCounterId, SkipsReservedRangeWhenOnlyReservedCountersExist)
     EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId);
 }
 
-TEST(NewCounterId, SkipsTaxCounterIds)
-{
-    PlayerFixture f;
-    f.player.addCounter(new Server_Counter(CounterIds::TaxCounter1, "tax1", color(), 0, 0));
-    f.player.addCounter(new Server_Counter(CounterIds::TaxCounter2, "tax2", color(), 0, 0));
-    EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId);
-}
-
 TEST(NewCounterId, ReturnsNextIdAboveHighestUserCounter)
 {
     PlayerFixture f;

tests/command_zone_tests/setup_zones_command_zone_test.cpp

@@ -47,7 +47,7 @@ struct CommandZoneTestPlayer : Server_Player
 
 /**
  * @brief Builds a Server_Player on a minimal fake game whose command zone is toggled by the
- *        constructor argument, then runs setupZones() so getCounters() reflects the result.
+ *        constructor argument, then runs setupZones() so getCastCounts() reflects the result.
  */
 struct SetupFixture
 {
@@ -86,55 +86,27 @@ struct SetupFixture
 };
 } // namespace
 
-TEST(SetupZonesCommandZone, CreatesAllTaxCountersWhenEnabled)
+TEST(SetupZonesCommandZone, CreatesFirstCastCountWhenEnabled)
 {
     SetupFixture f(true);
-    const QMap<int, Server_Counter *> &counters = f.player.getCounters();
+    const QMap<int, int> &castCounts = f.player.getCastCounts();
 
     EXPECT_TRUE(f.player.getZones().contains(ZoneNames::COMMAND));
 
-    for (int i = 0; i < CounterIds::TaxCounterCount; ++i) {
-        int id = CounterIds::taxCounterIdFromIndex(i);
-        ASSERT_TRUE(counters.contains(id));
-    }
-
-    const Server_Counter *tax1 = counters.value(CounterIds::TaxCounter1);
-    EXPECT_TRUE(tax1->isActive());
-    EXPECT_EQ(tax1->getCount(), 0);
-
-    for (int i = 1; i < CounterIds::TaxCounterCount; ++i) {
-        int id = CounterIds::taxCounterIdFromIndex(i);
-        const Server_Counter *counter = counters.value(id);
-        EXPECT_FALSE(counter->isActive());
-        EXPECT_EQ(counter->getCount(), 0);
-    }
+    EXPECT_EQ(castCounts.size(), 1);
+    EXPECT_TRUE(castCounts.contains(1));
+    EXPECT_EQ(castCounts.value(1), 0);
 }
 
-TEST(SetupZonesCommandZone, TaxCountersUseBounds)
-{
-    SetupFixture f(true);
-    Server_Counter *tax1 = f.player.getCounters().value(CounterIds::TaxCounter1);
-    ASSERT_NE(tax1, nullptr);
-
-    EXPECT_TRUE(tax1->setCount(MAX_COUNTER_VALUE + 1000));
-    EXPECT_EQ(tax1->getCount(), MAX_COUNTER_VALUE);
-    EXPECT_TRUE(tax1->setCount(-1));
-    EXPECT_EQ(tax1->getCount(), 0);
-}
-
-TEST(SetupZonesCommandZone, NoTaxCountersWhenDisabled)
+TEST(SetupZonesCommandZone, NoCastCountsWhenDisabled)
 {
     SetupFixture f(false);
-    const QMap<int, Server_Counter *> &counters = f.player.getCounters();
+    const QMap<int, int> &castCounts = f.player.getCastCounts();
 
     EXPECT_FALSE(f.player.getZones().contains(ZoneNames::COMMAND));
+    EXPECT_TRUE(castCounts.isEmpty());
 
-    for (int i = 0; i < CounterIds::TaxCounterCount; ++i) {
-        int id = CounterIds::taxCounterIdFromIndex(i);
-        EXPECT_FALSE(counters.contains(id));
-    }
-
-    EXPECT_TRUE(counters.contains(0));
+    EXPECT_TRUE(f.player.getCounters().contains(0));
 }
 
 int main(int argc, char **argv)

tests/server_counter_test.cpp

@@ -102,13 +102,13 @@ TEST(ServerCounter, IncrementClampsToCustomBounds)
     EXPECT_EQ(c.getCount(), 0);
 }
 
-TEST(ServerCounter, CustomBoundsForCommanderTax)
+TEST(ServerCounter, CustomBoundsForCastCount)
 {
-    Server_Counter taxCounter(1, "tax", color(), 20, 0, 0, MAX_COUNTER_VALUE);
-    EXPECT_TRUE(taxCounter.setCount(1000));
-    EXPECT_EQ(taxCounter.getCount(), MAX_COUNTER_VALUE);
-    EXPECT_TRUE(taxCounter.setCount(-5));
-    EXPECT_EQ(taxCounter.getCount(), 0);
+    Server_Counter castCount(1, "cast_count", color(), 20, 0, 0, MAX_COUNTER_VALUE);
+    EXPECT_TRUE(castCount.setCount(1000));
+    EXPECT_EQ(castCount.getCount(), MAX_COUNTER_VALUE);
+    EXPECT_TRUE(castCount.setCount(-5));
+    EXPECT_EQ(castCount.getCount(), 0);
 }
 
 TEST(ServerCounter, SetActiveReportsChangeOnce)