mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-04-30 04:16:57 -05:00
Refactor/z value constants (#6651)
* feat(z-values): add centralized Z-value constants infrastructure Magic numbers scattered across the codebase make Z-value layering hard to understand and maintain. Centralizing them provides: - Self-documenting layer hierarchy - Validation utilities for development - Single source of truth for Z-value ranges Two-tier header design: - z_value_layer_manager.h: Foundation with constants and validation - z_values.h: User-facing namespace with semantic constants * refactor(z-values): replace magic Z-value numbers with ZValues constants Magic numbers like 2000000007 are impossible to understand without context. Named constants (ZValues::DRAG_ITEM) are self-documenting. This intentionally renumbers overlay Z-values to use a cleaner offset sequence. The relative stacking order is preserved, which is what matters for correct rendering. Each consumer now includes z_values.h and uses semantic constants instead of magic numbers. * refactor(z-values): removed redundant inline with contexpr, Updated doc, magic numbers removed from TableZone.
This commit is contained in:
parent
1bcea27a44
commit
04f06206b7
|
|
@ -1,6 +1,7 @@
|
||||||
#include "abstract_card_drag_item.h"
|
#include "abstract_card_drag_item.h"
|
||||||
|
|
||||||
#include "../../client/settings/cache_settings.h"
|
#include "../../client/settings/cache_settings.h"
|
||||||
|
#include "../z_values.h"
|
||||||
|
|
||||||
#include <QCursor>
|
#include <QCursor>
|
||||||
#include <QDebug>
|
#include <QDebug>
|
||||||
|
|
@ -18,13 +19,13 @@ AbstractCardDragItem::AbstractCardDragItem(AbstractCardItem *_item,
|
||||||
{
|
{
|
||||||
if (parentDrag) {
|
if (parentDrag) {
|
||||||
parentDrag->addChildDrag(this);
|
parentDrag->addChildDrag(this);
|
||||||
setZValue(2000000007 + hotSpot.x() * 1000000 + hotSpot.y() * 1000 + 1000);
|
setZValue(ZValues::childDragZValue(hotSpot.x(), hotSpot.y()));
|
||||||
connect(parentDrag, &QObject::destroyed, this, &AbstractCardDragItem::deleteLater);
|
connect(parentDrag, &QObject::destroyed, this, &AbstractCardDragItem::deleteLater);
|
||||||
} else {
|
} else {
|
||||||
hotSpot = QPointF{qBound(0.0, hotSpot.x(), static_cast<qreal>(CARD_WIDTH - 1)),
|
hotSpot = QPointF{qBound(0.0, hotSpot.x(), static_cast<qreal>(CARD_WIDTH - 1)),
|
||||||
qBound(0.0, hotSpot.y(), static_cast<qreal>(CARD_HEIGHT - 1))};
|
qBound(0.0, hotSpot.y(), static_cast<qreal>(CARD_HEIGHT - 1))};
|
||||||
setCursor(Qt::ClosedHandCursor);
|
setCursor(Qt::ClosedHandCursor);
|
||||||
setZValue(2000000007);
|
setZValue(ZValues::DRAG_ITEM);
|
||||||
}
|
}
|
||||||
if (item->getTapped())
|
if (item->getTapped())
|
||||||
setTransform(QTransform()
|
setTransform(QTransform()
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,7 @@
|
||||||
#include "../../client/settings/cache_settings.h"
|
#include "../../client/settings/cache_settings.h"
|
||||||
#include "../../interface/card_picture_loader/card_picture_loader.h"
|
#include "../../interface/card_picture_loader/card_picture_loader.h"
|
||||||
#include "../game_scene.h"
|
#include "../game_scene.h"
|
||||||
|
#include "../z_values.h"
|
||||||
|
|
||||||
#include <QCursor>
|
#include <QCursor>
|
||||||
#include <QGraphicsScene>
|
#include <QGraphicsScene>
|
||||||
|
|
@ -215,7 +216,7 @@ void AbstractCardItem::setHovered(bool _hovered)
|
||||||
if (_hovered)
|
if (_hovered)
|
||||||
processHoverEvent();
|
processHoverEvent();
|
||||||
isHovered = _hovered;
|
isHovered = _hovered;
|
||||||
setZValue(_hovered ? 2000000004 : realZValue);
|
setZValue(_hovered ? ZValues::HOVERED_CARD : realZValue);
|
||||||
setScale(_hovered && SettingsCache::instance().getScaleCards() ? 1.1 : 1);
|
setScale(_hovered && SettingsCache::instance().getScaleCards() ? 1.1 : 1);
|
||||||
setTransformOriginPoint(_hovered ? CARD_WIDTH / 2 : 0, _hovered ? CARD_HEIGHT / 2 : 0);
|
setTransformOriginPoint(_hovered ? CARD_WIDTH / 2 : 0, _hovered ? CARD_HEIGHT / 2 : 0);
|
||||||
update();
|
update();
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@
|
||||||
#include "../player/player.h"
|
#include "../player/player.h"
|
||||||
#include "../player/player_actions.h"
|
#include "../player/player_actions.h"
|
||||||
#include "../player/player_target.h"
|
#include "../player/player_target.h"
|
||||||
|
#include "../z_values.h"
|
||||||
#include "../zones/card_zone.h"
|
#include "../zones/card_zone.h"
|
||||||
#include "card_item.h"
|
#include "card_item.h"
|
||||||
|
|
||||||
|
|
@ -23,7 +24,7 @@ ArrowItem::ArrowItem(Player *_player, int _id, ArrowTarget *_startItem, ArrowTar
|
||||||
: QGraphicsItem(), player(_player), id(_id), startItem(_startItem), targetItem(_targetItem), targetLocked(false),
|
: QGraphicsItem(), player(_player), id(_id), startItem(_startItem), targetItem(_targetItem), targetLocked(false),
|
||||||
color(_color), fullColor(true)
|
color(_color), fullColor(true)
|
||||||
{
|
{
|
||||||
setZValue(2000000005);
|
setZValue(ZValues::ARROWS);
|
||||||
|
|
||||||
if (startItem)
|
if (startItem)
|
||||||
startItem->addArrowFrom(this);
|
startItem->addArrowFrom(this);
|
||||||
|
|
|
||||||
136
cockatrice/src/game/z_value_layer_manager.h
Normal file
136
cockatrice/src/game/z_value_layer_manager.h
Normal file
|
|
@ -0,0 +1,136 @@
|
||||||
|
/**
|
||||||
|
* @file z_value_layer_manager.h
|
||||||
|
* @ingroup GameGraphics
|
||||||
|
* @brief Semantic Z-value layer management for game scene rendering.
|
||||||
|
*
|
||||||
|
* This file provides a structured approach to Z-value allocation in the game scene.
|
||||||
|
* Z-values in Qt determine stacking order - higher values render on top of lower values.
|
||||||
|
*
|
||||||
|
* ## Layer Architecture
|
||||||
|
*
|
||||||
|
* The game scene is organized into three conceptual layers:
|
||||||
|
*
|
||||||
|
* 1. **Zone Layer (0-999)**: Zone backgrounds, containers, and static elements
|
||||||
|
* - Zone backgrounds (0.5-1.0)
|
||||||
|
* - Cards within zones (1.0 base + index)
|
||||||
|
*
|
||||||
|
* 2. **Card Layer (1-40,000,000)**: Dynamic card rendering on the table zone
|
||||||
|
* - Cards use formula: (actualY + CARD_HEIGHT) * 100000 + (actualX + 1) * 100
|
||||||
|
* - Maximum card Z-value: ~40,000,000 (with 3 rows, actualY <= ~289)
|
||||||
|
*
|
||||||
|
* 3. **Overlay Layer (2,000,000,000+)**: UI elements that must appear above all cards
|
||||||
|
* - Hovered cards (+1)
|
||||||
|
* - Arrows (+3)
|
||||||
|
* - Zone views (+4)
|
||||||
|
* - Drag items (+5, +6)
|
||||||
|
* - Top UI elements (+7)
|
||||||
|
*
|
||||||
|
* ## Design Rationale
|
||||||
|
*
|
||||||
|
* The large gap between card Z-values (max ~40M) and overlay base (2B) provides
|
||||||
|
* safety margin for future table zone expansions while ensuring overlays always
|
||||||
|
* render above cards regardless of table position.
|
||||||
|
*
|
||||||
|
* ## Usage
|
||||||
|
*
|
||||||
|
* Prefer using the semantic constants from ZValues namespace:
|
||||||
|
* @code
|
||||||
|
* card->setZValue(ZValues::HOVERED_CARD);
|
||||||
|
* arrow->setZValue(ZValues::ARROWS);
|
||||||
|
* @endcode
|
||||||
|
*
|
||||||
|
* Use validation functions to verify card Z-values during development:
|
||||||
|
* @code
|
||||||
|
* Q_ASSERT(ZValueLayerManager::isValidCardZValue(cardZ));
|
||||||
|
* @endcode
|
||||||
|
*/
|
||||||
|
|
||||||
|
#ifndef Z_VALUE_LAYER_MANAGER_H
|
||||||
|
#define Z_VALUE_LAYER_MANAGER_H
|
||||||
|
|
||||||
|
#include <QtGlobal>
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @namespace ZValueLayerManager
|
||||||
|
* @brief Utilities for Z-value validation and layer management.
|
||||||
|
*/
|
||||||
|
namespace ZValueLayerManager
|
||||||
|
{
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @enum Layer
|
||||||
|
* @brief Semantic layer identifiers for Z-value allocation.
|
||||||
|
*
|
||||||
|
* These represent conceptual rendering layers, not actual Z-values.
|
||||||
|
* Use the corresponding ZValues constants for actual rendering.
|
||||||
|
*/
|
||||||
|
enum class Layer
|
||||||
|
{
|
||||||
|
/// Zone-level elements like backgrounds and containers
|
||||||
|
Zone,
|
||||||
|
/// Cards rendered in zones (uses sequential Z-values)
|
||||||
|
Card,
|
||||||
|
/// Temporary UI elements like hovered cards and drag items
|
||||||
|
Overlay
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Maximum Z-value a card can have on the table zone.
|
||||||
|
*
|
||||||
|
* Based on table zone formula: (actualY + CARD_HEIGHT) * 100000 + (actualX + 1) * 100
|
||||||
|
* With maximum 3 rows and CARD_HEIGHT ~96, actualY <= ~289.
|
||||||
|
* Maximum: (289 + 96) * 100000 + 100 * 100 = 38,510,000
|
||||||
|
*
|
||||||
|
* We use 40,000,000 as a safe upper bound with margin.
|
||||||
|
*/
|
||||||
|
constexpr qreal CARD_Z_VALUE_MAX = 40000000.0;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Base Z-value for overlay elements.
|
||||||
|
*
|
||||||
|
* Must exceed CARD_Z_VALUE_MAX to ensure overlays render above all cards.
|
||||||
|
* The 50x margin (2B vs 40M) provides safety for future expansion.
|
||||||
|
*/
|
||||||
|
constexpr qreal OVERLAY_BASE = 2000000000.0;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Validates that a Z-value is within the valid card range.
|
||||||
|
*
|
||||||
|
* Cards should have Z-values between CARD_BASE (1.0) and CARD_Z_VALUE_MAX.
|
||||||
|
* Values outside this range may interfere with overlay rendering.
|
||||||
|
*
|
||||||
|
* @param zValue The Z-value to validate
|
||||||
|
* @return true if the Z-value is valid for a card
|
||||||
|
*/
|
||||||
|
[[nodiscard]] constexpr bool isValidCardZValue(qreal zValue)
|
||||||
|
{
|
||||||
|
return zValue >= 1.0 && zValue <= CARD_Z_VALUE_MAX;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Validates that a Z-value is in the overlay layer.
|
||||||
|
*
|
||||||
|
* Overlay elements should have Z-values at or above OVERLAY_BASE.
|
||||||
|
*
|
||||||
|
* @param zValue The Z-value to validate
|
||||||
|
* @return true if the Z-value is valid for an overlay element
|
||||||
|
*/
|
||||||
|
[[nodiscard]] constexpr bool isOverlayZValue(qreal zValue)
|
||||||
|
{
|
||||||
|
return zValue >= OVERLAY_BASE;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Returns the Z-value for a specific overlay element.
|
||||||
|
*
|
||||||
|
* @param offset Offset from OVERLAY_BASE (0-7 for current elements)
|
||||||
|
* @return The absolute Z-value for the overlay element
|
||||||
|
*/
|
||||||
|
[[nodiscard]] constexpr qreal overlayZValue(qreal offset)
|
||||||
|
{
|
||||||
|
return OVERLAY_BASE + offset;
|
||||||
|
}
|
||||||
|
|
||||||
|
} // namespace ZValueLayerManager
|
||||||
|
|
||||||
|
#endif // Z_VALUE_LAYER_MANAGER_H
|
||||||
83
cockatrice/src/game/z_values.h
Normal file
83
cockatrice/src/game/z_values.h
Normal file
|
|
@ -0,0 +1,83 @@
|
||||||
|
#ifndef Z_VALUES_H
|
||||||
|
#define Z_VALUES_H
|
||||||
|
|
||||||
|
#include "z_value_layer_manager.h"
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @file z_values.h
|
||||||
|
* @ingroup GameGraphics
|
||||||
|
* @brief Centralized Z-value constants for rendering layer order.
|
||||||
|
*
|
||||||
|
* Z-values in Qt determine stacking order. Higher values render on top.
|
||||||
|
* These constants define the visual layering hierarchy for the game scene.
|
||||||
|
*
|
||||||
|
* ## Layer Architecture
|
||||||
|
*
|
||||||
|
* See z_value_layer_manager.h for detailed documentation on the three-layer
|
||||||
|
* architecture (Zone, Card, Overlay) and the rationale for Z-value choices.
|
||||||
|
*
|
||||||
|
* ## Quick Reference
|
||||||
|
*
|
||||||
|
* | Layer | Z-Value Range | Purpose |
|
||||||
|
* |----------|------------------|-----------------------------------|
|
||||||
|
* | Zone | 0.5 - 1.0 | Zone backgrounds, containers |
|
||||||
|
* | Card | 1.0 - 40,000,000 | Cards on table (position-based) |
|
||||||
|
* | Overlay | 2,000,000,000+ | UI elements above all cards |
|
||||||
|
*/
|
||||||
|
|
||||||
|
namespace ZValues
|
||||||
|
{
|
||||||
|
|
||||||
|
// Expose base for callers that need it
|
||||||
|
constexpr qreal OVERLAY_BASE = ZValueLayerManager::OVERLAY_BASE;
|
||||||
|
|
||||||
|
// Overlay layer Z-values for items that should appear above normal cards
|
||||||
|
constexpr qreal HOVERED_CARD = ZValueLayerManager::overlayZValue(1.0);
|
||||||
|
constexpr qreal ARROWS = ZValueLayerManager::overlayZValue(3.0);
|
||||||
|
constexpr qreal ZONE_VIEW_WIDGET = ZValueLayerManager::overlayZValue(4.0);
|
||||||
|
constexpr qreal DRAG_ITEM = ZValueLayerManager::overlayZValue(5.0);
|
||||||
|
constexpr qreal DRAG_ITEM_CHILD = ZValueLayerManager::overlayZValue(6.0);
|
||||||
|
constexpr qreal TOP_UI = ZValueLayerManager::overlayZValue(7.0);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Compute Z-value for child drag items based on hotspot position.
|
||||||
|
*
|
||||||
|
* When dragging multiple cards together, each child card needs a unique Z-value
|
||||||
|
* to prevent Z-fighting (flickering/flashing). The Z-values are derived from
|
||||||
|
* their position when grabbed to conserve original stacking. The formula encodes
|
||||||
|
* 2D coordinates into a single value where X has higher weight, ensuring
|
||||||
|
* deterministic visual stacking.
|
||||||
|
*
|
||||||
|
* @param hotSpotX The X coordinate of the grab position
|
||||||
|
* @param hotSpotY The Y coordinate of the grab position
|
||||||
|
* @return Unique Z-value for the child drag item
|
||||||
|
*/
|
||||||
|
[[nodiscard]] constexpr qreal childDragZValue(qreal hotSpotX, qreal hotSpotY)
|
||||||
|
{
|
||||||
|
return DRAG_ITEM_CHILD + hotSpotX * 1000000 + hotSpotY * 1000 + 1000;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Compute Z-value for cards on the table zone based on position.
|
||||||
|
*
|
||||||
|
* Cards lower on the table (higher Y) render above cards higher up,
|
||||||
|
* and cards to the right (higher X) render above cards to the left.
|
||||||
|
* This creates natural visual stacking for overlapping cards.
|
||||||
|
*
|
||||||
|
* @param x The X coordinate of the card position
|
||||||
|
* @param y The Y coordinate of the card position
|
||||||
|
* @return Z-value for the card's table position
|
||||||
|
*/
|
||||||
|
[[nodiscard]] constexpr qreal tableCardZValue(qreal x, qreal y)
|
||||||
|
{
|
||||||
|
constexpr qreal CARD_HEIGHT_FOR_Z = 102.0;
|
||||||
|
return (y + CARD_HEIGHT_FOR_Z) * 100000.0 + (x + 1) * 100.0;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Card layering (general architecture, not command-zone specific)
|
||||||
|
constexpr qreal CARD_BASE = 1.0;
|
||||||
|
constexpr qreal CARD_MAX = ZValueLayerManager::CARD_Z_VALUE_MAX;
|
||||||
|
|
||||||
|
} // namespace ZValues
|
||||||
|
|
||||||
|
#endif // Z_VALUES_H
|
||||||
|
|
@ -7,6 +7,7 @@
|
||||||
#include "../board/card_item.h"
|
#include "../board/card_item.h"
|
||||||
#include "../player/player.h"
|
#include "../player/player.h"
|
||||||
#include "../player/player_actions.h"
|
#include "../player/player_actions.h"
|
||||||
|
#include "../z_values.h"
|
||||||
#include "logic/table_zone_logic.h"
|
#include "logic/table_zone_logic.h"
|
||||||
|
|
||||||
#include <QGraphicsScene>
|
#include <QGraphicsScene>
|
||||||
|
|
@ -169,7 +170,7 @@ void TableZone::reorganizeCards()
|
||||||
actualY += 15;
|
actualY += 15;
|
||||||
|
|
||||||
getLogic()->getCards()[i]->setPos(actualX, actualY);
|
getLogic()->getCards()[i]->setPos(actualX, actualY);
|
||||||
getLogic()->getCards()[i]->setRealZValue((actualY + CARD_HEIGHT) * 100000 + (actualX + 1) * 100);
|
getLogic()->getCards()[i]->setRealZValue(ZValues::tableCardZValue(actualX, actualY));
|
||||||
|
|
||||||
QListIterator<CardItem *> attachedCardIterator(getLogic()->getCards()[i]->getAttachedCards());
|
QListIterator<CardItem *> attachedCardIterator(getLogic()->getCards()[i]->getAttachedCards());
|
||||||
int j = 0;
|
int j = 0;
|
||||||
|
|
@ -179,7 +180,7 @@ void TableZone::reorganizeCards()
|
||||||
qreal childX = actualX - j * STACKED_CARD_OFFSET_X;
|
qreal childX = actualX - j * STACKED_CARD_OFFSET_X;
|
||||||
qreal childY = y + 5;
|
qreal childY = y + 5;
|
||||||
attachedCard->setPos(childX, childY);
|
attachedCard->setPos(childX, childY);
|
||||||
attachedCard->setRealZValue((childY + CARD_HEIGHT) * 100000 + (childX + 1) * 100);
|
attachedCard->setRealZValue(ZValues::tableCardZValue(childX, childY));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@
|
||||||
#include "../game_scene.h"
|
#include "../game_scene.h"
|
||||||
#include "../player/player.h"
|
#include "../player/player.h"
|
||||||
#include "../player/player_actions.h"
|
#include "../player/player_actions.h"
|
||||||
|
#include "../z_values.h"
|
||||||
#include "view_zone.h"
|
#include "view_zone.h"
|
||||||
|
|
||||||
#include <QCheckBox>
|
#include <QCheckBox>
|
||||||
|
|
@ -47,7 +48,7 @@ ZoneViewWidget::ZoneViewWidget(Player *_player,
|
||||||
{
|
{
|
||||||
setAcceptHoverEvents(true);
|
setAcceptHoverEvents(true);
|
||||||
setAttribute(Qt::WA_DeleteOnClose);
|
setAttribute(Qt::WA_DeleteOnClose);
|
||||||
setZValue(2000000006);
|
setZValue(ZValues::ZONE_VIEW_WIDGET);
|
||||||
setFlag(ItemIgnoresTransformations);
|
setFlag(ItemIgnoresTransformations);
|
||||||
|
|
||||||
QGraphicsLinearLayout *vbox = new QGraphicsLinearLayout(Qt::Vertical);
|
QGraphicsLinearLayout *vbox = new QGraphicsLinearLayout(Qt::Vertical);
|
||||||
|
|
@ -71,7 +72,7 @@ ZoneViewWidget::ZoneViewWidget(Player *_player,
|
||||||
|
|
||||||
QGraphicsProxyWidget *searchEditProxy = new QGraphicsProxyWidget;
|
QGraphicsProxyWidget *searchEditProxy = new QGraphicsProxyWidget;
|
||||||
searchEditProxy->setWidget(&searchEdit);
|
searchEditProxy->setWidget(&searchEdit);
|
||||||
searchEditProxy->setZValue(2000000007);
|
searchEditProxy->setZValue(ZValues::DRAG_ITEM);
|
||||||
vbox->addItem(searchEditProxy);
|
vbox->addItem(searchEditProxy);
|
||||||
|
|
||||||
// top row
|
// top row
|
||||||
|
|
@ -80,13 +81,13 @@ ZoneViewWidget::ZoneViewWidget(Player *_player,
|
||||||
// groupBy options
|
// groupBy options
|
||||||
QGraphicsProxyWidget *groupBySelectorProxy = new QGraphicsProxyWidget;
|
QGraphicsProxyWidget *groupBySelectorProxy = new QGraphicsProxyWidget;
|
||||||
groupBySelectorProxy->setWidget(&groupBySelector);
|
groupBySelectorProxy->setWidget(&groupBySelector);
|
||||||
groupBySelectorProxy->setZValue(2000000008);
|
groupBySelectorProxy->setZValue(ZValues::TOP_UI);
|
||||||
hTopRow->addItem(groupBySelectorProxy);
|
hTopRow->addItem(groupBySelectorProxy);
|
||||||
|
|
||||||
// sortBy options
|
// sortBy options
|
||||||
QGraphicsProxyWidget *sortBySelectorProxy = new QGraphicsProxyWidget;
|
QGraphicsProxyWidget *sortBySelectorProxy = new QGraphicsProxyWidget;
|
||||||
sortBySelectorProxy->setWidget(&sortBySelector);
|
sortBySelectorProxy->setWidget(&sortBySelector);
|
||||||
sortBySelectorProxy->setZValue(2000000007);
|
sortBySelectorProxy->setZValue(ZValues::DRAG_ITEM);
|
||||||
hTopRow->addItem(sortBySelectorProxy);
|
hTopRow->addItem(sortBySelectorProxy);
|
||||||
|
|
||||||
vbox->addItem(hTopRow);
|
vbox->addItem(hTopRow);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user