cleaned up comments from tests

tests/command_zone_tests/counter_command_auth_test.cpp

@@ -1,8 +1,7 @@
-// Unit tests for the pure authorization logic extracted from the counter command
+/** @file counter_command_auth_test.cpp
-// handlers. Server_Player::evaluateDelCounter(), evaluateSetCounterActive() and
+ *  @brief Tests for Server_Player counter-command authorization helpers.
-// evaluateModifyCounter() are static and depend only on their arguments, so each
+ *  @ingroup Tests
-// guard branch can be exercised directly without a started game, network stack, or
+ */
-// player fixture.
 
 #include "game/server_counter.h"
 #include "game/server_player.h"
@@ -17,22 +16,16 @@ RNG_Abstract *rng = nullptr; // required by linked server code
 
 namespace
 {
-// A user-range counter id that is never a reserved tax counter.
 constexpr int UserCounterId = CounterIds::FirstUserId;
 
-// Builds a counter with the given id and current value; the other fields are
-// irrelevant to the authorization decisions under test.
 Server_Counter makeCounter(int id, int count)
 {
     return Server_Counter(id, "c", color(), 20, count);
 }
 } // namespace
 
-// ---------------------------------------------------------------------------
 // evaluateDelCounter
-// ---------------------------------------------------------------------------
 
-// The game must be started before any counter can be deleted.
 TEST(EvaluateDelCounter, RejectsWhenGameNotStarted)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
@@ -41,7 +34,6 @@ TEST(EvaluateDelCounter, RejectsWhenGameNotStarted)
         Response::RespGameNotStarted);
 }
 
-// A conceded player may no longer mutate counters.
 TEST(EvaluateDelCounter, RejectsWhenPlayerConceded)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
@@ -49,7 +41,6 @@ TEST(EvaluateDelCounter, RejectsWhenPlayerConceded)
               Response::RespContextError);
 }
 
-// Reserved tax counters are server-managed and may never be deleted by a client.
 TEST(EvaluateDelCounter, RejectsTaxCounters)
 {
     Server_Counter commander = makeCounter(CounterIds::CommanderTax, 0);
@@ -61,21 +52,17 @@ TEST(EvaluateDelCounter, RejectsTaxCounters)
               Response::RespFunctionNotAllowed);
 }
 
-// A non-existent counter (null lookup) is reported as not found.
 TEST(EvaluateDelCounter, RejectsMissingCounter)
 {
     EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, UserCounterId, nullptr), Response::RespNameNotFound);
 }
 
-// A live user counter in a started, non-conceded game may be deleted.
 TEST(EvaluateDelCounter, AllowsDeletingUserCounter)
 {
     Server_Counter counter = makeCounter(UserCounterId, 7);
     EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, UserCounterId, &counter), Response::RespOk);
 }
 
-// The game-not-started guard is checked before the tax-counter guard: an attempt
-// to delete a tax counter before the game starts reports RespGameNotStarted.
 TEST(EvaluateDelCounter, GameNotStartedTakesPrecedenceOverTaxGuard)
 {
     Server_Counter commander = makeCounter(CounterIds::CommanderTax, 0);
@@ -83,11 +70,8 @@ TEST(EvaluateDelCounter, GameNotStartedTakesPrecedenceOverTaxGuard)
               Response::RespGameNotStarted);
 }
 
-// ---------------------------------------------------------------------------
 // evaluateSetCounterActive
-// ---------------------------------------------------------------------------
 
-// The game must be started before a counter's active state can change.
 TEST(EvaluateSetCounterActive, RejectsWhenGameNotStarted)
 {
     Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0);
@@ -97,7 +81,6 @@ TEST(EvaluateSetCounterActive, RejectsWhenGameNotStarted)
               Response::RespGameNotStarted);
 }
 
-// A conceded player may no longer toggle counters.
 TEST(EvaluateSetCounterActive, RejectsWhenPlayerConceded)
 {
     Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0);
