Unify counter clamp arithmetic into shared addClamped() helper

Rename MAX_COUNTERS_ON_CARD to MAX_COUNTER_VALUE

  Add addClamped() in trice_limits.h, which uses a 64-bit intermediate so
  the addition cannot overflow int. Both Server_Card and Server_Counter use it.

  Add optional [minValue, maxValue] bounds to Server_Counter; setCount()
  and incrementCount() both clamp. Defaults are unbounded, so existing
  callers are unaffected.
This commit is contained in:
DawnFire42 2026-06-19 12:20:50 -04:00
parent 687e6644bc
commit 80946b2fd4
8 changed files with 114 additions and 50 deletions

View file

@ -114,8 +114,8 @@ QString Server_Card::setAttribute(CardAttribute attribute, const QString &avalue
bool Server_Card::setCounter(int _id, int value, Event_SetCardCounter *event)
{
// Clamp to valid card counter range [0, MAX_COUNTERS_ON_CARD]
value = qBound(0, value, MAX_COUNTERS_ON_CARD);
// Clamp to valid card counter range [0, MAX_COUNTER_VALUE]
value = qBound(0, value, MAX_COUNTER_VALUE);
const int oldValue = counters.value(_id, 0);
if (value == oldValue) {
@ -139,10 +139,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<int64_t>(oldValue) + static_cast<int64_t>(delta);
// Clamp to [0, MAX_COUNTERS_ON_CARD] for card counters
const int newValue =
static_cast<int>(qBound(static_cast<int64_t>(0), result, static_cast<int64_t>(MAX_COUNTERS_ON_CARD)));
// Clamp to [0, MAX_COUNTER_VALUE] for card counters
const int newValue = addClamped(oldValue, delta, 0, MAX_COUNTER_VALUE);
if (newValue == oldValue) {
return false;

View file

@ -156,7 +156,7 @@ public:
/**
* @brief Sets a card counter to an exact value with clamping.
* @param _id The counter ID.
* @param value The desired value (clamped to [0, MAX_COUNTERS_ON_CARD]; 0 removes the counter).
* @param value The desired value (clamped to [0, MAX_COUNTER_VALUE]; 0 removes the counter).
* @param event Optional event to populate with counter state.
* @return true if the value changed, false otherwise.
*/
@ -168,7 +168,7 @@ public:
* @param event Optional event to populate with counter state.
* @return true if the value changed, false otherwise.
* @note If counter does not exist, starts from 0. Counter is removed if result is 0.
* @note Clamps result to [0, MAX_COUNTERS_ON_CARD].
* @note Clamps result to [0, MAX_COUNTER_VALUE].
*/
[[nodiscard]] bool incrementCounter(int counterId, int delta, Event_SetCardCounter *event = nullptr);
void setTapped(bool _tapped)

View file

@ -1,24 +1,19 @@
#include "server_counter.h"
#include <libcockatrice/protocol/pb/serverinfo_counter.pb.h>
#include <limits>
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)
Server_Counter::Server_Counter(int _id,
const QString &_name,
const color &_counterColor,
int _radius,
int _count,
int _minValue,
int _maxValue)
: id(_id), name(_name), counterColor(_counterColor), radius(_radius), count(_count), minValue(_minValue),
maxValue(_maxValue)
{
}
//! \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<int64_t>(count) + static_cast<int64_t>(delta);
count = static_cast<int>(qBound(static_cast<int64_t>(std::numeric_limits<int>::min()), result,
static_cast<int64_t>(std::numeric_limits<int>::max())));
return count != oldCount;
}
void Server_Counter::getInfo(ServerInfo_Counter *info)
{
info->set_id(id);

View file

@ -22,18 +22,19 @@
#include <QString>
#include <libcockatrice/protocol/pb/color.pb.h>
#include <libcockatrice/utility/trice_limits.h>
#include <limits>
class ServerInfo_Counter;
/**
* @class Server_Counter
* @brief Represents a player counter with overflow-safe increment arithmetic.
* @brief Represents a player counter with overflow-safe increment arithmetic and optional bounds.
*
* All value modifications return whether the value actually changed,
* enabling callers to skip unnecessary network events.
*
* @note Direct assignment via setCount() does not clamp; only
* incrementCount() enforces int boundary saturation.
* @note Values are clamped to [minValue, maxValue] on both setCount() and incrementCount().
* @note Unlike card counters, player counters are never auto-removed
* when they reach zero - they persist with value 0.
*/
@ -45,9 +46,29 @@ protected:
color counterColor;
int radius;
int count;
int minValue; ///< Minimum allowed value (default: INT_MIN, i.e. unbounded)
int maxValue; ///< Maximum allowed value (default: INT_MAX, i.e. unbounded)
static constexpr int DEFAULT_MAX_VALUE = std::numeric_limits<int>::max();
public:
Server_Counter(int _id, const QString &_name, const color &_counterColor, int _radius, int _count = 0);
/**
* @brief Constructs a counter.
* @param _id Unique counter identifier
* @param _name Display name
* @param _counterColor Counter color
* @param _radius Display radius
* @param _count Initial value (default 0)
* @param _minValue Minimum allowed value (default INT_MIN)
* @param _maxValue Maximum allowed value (default INT_MAX)
*/
Server_Counter(int _id,
const QString &_name,
const color &_counterColor,
int _radius,
int _count = 0,
int _minValue = std::numeric_limits<int>::min(),
int _maxValue = DEFAULT_MAX_VALUE);
~Server_Counter()
{
}
@ -73,16 +94,15 @@ public:
}
/**
* @brief Sets the counter to an exact value.
* @param _count The new value (assigned directly without clamping).
* @return true if the value changed, false otherwise.
* @warning This performs raw assignment. For overflow-safe incrementing,
* use incrementCount().
* @brief Sets the counter value, clamping to [minValue, maxValue].
* @param _count The desired new value
* @return true if the clamped value differs from the previous value
* @note For increment operations, prefer incrementCount() which handles overflow safely.
*/
[[nodiscard]] bool setCount(int _count)
{
const int oldCount = count;
count = _count;
int oldCount = count;
count = qBound(minValue, _count, maxValue);
return count != oldCount;
}
@ -90,9 +110,14 @@ public:
* @brief Increments the counter by delta with overflow-safe arithmetic.
* @param delta The amount to add (may be negative for decrement).
* @return true if the value changed, false otherwise.
* @note Clamps result to [INT_MIN, INT_MAX] to prevent overflow.
* @note Clamps result to [minValue, maxValue] to prevent overflow.
*/
[[nodiscard]] bool incrementCount(int delta);
[[nodiscard]] bool incrementCount(int delta)
{
const int oldCount = count;
count = addClamped(count, delta, minValue, maxValue);
return count != oldCount;
}
/**
* @brief Populates info with this counter's current state for network serialization.