From 392a2b8d006024b52075409ee5f4a1bec43b0c71 Mon Sep 17 00:00:00 2001 From: Malte Langkabel Date: Mon, 17 Aug 2026 16:24:14 +0200 Subject: [PATCH] keep the selection panel half a tile off what it describes The panel had one distance for the view edges, the widgets it steps around, and the selection alike, so it stood eight pixels from a building and touched it outright along the top. The gap from the selection is now its own value, half a tile, and it is horizontal only -- the top edges stay level. It is sampled where the tile size is known, in the same moment as the anchor rectangle, and travels with it: a rectangle frozen in one moment has no meaningful distance to a tile size measured in another. chooseSide now takes the gap in place of the margin, the band having already taken the margin off. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ne3mejABZoLWKLh8fgpM3x --- src/lib/core/FloatingPanelPlacement.cpp | 23 +++--- src/lib/core/FloatingPanelPlacement.h | 24 ++++-- .../event/SelectionAnchorChangedEvent.h | 10 ++- src/test/FloatingPanelPlacementTest.cpp | 73 +++++++++++++------ src/ui/GameWorldView.cpp | 7 +- src/ui/SelectionPanel.cpp | 25 ++++--- src/ui/SelectionPanel.h | 14 ++-- 7 files changed, 115 insertions(+), 61 deletions(-) diff --git a/src/lib/core/FloatingPanelPlacement.cpp b/src/lib/core/FloatingPanelPlacement.cpp index cf1218b..6734ccb 100644 --- a/src/lib/core/FloatingPanelPlacement.cpp +++ b/src/lib/core/FloatingPanelPlacement.cpp @@ -57,12 +57,13 @@ int getAvailableBottomPx(const QRect& band, const std::vector& occupiedRe } PanelSide chooseSide(const QRect& band, const QRect& anchorRect, int widthPx, - int marginPx) + int selectionGapPx) { - // What each side offers: the gap between the anchor and that edge of the band, less - // the margin the panel keeps from the anchor. - const int roomRightPx = band.right() - anchorRect.right() - marginPx; - const int roomLeftPx = anchorRect.left() - band.left() - marginPx; + // What each side offers: the room between the anchor and that edge of the band, less + // the gap the panel keeps from the anchor. The band's own inset from the view has + // already taken the edge margin off. + const int roomRightPx = band.right() - anchorRect.right() - selectionGapPx; + const int roomLeftPx = anchorRect.left() - band.left() - selectionGapPx; if (roomRightPx >= widthPx) { @@ -79,20 +80,20 @@ PanelSide chooseSide(const QRect& band, const QRect& anchorRect, int widthPx, QRect placeBesideAnchor(const QRect& band, const QRect& anchorRect, PanelSide side, QSize wantedSize, const std::vector& occupiedRects, - int marginPx) + int selectionGapPx, int marginPx) { const int widthPx = std::min(wantedSize.width(), band.width()); - // Against the anchor on the chosen side, growing away from it: the edge facing the + // A gap from the anchor on the chosen side, growing away from it: the edge facing the // selection is the one that stays put as the panel's content resizes. A panel that // does not fit there is pushed back inside the view rather than hanging off it, which // is what puts it over the selection when neither side had room. const int wantedLeftPx = (side == PanelSide::Right) - ? anchorRect.right() + marginPx + 1 - : anchorRect.left() - marginPx - widthPx; + ? anchorRect.right() + selectionGapPx + 1 + : anchorRect.left() - selectionGapPx - widthPx; - // Top-aligned with the anchor, then lifted by however much of it hangs below what is - // free. + // Top-aligned with the anchor -- the gap separates the two horizontally and plays no + // part here -- then lifted by however much of the panel hangs below what is free. return fitInBand(band, wantedLeftPx, anchorRect.top(), wantedSize, occupiedRects, marginPx); } diff --git a/src/lib/core/FloatingPanelPlacement.h b/src/lib/core/FloatingPanelPlacement.h index c7f743c..52c7c5c 100644 --- a/src/lib/core/FloatingPanelPlacement.h +++ b/src/lib/core/FloatingPanelPlacement.h @@ -10,6 +10,12 @@ // owner places them in one ordered pass, each into the space the earlier ones left free, // and these are the rules they place themselves by. Pure geometry -- no widget is // involved, which is what lets the rules be tested without a display. +// +// Two distances run through these rules and are deliberately different (see +// REQ-UI-SELECTION-PANEL). marginPx is the edge margin: what a widget keeps from the +// view's edges and from the widgets it steps around. selectionGapPx is the gap the +// selection panel keeps from the selection it describes -- half a tile, which is the +// wider of the two, so the panel stands clear of the objects rather than touching them. // The lowest bottom edge available to a widget occupying the horizontal span // [leftPx, rightPx] inside band: the band's own bottom, or marginPx above the topmost @@ -29,19 +35,21 @@ enum class PanelSide // The side a panel widthPx wide takes beside anchorRect: the right of it where it fits // within band, otherwise the left, and where it fits on neither, whichever side leaves // more room -- the one case in which the panel ends up over the selection -// (REQ-UI-SELECTION-PANEL). Decided once when the selection starts and kept for as long -// as it lasts, so a card that grows later never flips the panel across the object. +// (REQ-UI-SELECTION-PANEL). The room a side offers is what is left of it once the panel's +// gap from the selection is taken off. Decided once when the selection starts and kept for +// as long as it lasts, so a card that grows later never flips the panel across the object. PanelSide chooseSide(const QRect& band, const QRect& anchorRect, int widthPx, - int marginPx); + int selectionGapPx); // Where a panel of wantedSize stands beside anchorRect on the given side: separated from -// it by marginPx and growing away from it, its top edge on the anchor's top edge, pushed -// inside band and above whatever occupies it. The returned height is short of -// wantedSize's when there was not enough room, which is the caller's cue to scroll its -// content (REQ-UI-SELECTION-PANEL). +// it by selectionGapPx and growing away from it, its top edge on the anchor's top edge, +// pushed inside band and above whatever occupies it. The gap is horizontal only -- the +// panel's top sits level with the anchor's, however wide the gap. The returned height is +// short of wantedSize's when there was not enough room, which is the caller's cue to +// scroll its content (REQ-UI-SELECTION-PANEL). QRect placeBesideAnchor(const QRect& band, const QRect& anchorRect, PanelSide side, QSize wantedSize, const std::vector& occupiedRects, - int marginPx); + int selectionGapPx, int marginPx); // Where a panel of wantedSize stands once the player has dragged it to desiredTopLeftPx: // at that point, by the same rules that place it beside a selection -- pushed inside band, diff --git a/src/lib/eventsystem/event/SelectionAnchorChangedEvent.h b/src/lib/eventsystem/event/SelectionAnchorChangedEvent.h index 61c0b14..c57c9dc 100644 --- a/src/lib/eventsystem/event/SelectionAnchorChangedEvent.h +++ b/src/lib/eventsystem/event/SelectionAnchorChangedEvent.h @@ -15,11 +15,17 @@ // is while the selection grows; and because the rectangle is screen space frozen at that // moment, scrolling the view or a selected ship flying off does not move the panel // either. +// +// The gap the panel keeps from that rectangle travels with it, for the same reason: it is +// half a tile as the tile stood in this moment (REQ-UI-SELECTION-PANEL, REQ-GW-TILE-SIZE) +// and stays that for as long as the selection lasts, a rectangle frozen in one moment +// having no meaningful distance to a tile size measured in another. class SelectionAnchorChangedEvent : public Event { public: - explicit SelectionAnchorChangedEvent(QRect rectPx) - : rectPx(rectPx) {} + SelectionAnchorChangedEvent(QRect rectPx, int selectionGapPx) + : rectPx(rectPx), selectionGapPx(selectionGapPx) {} const QRect rectPx; + const int selectionGapPx; }; diff --git a/src/test/FloatingPanelPlacementTest.cpp b/src/test/FloatingPanelPlacementTest.cpp index b30e176..2ee18c4 100644 --- a/src/test/FloatingPanelPlacementTest.cpp +++ b/src/test/FloatingPanelPlacementTest.cpp @@ -12,7 +12,11 @@ static QRect makeBand() return QRect(0, 0, 1000, 600); } -static const int kMarginPx = 8; +// The two distances the rules run on, deliberately different (REQ-UI-SELECTION-PANEL): +// the edge margin the panel keeps from the view and from the widgets it steps around, and +// the wider gap it keeps from the selection -- half a tile, so 20 px at a 40 px tile. +static const int kMarginPx = 8; +static const int kSelectionGapPx = 20; TEST_CASE("With nothing in the way a widget may use the whole band", "[layout]") { @@ -74,25 +78,34 @@ TEST_CASE("A widget filling the column leaves nothing", "[layout]") TEST_CASE("The panel stands to the right of the selection where it fits", "[layout]") { // REQ-UI-SELECTION-PANEL: right of the anchor is the first choice. - REQUIRE(chooseSide(makeBand(), QRect(100, 100, 60, 60), 300, kMarginPx) + REQUIRE(chooseSide(makeBand(), QRect(100, 100, 60, 60), 300, kSelectionGapPx) == PanelSide::Right); } TEST_CASE("The panel goes left when the right cannot hold it", "[layout]") { - // A selection near the right edge leaves 100 px there, not enough for a 300 px - // panel, and the left is wide open. - REQUIRE(chooseSide(makeBand(), QRect(880, 100, 20, 60), 300, kMarginPx) + // A selection near the right edge leaves 80 px there once the gap it keeps from the + // selection is taken off, not enough for a 300 px panel, and the left is wide open. + REQUIRE(chooseSide(makeBand(), QRect(880, 100, 20, 60), 300, kSelectionGapPx) == PanelSide::Left); } +TEST_CASE("The room a side offers is measured less the gap", "[layout]") +{ + // A 200 px panel beside a selection whose right edge leaves 214 px to the band's: it + // fits there on the margin alone, but not once the wider gap is taken off. + const QRect anchorRect(766, 100, 20, 60); + REQUIRE(chooseSide(makeBand(), anchorRect, 200, kMarginPx) == PanelSide::Right); + REQUIRE(chooseSide(makeBand(), anchorRect, 200, kSelectionGapPx) == PanelSide::Left); +} + TEST_CASE("Fitting on neither side, the panel takes the roomier one", "[layout]") { - // A bounding box spanning most of the view: 192 px free on the left, 92 on the + // A bounding box spanning most of the view: 180 px free on the left, 80 on the // right, and a 300 px panel fits in neither. It covers as little as it can. - REQUIRE(chooseSide(makeBand(), QRect(200, 100, 700, 200), 300, kMarginPx) + REQUIRE(chooseSide(makeBand(), QRect(200, 100, 700, 200), 300, kSelectionGapPx) == PanelSide::Left); - REQUIRE(chooseSide(makeBand(), QRect(100, 100, 700, 200), 300, kMarginPx) + REQUIRE(chooseSide(makeBand(), QRect(100, 100, 700, 200), 300, kSelectionGapPx) == PanelSide::Right); } @@ -102,17 +115,30 @@ TEST_CASE("Fitting on neither side, the panel takes the roomier one", "[layout]" TEST_CASE("The panel sits beside the anchor with its top edges aligned", "[layout]") { - // REQ-UI-SELECTION-PANEL: separated by the margin, growing away from the selection, - // top edge on the anchor's top edge. + // REQ-UI-SELECTION-PANEL: separated by the gap it keeps from the selection, growing + // away from it, top edge on the anchor's top edge. const QRect placed = placeBesideAnchor(makeBand(), QRect(100, 120, 60, 60), PanelSide::Right, QSize(300, 200), {}, - kMarginPx); - REQUIRE(placed == QRect(168, 120, 300, 200)); + kSelectionGapPx, kMarginPx); + REQUIRE(placed == QRect(180, 120, 300, 200)); - const QRect placedLeft = placeBesideAnchor(makeBand(), QRect(500, 120, 60, 60), + const QRect placedLeft = placeBesideAnchor(makeBand(), QRect(520, 120, 60, 60), PanelSide::Left, QSize(300, 200), {}, - kMarginPx); - REQUIRE(placedLeft == QRect(192, 120, 300, 200)); + kSelectionGapPx, kMarginPx); + REQUIRE(placedLeft == QRect(200, 120, 300, 200)); +} + +TEST_CASE("The gap separates the panel horizontally only", "[layout]") +{ + // REQ-UI-SELECTION-PANEL: widening the gap moves the panel further from the selection + // sideways and nowhere else -- its top stays level with the top of what it describes. + const QRect anchorRect(100, 120, 60, 60); + REQUIRE(placeBesideAnchor(makeBand(), anchorRect, PanelSide::Right, QSize(300, 200), + {}, 0, kMarginPx) + == QRect(160, 120, 300, 200)); + REQUIRE(placeBesideAnchor(makeBand(), anchorRect, PanelSide::Right, QSize(300, 200), + {}, kSelectionGapPx, kMarginPx) + == QRect(180, 120, 300, 200)); } TEST_CASE("A panel that would hang below the view is lifted", "[layout]") @@ -121,19 +147,20 @@ TEST_CASE("A panel that would hang below the view is lifted", "[layout]") // the band, so it rises until it fits rather than overrunning it. const QRect placed = placeBesideAnchor(makeBand(), QRect(100, 500, 60, 60), PanelSide::Right, QSize(300, 200), {}, - kMarginPx); - REQUIRE(placed == QRect(168, 400, 300, 200)); + kSelectionGapPx, kMarginPx); + REQUIRE(placed == QRect(180, 400, 300, 200)); } TEST_CASE("A panel standing over another widget rises above it", "[layout]") { // The controls panel in the bottom-left is in the way of a panel placed to the left - // of a selection: it clears the top of it by the margin (REQ-UI-CONTROLS-PANEL). + // of a selection: it clears the top of it by the edge margin, the gap from the + // selection having settled its left edge (REQ-UI-CONTROLS-PANEL). const std::vector occupied = { QRect(0, 300, 260, 300) }; const QRect placed = placeBesideAnchor(makeBand(), QRect(500, 250, 60, 60), PanelSide::Left, QSize(300, 200), occupied, - kMarginPx); - REQUIRE(placed == QRect(192, 92, 300, 200)); + kSelectionGapPx, kMarginPx); + REQUIRE(placed == QRect(180, 92, 300, 200)); } TEST_CASE("A panel taller than the space left is capped", "[layout]") @@ -141,8 +168,8 @@ TEST_CASE("A panel taller than the space left is capped", "[layout]") // Capping is the caller's cue to scroll: it asked for 700 and got what there was. const QRect placed = placeBesideAnchor(makeBand(), QRect(100, 100, 60, 60), PanelSide::Right, QSize(300, 700), {}, - kMarginPx); - REQUIRE(placed == QRect(168, 0, 300, 600)); + kSelectionGapPx, kMarginPx); + REQUIRE(placed == QRect(180, 0, 300, 600)); } TEST_CASE("A panel that fits on neither side is pushed inside the view", "[layout]") @@ -151,7 +178,7 @@ TEST_CASE("A panel that fits on neither side is pushed inside the view", "[layou // stands as far from the anchor as the band allows, not off the edge of it. const QRect placed = placeBesideAnchor(makeBand(), QRect(100, 100, 700, 200), PanelSide::Right, QSize(300, 200), {}, - kMarginPx); + kSelectionGapPx, kMarginPx); REQUIRE(placed == QRect(700, 100, 300, 200)); } diff --git a/src/ui/GameWorldView.cpp b/src/ui/GameWorldView.cpp index 6b3bead..4be69da 100644 --- a/src/ui/GameWorldView.cpp +++ b/src/ui/GameWorldView.cpp @@ -1468,8 +1468,13 @@ void GameWorldView::publishSelectionAnchor(SelectionMode mode, { return; } + // The gap the panel keeps from that rectangle is half a tile (REQ-UI-SELECTION-PANEL), + // and this is where the tile size is known. It is sampled in the same moment as the + // rectangle and travels with it, so both describe the view as it stood when the + // selection started. + const int selectionGapPx = qRound(getCoordinates().getTilePx() / 2.0f); EventManager::getInstance()->sendEventImmediately( - std::make_shared(anchorRect)); + std::make_shared(anchorRect, selectionGapPx)); } void GameWorldView::mouseMoveEvent(QMouseEvent* event) diff --git a/src/ui/SelectionPanel.cpp b/src/ui/SelectionPanel.cpp index 9a51a86..9a4d591 100644 --- a/src/ui/SelectionPanel.cpp +++ b/src/ui/SelectionPanel.cpp @@ -17,8 +17,11 @@ namespace { -// Distance kept between the panel and the edges of the game world view, and between it -// and the widgets it steps around (REQ-UI-SELECTION-PANEL). +// The edge margin: the distance kept between the panel and the edges of the game world +// view, and between it and the widgets it steps around (REQ-UI-SELECTION-PANEL). The gap +// the panel keeps from the selection itself is the wider of the two and is not this: being +// half a tile, it is measured where the tile size is known and arrives with the anchor +// rectangle (SelectionAnchorChangedEvent). const int kMarginPx = 8; // Upper bound on the card width. The panel is content-sized, but several of the cards' @@ -170,12 +173,14 @@ void SelectionPanel::handleEvent(std::shared_ptr ev void SelectionPanel::handleEvent( std::shared_ptr event) { - // A new selection is starting. Both the anchor and the side are settled against it - // and then left alone for as long as it lasts (REQ-UI-SELECTION-PANEL); the side is - // only reset here, being resolved on the next placement once the card's width is - // known. The rect arrives in the world view's coordinates and is translated when the - // panel is placed, the two widgets being siblings in the same parent. - m_anchorRect = event->rectPx; + // A new selection is starting. The anchor, the gap kept from it, and the side are all + // settled against it and then left alone for as long as it lasts + // (REQ-UI-SELECTION-PANEL); the side is only reset here, being resolved on the next + // placement once the card's width is known. The rect arrives in the world view's + // coordinates and is translated when the panel is placed, the two widgets being + // siblings in the same parent. + m_anchorRect = event->rectPx; + m_selectionGapPx = event->selectionGapPx; m_side.reset(); // A position the player dragged the panel to belongs to the selection it was set in. // A new selection places the panel anew against its own anchor @@ -354,7 +359,7 @@ void SelectionPanel::placeIn(const QRect& viewRect, const std::vector& oc wantedSize, occupiedRects, kMarginPx); } return placeBesideAnchor(band, anchorRect, *m_side, wantedSize, occupiedRects, - kMarginPx); + m_selectionGapPx, kMarginPx); }; // Run twice. Parts of a card report an unstyled size until the style has actually @@ -376,7 +381,7 @@ void SelectionPanel::placeIn(const QRect& viewRect, const std::vector& oc if (!m_side.has_value()) { m_side = chooseSide(band, anchorRect, contentWidthPx + 2 * borderPx, - kMarginPx); + m_selectionGapPx); } // How much height there is depends on where the panel ends up standing: of the diff --git a/src/ui/SelectionPanel.h b/src/ui/SelectionPanel.h index 3223640..4ce01f2 100644 --- a/src/ui/SelectionPanel.h +++ b/src/ui/SelectionPanel.h @@ -103,14 +103,16 @@ private: SelectionContent* m_content = nullptr; // Where the current selection was on the screen when it started, in the game world - // view's coordinates, and which side of it the panel took. Both are frozen for as - // long as the selection lasts: the anchor because the panel does not chase a - // scrolling view or a moving ship, the side because a card that grows must not flip - // the panel across the object (REQ-UI-SELECTION-PANEL). The side is resolved on the - // first placement after a new anchor, being the first point at which the panel's - // width is known. Dragging the panel supersedes the pair for the rest of the + // view's coordinates, the gap the panel keeps from it, and which side of it the panel + // took. All three are frozen for as long as the selection lasts: the anchor because + // the panel does not chase a scrolling view or a moving ship, the gap because it is + // measured against that frozen rectangle, the side because a card that grows must not + // flip the panel across the object (REQ-UI-SELECTION-PANEL). The side is resolved on + // the first placement after a new anchor, being the first point at which the panel's + // width is known. Dragging the panel supersedes all three for the rest of the // selection (REQ-UI-SELECTION-PANEL-DRAG). QRect m_anchorRect; + int m_selectionGapPx = 0; std::optional m_side; // Where the player dragged the panel, in the game world view's coordinates, and the