@@ -106,8 +89,6 @@ TEST(EvaluateSetCounterActive, RejectsWhenPlayerConceded)
               Response::RespContextError);
 }
 
-// Only the reserved tax counters support an active/inactive toggle; user counters
-// are rejected as a disallowed operation.
 TEST(EvaluateSetCounterActive, RejectsNonTaxCounter)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
@@ -115,7 +96,6 @@ TEST(EvaluateSetCounterActive, RejectsNonTaxCounter)
               Response::RespFunctionNotAllowed);
 }
 
-// Toggling a tax counter is only meaningful when the command zone is enabled.
 TEST(EvaluateSetCounterActive, RejectsWhenCommandZoneDisabled)
 {
     Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0);
@@ -124,15 +104,12 @@ TEST(EvaluateSetCounterActive, RejectsWhenCommandZoneDisabled)
               Response::RespContextError);
 }
 
-// A missing counter is reported as not found.
 TEST(EvaluateSetCounterActive, RejectsMissingCounter)
 {
     EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::CommanderTax, nullptr, true),
               Response::RespNameNotFound);
 }
 
-// A tax counter may not be disabled while it still holds accumulated tax; the
-// player must reset it to zero first.
 TEST(EvaluateSetCounterActive, RejectsDisablingWhenTaxAccumulated)
 {
     Server_Counter counter = makeCounter(CounterIds::CommanderTax, 3);
@@ -141,7 +118,6 @@ TEST(EvaluateSetCounterActive, RejectsDisablingWhenTaxAccumulated)
               Response::RespContextError);
 }
 
-// Enabling a tax counter is allowed regardless of its current value.
 TEST(EvaluateSetCounterActive, AllowsEnablingWithAccumulatedTax)
 {
     Server_Counter counter = makeCounter(CounterIds::CommanderTax, 3);
@@ -150,7 +126,6 @@ TEST(EvaluateSetCounterActive, AllowsEnablingWithAccumulatedTax)
               Response::RespOk);
 }
 
-// Disabling a tax counter that is already at zero is permitted.
 TEST(EvaluateSetCounterActive, AllowsDisablingWhenCounterIsZero)
 {
     Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0);
@@ -159,8 +134,6 @@ TEST(EvaluateSetCounterActive, AllowsDisablingWhenCounterIsZero)
               Response::RespOk);
 }
 
-// PartnerTax is the second reserved tax counter and must flow through the same auth
-// path as CommanderTax: enabling is always permitted...
 TEST(EvaluateSetCounterActive, AllowsEnablingPartnerTax)
 {
     Server_Counter counter = makeCounter(CounterIds::PartnerTax, 0);
@@ -169,7 +142,6 @@ TEST(EvaluateSetCounterActive, AllowsEnablingPartnerTax)
               Response::RespOk);
 }
 
-// ...and disabling is rejected while it still holds accumulated tax.
 TEST(EvaluateSetCounterActive, RejectsDisablingPartnerTaxWhenAccumulated)
 {
     Server_Counter counter = makeCounter(CounterIds::PartnerTax, 2);
@@ -178,11 +150,8 @@ TEST(EvaluateSetCounterActive, RejectsDisablingPartnerTaxWhenAccumulated)
               Response::RespContextError);
 }
 
-// ---------------------------------------------------------------------------
 // evaluateModifyCounter (shared by cmdIncCounter / cmdSetCounter)
-// ---------------------------------------------------------------------------
 
-// No counter may be modified before the game starts.
 TEST(EvaluateModifyCounter, RejectsWhenGameNotStarted)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
@@ -191,7 +160,6 @@ TEST(EvaluateModifyCounter, RejectsWhenGameNotStarted)
               Response::RespGameNotStarted);
 }
 
-// A conceded player may no longer modify counters.
 TEST(EvaluateModifyCounter, RejectsWhenPlayerConceded)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
