diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_card.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_card.cpp index b858314c0..a4e75e416 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_card.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_card.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -139,10 +140,8 @@ bool Server_Card::setCounter(int _id, int value, Event_SetCardCounter *event) bool Server_Card::incrementCounter(int counterId, int delta, Event_SetCardCounter *event) { const int oldValue = counters.value(counterId, 0); - const auto result = static_cast(oldValue) + static_cast(delta); // Clamp to [0, MAX_COUNTERS_ON_CARD] for card counters - const int newValue = - static_cast(qBound(static_cast(0), result, static_cast(MAX_COUNTERS_ON_CARD))); + const int newValue = addClamped(oldValue, delta, 0, MAX_COUNTERS_ON_CARD); if (newValue == oldValue) { return false; diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_counter.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_counter.cpp index e65205cbb..b18e11c2b 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_counter.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_counter.cpp @@ -1,24 +1,12 @@ #include "server_counter.h" #include -#include Server_Counter::Server_Counter(int _id, const QString &_name, const color &_counterColor, int _radius, int _count) : id(_id), name(_name), counterColor(_counterColor), radius(_radius), count(_count) { } -//! \todo Extract overflow-safe arithmetic into shared helper. -//! Duplicated in Server_Card::incrementCounter() - keep in sync if modified. -bool Server_Counter::incrementCount(int delta) -{ - const int oldCount = count; - const auto result = static_cast(count) + static_cast(delta); - count = static_cast(qBound(static_cast(std::numeric_limits::min()), result, - static_cast(std::numeric_limits::max()))); - return count != oldCount; -} - void Server_Counter::getInfo(ServerInfo_Counter *info) { info->set_id(id); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_counter.h b/libcockatrice_network/libcockatrice/network/server/remote/game/server_counter.h index 8226e663f..ca093b7cf 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_counter.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_counter.h @@ -22,6 +22,8 @@ #include #include +#include +#include class ServerInfo_Counter; @@ -92,7 +94,12 @@ public: * @return true if the value changed, false otherwise. * @note Clamps result to [INT_MIN, INT_MAX] to prevent overflow. */ - [[nodiscard]] bool incrementCount(int delta); + [[nodiscard]] bool incrementCount(int delta) + { + const int oldCount = count; + count = addClamped(count, delta, std::numeric_limits::min(), std::numeric_limits::max()); + return count != oldCount; + } /** * @brief Populates info with this counter's current state for network serialization. diff --git a/libcockatrice_utility/CMakeLists.txt b/libcockatrice_utility/CMakeLists.txt index df29d6c9f..13f13bb56 100644 --- a/libcockatrice_utility/CMakeLists.txt +++ b/libcockatrice_utility/CMakeLists.txt @@ -16,6 +16,7 @@ set(UTILITY_HEADERS libcockatrice/utility/macros.h libcockatrice/utility/passwordhasher.h libcockatrice/utility/trice_limits.h + libcockatrice/utility/clamped_arithmetic.h libcockatrice/utility/zone_names.h libcockatrice/utility/days_years_between.h ) diff --git a/libcockatrice_utility/libcockatrice/utility/clamped_arithmetic.h b/libcockatrice_utility/libcockatrice/utility/clamped_arithmetic.h new file mode 100644 index 000000000..5391cdbd8 --- /dev/null +++ b/libcockatrice_utility/libcockatrice/utility/clamped_arithmetic.h @@ -0,0 +1,22 @@ +#ifndef CLAMPED_ARITHMETIC_H +#define CLAMPED_ARITHMETIC_H + +#include +#include + +/** + * @brief Overflow-safe clamped addition: returns value + delta bounded to [minValue, maxValue]. + * + * Uses a 64-bit intermediate so the addition itself cannot overflow int. Shared by the + * counter arithmetic in Server_Card and Server_Counter so both stay in sync. + * + * @note Requires minValue <= maxValue. Bounds come from trusted compile-time call sites; + * qBound() asserts this internally in debug builds. + */ +inline int addClamped(int value, int delta, int minValue, int maxValue) +{ + const auto result = static_cast(value) + static_cast(delta); + return static_cast(qBound(static_cast(minValue), result, static_cast(maxValue))); +} + +#endif // CLAMPED_ARITHMETIC_H diff --git a/libcockatrice_utility/libcockatrice/utility/trice_limits.h b/libcockatrice_utility/libcockatrice/utility/trice_limits.h index 833ce1b98..220e23a9e 100644 --- a/libcockatrice_utility/libcockatrice/utility/trice_limits.h +++ b/libcockatrice_utility/libcockatrice/utility/trice_limits.h @@ -1,6 +1,9 @@ #ifndef TRICE_LIMITS_H #define TRICE_LIMITS_H +//! \todo Split trice_limits.h into focused single-purpose headers: string_limits.h, +//! dice_limits.h, counter_limits.h. + #include // max size for short strings, like names and things that are generally a single phrase @@ -16,7 +19,9 @@ constexpr uint MINIMUM_DICE_TO_ROLL = 1; constexpr uint MAXIMUM_DICE_TO_ROLL = 100; // Card counter value bounds [0, MAX_COUNTERS_ON_CARD]. -// Counters on cards (e.g., +1/+1 counters, charge counters) are non-negative physical game objects. +// This caps an individual counter's VALUE (e.g. a +1/+1 counter at 999), not how many counters a card holds. +// Applies to card counters only; player counters (Server_Counter) are unbounded and may go +// negative (e.g. life total), saturating only at the int range. // The max of 999 is a display constraint (3-digit rendering) and reasonable gameplay limit. // Server enforces these bounds; client may also check for UX optimization. constexpr int MAX_COUNTERS_ON_CARD = 999;