@@ -199,7 +167,6 @@ TEST(EvaluateModifyCounter, RejectsWhenPlayerConceded)
               Response::RespContextError);
 }
 
-// An ordinary user counter can be modified regardless of command-zone state.
 TEST(EvaluateModifyCounter, AllowsUserCounter)
 {
     Server_Counter counter = makeCounter(UserCounterId, 0);
@@ -207,14 +174,12 @@ TEST(EvaluateModifyCounter, AllowsUserCounter)
               Response::RespOk);
 }
 
-// A non-existent counter is reported as not found.
 TEST(EvaluateModifyCounter, RejectsMissingCounter)
 {
     EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, true, UserCounterId, nullptr),
               Response::RespNameNotFound);
 }
 
-// A tax counter may not be modified outside a Commander game (command zone disabled).
 TEST(EvaluateModifyCounter, RejectsTaxCounterWhenCommandZoneDisabled)
 {
     Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0);
@@ -223,7 +188,6 @@ TEST(EvaluateModifyCounter, RejectsTaxCounterWhenCommandZoneDisabled)
               Response::RespContextError);
 }
 
-// An inactive (hidden) tax counter must not accumulate value behind the scenes.
 TEST(EvaluateModifyCounter, RejectsInactiveTaxCounter)
 {
     Server_Counter counter = makeCounter(CounterIds::PartnerTax, 0);
@@ -233,7 +197,6 @@ TEST(EvaluateModifyCounter, RejectsInactiveTaxCounter)
               Response::RespContextError);
 }
 
-// An active tax counter in a Commander game may be modified.
 TEST(EvaluateModifyCounter, AllowsActiveTaxCounter)
 {
     Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0);

tests/command_zone_tests/new_counter_id_test.cpp

@@ -1,3 +1,8 @@
+/** @file new_counter_id_test.cpp
+ *  @brief Tests for Server_Player::newCounterId() id allocation.
+ *  @ingroup Tests
+ */
+
 #include "../movecard_tests/server_test_helpers.h"
 #include "game/server_counter.h"
 #include "game/server_game.h"
@@ -14,10 +19,6 @@ RNG_Abstract *rng = nullptr; // required by linked server code
 
 namespace
 {
-// Wires up a Server_Player backed by a minimal fake game so that counters can be
-// added directly via addCounter() to exercise newCounterId() in isolation.
-// newCounterId() depends only on the player's counter map, not on game state,
-// so the game never needs to be started.
 struct PlayerFixture
 {
     ServerInfo_User user;
@@ -34,17 +35,12 @@ struct PlayerFixture
 };
 } // namespace
 
-// With no counters at all, the first allocated id must be FirstUserId, never 0.
 TEST(NewCounterId, ReturnsFirstUserIdWhenNoCounters)
 {
     PlayerFixture f;
     EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId);
 }
 
-// Reserved ids must not drag a new user counter down into the reserved range. The
-// ids here (3 and 5) are chosen so a naive "highest id + 1" implementation would
-// return 6 (inside the reserved range) and fail this test; only the FirstUserId
-// floor yields the correct 10, so this pins the floor against regression.
 TEST(NewCounterId, SkipsReservedRangeWhenOnlyReservedCountersExist)
 {
     PlayerFixture f;
@@ -53,7 +49,6 @@ TEST(NewCounterId, SkipsReservedRangeWhenOnlyReservedCountersExist)
     EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId);
 }
 
-// Once user counters exist, the next id is one above the highest of them.
 TEST(NewCounterId, ReturnsNextIdAboveHighestUserCounter)
 {
     PlayerFixture f;
@@ -62,7 +57,6 @@ TEST(NewCounterId, ReturnsNextIdAboveHighestUserCounter)
     EXPECT_EQ(f.player.newCounterId(), 16);
 }
 
-// A reserved counter present alongside user counters must not affect the result.
 TEST(NewCounterId, IgnoresReservedCountersWhenUserCountersPresent)
 {
     PlayerFixture f;