From f2d0eb18ed4bb02691f1803f03bfe9fe68122008 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Wed, 15 Jul 2026 14:18:31 -0700 Subject: [PATCH 1/2] feat(editor): 2D Editor pane, status instructions, generated shortcut map (#213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes out P1 on top of the merged p1-s1/s2/s3. The vertical profile moves into a new tabbed "2D Editor" pane, ready for the cross-section and Signal Phase editors (GW-4 step 4). The host is thin on purpose: a page keeps its own selection subscription and its own commands, so ProfilePanel moved without a line changing and test_profile_panel.cpp passes unmodified. The host only raises a relevant tab, and never yanks a tab the user is already using. The dock is dock.editor2d, NOT dock.profile: restoreState matches saved geometry by objectName, so reusing the name would let a returning user's stale entry capture a differently-shaped pane. An unknown name is ignored instead, and the default placement wins. Every binding now comes from one table (app/shortcut_registry.cpp): Actions binds from it via setShortcuts, and docs/user-guide/shortcuts.md is rendered from it, with a gtest gate that fails when the two drift — before this, key letters were hand-copied into tooltips and nothing noticed when they went stale. The page cannot be rendered from QKeySequence::StandardKey: keyBindings() returns a DIFFERENT list per platform (Redo is three sequences on Windows, one on macOS) and some are empty (Quit has none on macOS, where the platform menu owns it), so a committed page would pass the gate only on the machine that generated it. Standard rows therefore carry the one spelling the docs commit to, and the app still binds natively. The status bar gains a persistent instruction line fed from a new Tool::instruction(). Rendering the window showed why that needed more than a new label: every tool ALSO emitted the same guidance transiently from activate(), so the bar carried two wordings of one sentence, the transient one clipped. instruction() is now the single source — it feeds both the status line and the viewport corner hint (issue #103) — and the transient channel keeps only results and state-dependent guidance. The label is permanent, so showMessage can neither hide nor overlap it. Refs #213 --- docs/user-guide/index.md | 1 + docs/user-guide/shortcuts.md | 63 +++++++ editor/CMakeLists.txt | 2 + editor/src/app/actions.cpp | 69 ++++---- editor/src/app/main_window.cpp | 53 ++++-- editor/src/app/main_window.hpp | 7 +- editor/src/app/shortcut_registry.cpp | 201 ++++++++++++++++++++++ editor/src/app/shortcut_registry.hpp | 96 +++++++++++ editor/src/panels/editor2d_host.cpp | 63 +++++++ editor/src/panels/editor2d_host.hpp | 93 ++++++++++ editor/src/tools/create_junction_tool.cpp | 7 +- editor/src/tools/create_junction_tool.hpp | 2 + editor/src/tools/create_road_tool.cpp | 10 +- editor/src/tools/create_road_tool.hpp | 2 + editor/src/tools/delete_tool.cpp | 8 +- editor/src/tools/delete_tool.hpp | 2 + editor/src/tools/edit_nodes_tool.cpp | 10 +- editor/src/tools/edit_nodes_tool.hpp | 2 + editor/src/tools/elevation_tool.cpp | 10 +- editor/src/tools/elevation_tool.hpp | 2 + editor/src/tools/lane_profile_tool.cpp | 9 +- editor/src/tools/lane_profile_tool.hpp | 2 + editor/src/tools/select_tool.cpp | 23 ++- editor/src/tools/select_tool.hpp | 2 + editor/src/tools/split_tool.cpp | 8 +- editor/src/tools/split_tool.hpp | 2 + editor/src/tools/tool.hpp | 11 ++ editor/tests/CMakeLists.txt | 7 +- editor/tests/test_editor2d_host.cpp | 175 +++++++++++++++++++ editor/tests/test_shortcut_registry.cpp | 102 +++++++++++ 30 files changed, 963 insertions(+), 81 deletions(-) create mode 100644 docs/user-guide/shortcuts.md create mode 100644 editor/src/app/shortcut_registry.cpp create mode 100644 editor/src/app/shortcut_registry.hpp create mode 100644 editor/src/panels/editor2d_host.cpp create mode 100644 editor/src/panels/editor2d_host.hpp create mode 100644 editor/tests/test_editor2d_host.cpp create mode 100644 editor/tests/test_shortcut_registry.cpp diff --git a/docs/user-guide/index.md b/docs/user-guide/index.md index 549e83b..cd647e4 100644 --- a/docs/user-guide/index.md +++ b/docs/user-guide/index.md @@ -42,6 +42,7 @@ not undoable. | Tool | Task | |---|---| | [Camera & navigation](camera-navigation.md) | Orbit, zoom, and pan the viewport around its pivot | +| [Keyboard shortcuts](shortcuts.md) | Every binding, generated from the editor's own table | | [Editing attributes](attributes.md) | Scrub numeric fields by dragging their name; drop assets into slots | | [Library](library.md) | Drag ready-made roads, intersections, and props into the scene | | [Create Road](create-road.md) | Lay a new clothoid road through waypoints with a lane template | diff --git a/docs/user-guide/shortcuts.md b/docs/user-guide/shortcuts.md new file mode 100644 index 0000000..4a305fe --- /dev/null +++ b/docs/user-guide/shortcuts.md @@ -0,0 +1,63 @@ +# Keyboard shortcuts + +*Every keyboard binding in the RoadMaker editor, in one place.* + +Nothing here is only reachable by keyboard: each of these is also a menu or +toolbar entry, and the keys are accelerators for what you can already point at. +Mouse and trackpad navigation — orbiting, panning, zooming, and the modifier +chords — live in [Camera & navigation](camera-navigation.md). + +Keys are written in the cross-platform spelling. Where a platform differs, the +**Notes** column says so: `Ctrl` is `⌘ Command` on macOS for the File and Edit +conventions below. + +> The tables below are **generated** from the editor's shortcut table +> (`editor/src/app/shortcut_registry.cpp`), which is also what the editor binds +> from. A CI test compares this page against that table, so a binding cannot +> change without this page changing with it. Edit the table, not the tables +> below. + +## File + +| Action | Shortcut | Notes | +|---|---|---| +| New scene | `Ctrl+N` | ⌘N on macOS | +| Open an OpenDRIVE (.xodr) file | `Ctrl+O` | ⌘O on macOS | +| Save | `Ctrl+S` | ⌘S on macOS | +| Save as… | `Ctrl+Shift+S` | ⇧⌘S on macOS | +| Quit | `Ctrl+Q` | ⌘Q on macOS, where the platform menu owns it | + +## Edit + +| Action | Shortcut | Notes | +|---|---|---| +| Undo | `Ctrl+Z` | ⌘Z on macOS | +| Redo | `Ctrl+Shift+Z` | Ctrl+Y also works on Windows; ⇧⌘Z on macOS | + +## Tools + +| Action | Shortcut | Notes | +|---|---|---| +| Select/Move tool | `Q` | | +| Move tool | `M` | | +| Create Road tool | `C` | | +| Edit Nodes tool | `N` | | +| Lane Profile tool | `L` | | +| Elevation tool | `E` | | +| Create Junction tool | `J` | | +| Split tool | `K` | | +| Delete tool | `X` | | + +## View + +| Action | Shortcut | Notes | +|---|---|---| +| Frame the selection (the whole scene when nothing is selected) | `F` | | +| Frame on the point under the cursor (keeps the zoom) | `V` | | +| Perspective projection | `P` | | +| Orthographic projection | `O` | | +| Look from the north | `Num+8` or `8` | Numpad; the top-row digit is the alternate | +| Look from the south | `Num+2` or `2` | Numpad; the top-row digit is the alternate | +| Look from the west | `Num+4` or `4` | Numpad; the top-row digit is the alternate | +| Look from the east | `Num+6` or `6` | Numpad; the top-row digit is the alternate | +| Top-down view, north up | `Num+5` or `5` | Numpad; the top-row digit is the alternate | diff --git a/editor/CMakeLists.txt b/editor/CMakeLists.txt index cafa12b..56f2ad9 100644 --- a/editor/CMakeLists.txt +++ b/editor/CMakeLists.txt @@ -33,6 +33,7 @@ qt_standard_project_setup() # editor tests can link the same objects and run headless (offscreen QPA). add_library(roadmaker_editor_lib STATIC src/app/actions.cpp + src/app/shortcut_registry.cpp src/app/context_menu.cpp src/app/crash_handler.cpp src/app/icons.cpp @@ -53,6 +54,7 @@ add_library(roadmaker_editor_lib STATIC src/document/scene_tree_model.cpp src/document/selection_model.cpp src/panels/diagnostics_panel.cpp + src/panels/editor2d_host.cpp src/panels/library_panel.cpp src/panels/profile_panel.cpp src/panels/properties_panel.cpp diff --git a/editor/src/app/actions.cpp b/editor/src/app/actions.cpp index ac9fb4c..1ed8a2e 100644 --- a/editor/src/app/actions.cpp +++ b/editor/src/app/actions.cpp @@ -1,6 +1,7 @@ #include "app/actions.hpp" #include "app/icons.hpp" +#include "app/shortcut_registry.hpp" namespace roadmaker::editor { @@ -8,22 +9,22 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { // iconText carries the short label the labeled toolbar renders under the // icon (ToolButtonTextUnderIcon); the menu keeps the full text. new_file = new QAction(tr("&New"), this); - new_file->setShortcut(QKeySequence::New); + new_file->setShortcuts(shortcuts::sequences(shortcuts::Id::NewScene)); new_file->setIconText(tr("New")); new_file->setToolTip(tr("New scene — start an empty road network")); open = new QAction(tr("&Open…"), this); - open->setShortcut(QKeySequence::Open); + open->setShortcuts(shortcuts::sequences(shortcuts::Id::Open)); open->setIconText(tr("Open")); open->setToolTip(tr("Open an OpenDRIVE (.xodr) file")); save = new QAction(tr("&Save"), this); - save->setShortcut(QKeySequence::Save); + save->setShortcuts(shortcuts::sequences(shortcuts::Id::Save)); save->setIconText(tr("Save")); save->setToolTip(tr("Save the scene as OpenDRIVE")); save_as = new QAction(tr("Save &As…"), this); - save_as->setShortcut(QKeySequence::SaveAs); + save_as->setShortcuts(shortcuts::sequences(shortcuts::Id::SaveAs)); export_glb = new QAction(tr("&Export glTF…"), this); export_glb->setEnabled(false); // enabled once a file is loaded @@ -36,20 +37,20 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { #endif quit = new QAction(tr("&Quit"), this); - quit->setShortcut(QKeySequence::Quit); + quit->setShortcuts(shortcuts::sequences(shortcuts::Id::Quit)); quit->setMenuRole(QAction::QuitRole); undo = undo_stack.createUndoAction(this, tr("&Undo")); - undo->setShortcut(QKeySequence::Undo); + undo->setShortcuts(shortcuts::sequences(shortcuts::Id::Undo)); redo = undo_stack.createRedoAction(this, tr("&Redo")); - redo->setShortcut(QKeySequence::Redo); + redo->setShortcuts(shortcuts::sequences(shortcuts::Id::Redo)); tool_group = new QActionGroup(this); tool_select = new QAction(tr("&Select/Move"), this); tool_select->setCheckable(true); tool_select->setChecked(true); // the default tool // Q, not V: V is frame-on-cursor (GW-1 step 10). Rebound in p1-s2. - tool_select->setShortcut(Qt::Key_Q); + tool_select->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolSelect)); tool_select->setIconText(tr("Select")); tool_select->setToolTip(tr("Select/Move — click picks, drag spans a rubber band, " "drag a node handle moves it (Q)")); @@ -57,7 +58,7 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { tool_move = new QAction(tr("&Move"), this); tool_move->setCheckable(true); - tool_move->setShortcut(Qt::Key_M); + tool_move->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolMove)); tool_move->setIconText(tr("Move")); tool_move->setToolTip(tr("Move — hover shows the 4-arrow cursor; drag a road or a prop to " "move it, or click to select and transform it (M)")); @@ -65,7 +66,7 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { tool_create_road = new QAction(tr("&Create Road"), this); tool_create_road->setCheckable(true); - tool_create_road->setShortcut(Qt::Key_C); + tool_create_road->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolCreateRoad)); tool_create_road->setIconText(tr("Road")); tool_create_road->setToolTip(tr("Create Road — click places waypoints, Enter or double-click " "creates the road, Esc cancels (C)")); @@ -73,7 +74,7 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { tool_edit_nodes = new QAction(tr("Edit &Nodes"), this); tool_edit_nodes->setCheckable(true); - tool_edit_nodes->setShortcut(Qt::Key_N); + tool_edit_nodes->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolEditNodes)); tool_edit_nodes->setIconText(tr("Nodes")); tool_edit_nodes->setToolTip(tr("Edit Nodes — drag a node to move it, click a midpoint " "marker to insert, Delete removes the active node (N)")); @@ -81,7 +82,7 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { tool_lane_profile = new QAction(tr("&Lane Profile"), this); tool_lane_profile->setCheckable(true); - tool_lane_profile->setShortcut(Qt::Key_L); + tool_lane_profile->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolLaneProfile)); tool_lane_profile->setIconText(tr("Lanes")); tool_lane_profile->setToolTip(tr("Lane Profile — click a lane, then edit its type, width, and " "road mark in the Properties panel (L)")); @@ -89,7 +90,7 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { tool_elevation = new QAction(tr("&Elevation"), this); tool_elevation->setCheckable(true); - tool_elevation->setShortcut(Qt::Key_E); + tool_elevation->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolElevation)); tool_elevation->setIconText(tr("Elevation")); tool_elevation->setToolTip(tr("Elevation — click a road node, then set its height in the " "Properties panel; the grade re-fits smoothly (E)")); @@ -97,7 +98,7 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { tool_create_junction = new QAction(tr("Create &Junction"), this); tool_create_junction->setCheckable(true); - tool_create_junction->setShortcut(Qt::Key_J); + tool_create_junction->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolCreateJunction)); tool_create_junction->setIconText(tr("Junction")); tool_create_junction->setToolTip(tr("Create Junction — click 2+ road ends, or 1 end + a road " "body to tee into it; Enter generates, Esc cancels (J)")); @@ -105,14 +106,14 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { tool_split = new QAction(tr("&Split"), this); tool_split->setCheckable(true); - tool_split->setShortcut(Qt::Key_K); + tool_split->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolSplit)); tool_split->setIconText(tr("Split")); tool_split->setToolTip(tr("Split — click a road to cut it in two at the marker (K)")); tool_group->addAction(tool_split); tool_delete = new QAction(tr("&Delete"), this); tool_delete->setCheckable(true); - tool_delete->setShortcut(Qt::Key_X); + tool_delete->setShortcuts(shortcuts::sequences(shortcuts::Id::ToolDelete)); tool_delete->setIconText(tr("Delete")); tool_delete->setToolTip(tr("Delete — click a road to delete it, undo restores (X)")); tool_group->addAction(tool_delete); @@ -144,12 +145,12 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { add_from_library->setToolTip(tr("Open the Library panel to drag in roads, intersections, " "and props")); frame_selection = new QAction(tr("&Frame Selection"), this); - frame_selection->setShortcut(Qt::Key_F); + frame_selection->setShortcuts(shortcuts::sequences(shortcuts::Id::FrameSelection)); frame_selection->setIconText(tr("Frame")); frame_selection->setToolTip(tr("Frame the selection — the whole scene when " "nothing is selected (F)")); frame_cursor = new QAction(tr("Frame Under &Cursor"), this); - frame_cursor->setShortcut(Qt::Key_V); + frame_cursor->setShortcuts(shortcuts::sequences(shortcuts::Id::FrameCursor)); frame_cursor->setIconText(tr("Frame Cursor")); frame_cursor->setToolTip(tr("Move the pivot to the point under the cursor, keeping the " "zoom (V)")); @@ -157,12 +158,12 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { // Projection (GW-1 step 11). Exclusive: the view is one or the other. projection_group = new QActionGroup(this); view_perspective = new QAction(tr("&Perspective"), this); - view_perspective->setShortcut(Qt::Key_P); + view_perspective->setShortcuts(shortcuts::sequences(shortcuts::Id::ViewPerspective)); view_perspective->setCheckable(true); view_perspective->setChecked(true); // the startup projection view_perspective->setToolTip(tr("Perspective projection (P)")); view_orthographic = new QAction(tr("&Orthographic"), this); - view_orthographic->setShortcut(Qt::Key_O); + view_orthographic->setShortcuts(shortcuts::sequences(shortcuts::Id::ViewOrthographic)); view_orthographic->setCheckable(true); view_orthographic->setToolTip(tr("Orthographic projection — parallel, no foreshortening (O)")); projection_group->addAction(view_perspective); @@ -170,33 +171,27 @@ Actions::Actions(QUndoStack& undo_stack, QObject* parent) : QObject(parent) { // Cardinal views (GW-1 steps 12-13). The numpad digits are the primary // binding; the top-row digits are the alternate for keyboards without one. - const auto make_cardinal = - [this](const QString& text, const QString& tip, Qt::Key numpad, Qt::Key digit) { - auto* action = new QAction(text, this); - action->setShortcuts({QKeySequence(Qt::KeypadModifier | numpad), QKeySequence(digit)}); - action->setToolTip(tip); - return action; - }; + const auto make_cardinal = [this](const QString& text, const QString& tip, shortcuts::Id id) { + auto* action = new QAction(text, this); + action->setShortcuts(shortcuts::sequences(id)); // numpad + the top-row alternate + action->setToolTip(tip); + return action; + }; view_north = make_cardinal(tr("Look from &North"), tr("Look from the north, southward (numpad 8, or 8)"), - Qt::Key_8, - Qt::Key_8); + shortcuts::Id::ViewNorth); view_south = make_cardinal(tr("Look from &South"), tr("Look from the south, northward (numpad 2, or 2)"), - Qt::Key_2, - Qt::Key_2); + shortcuts::Id::ViewSouth); view_west = make_cardinal(tr("Look from &West"), tr("Look from the west, eastward (numpad 4, or 4)"), - Qt::Key_4, - Qt::Key_4); + shortcuts::Id::ViewWest); view_east = make_cardinal(tr("Look from &East"), tr("Look from the east, westward (numpad 6, or 6)"), - Qt::Key_6, - Qt::Key_6); + shortcuts::Id::ViewEast); view_top = make_cardinal(tr("&Top-Down"), tr("Plan view from directly above, north up (numpad 5, or 5)"), - Qt::Key_5, - Qt::Key_5); + shortcuts::Id::ViewTop); merge_roads = new QAction(tr("&Merge Roads"), this); merge_roads->setIconText(tr("Merge")); merge_roads->setEnabled(false); // enabled only for a mergeable 2-road selection diff --git a/editor/src/app/main_window.cpp b/editor/src/app/main_window.cpp index e187404..7925465 100644 --- a/editor/src/app/main_window.cpp +++ b/editor/src/app/main_window.cpp @@ -35,8 +35,8 @@ #include "document/library_drop.hpp" #include "document/library_manifest.hpp" #include "panels/diagnostics_panel.hpp" +#include "panels/editor2d_host.hpp" #include "panels/library_panel.hpp" -#include "panels/profile_panel.hpp" #include "panels/properties_panel.hpp" #include "panels/scene_tree_panel.hpp" #include "tools/create_junction_tool.hpp" @@ -284,11 +284,12 @@ MainWindow::MainWindow(QWidget* parent, bool restore_saved_layout) tool_manager_.register_tool(ToolId::Elevation, std::move(elevation_tool)); connect(actions_->tool_elevation, &QAction::triggered, this, [this] { tool_manager_.set_active(ToolId::Elevation); - // Surface the Profile dock (vertical-profile handles + the overpass Cross + // Surface the 2D Editor pane (vertical-profile handles + the overpass Cross // Over/Under controls) when the elevation workflow starts — otherwise it // hides behind a View-menu toggle (discoverability rule, product-parity.md). - profile_dock_->show(); - profile_dock_->raise(); + editor2d_dock_->show(); + editor2d_dock_->raise(); + editor2d_host_->raise_relevant_page(); }); // The Properties panel edits the node the Elevation tool has made active. properties_panel_->set_elevation_tool(elevation_tool_); @@ -419,11 +420,22 @@ void MainWindow::build_docks() { properties_dock_->widget()->setMinimumWidth(300); addDockWidget(Qt::RightDockWidgetArea, properties_dock_); - profile_dock_ = new QDockWidget(tr("Profile"), this); - profile_dock_->setObjectName(QStringLiteral("dock.profile")); - profile_dock_->setWidget(new ProfilePanel(document_, selection_, profile_dock_)); - addDockWidget(Qt::BottomDockWidgetArea, profile_dock_); - profile_dock_->hide(); // opt-in via the View menu — vertical design is occasional + // The 2D Editor pane hosts the flat, per-entity editors. The vertical + // profile is the first page; the cross-section and Signal Phase Editor plug + // in later (GW-4 step 4, p4-s5). + // + // NOT "dock.profile": restoreState() matches saved geometry by objectName, so + // reusing the old name would drop this dock into the position a stale layout + // remembers for a different widget. The new name is simply unknown to old + // settings, which restoreState ignores — the default placement below wins. + editor2d_dock_ = new QDockWidget(tr("2D Editor"), this); + editor2d_dock_->setObjectName(QStringLiteral("dock.editor2d")); + editor2d_host_ = new Editor2DHost(selection_, editor2d_dock_); + editor2d_host_->register_page( + std::make_unique(document_, selection_, editor2d_host_)); + editor2d_dock_->setWidget(editor2d_host_); + addDockWidget(Qt::BottomDockWidgetArea, editor2d_dock_); + editor2d_dock_->hide(); // opt-in via the View menu — 2D editing is occasional diagnostics_dock_ = new QDockWidget(tr("Diagnostics"), this); diagnostics_dock_->setObjectName(QStringLiteral("dock.diagnostics")); @@ -471,7 +483,7 @@ void MainWindow::build_menus() { view_menu->addAction(library_dock_->toggleViewAction()); view_menu->addAction(properties_dock_->toggleViewAction()); view_menu->addAction(diagnostics_dock_->toggleViewAction()); - view_menu->addAction(profile_dock_->toggleViewAction()); + view_menu->addAction(editor2d_dock_->toggleViewAction()); view_menu->addSeparator(); auto* textured_action = new QAction(tr("&Textured Rendering"), this); textured_action->setCheckable(true); @@ -598,6 +610,27 @@ void MainWindow::update_tool_options() { void MainWindow::build_status_bar() { statusBar()->addWidget(status_hover_, 1); + // PERMANENT, not a normal widget: showMessage() hides the normal indications + // while a transient message is up, and the instruction must survive that — + // it answers "what can I do with this tool", which stays true while results + // and refusals come and go. Being permanent also lays it out clear of the + // message, so the two can never paint over each other. + status_instruction_ = new QLabel(this); + status_instruction_->setObjectName(QStringLiteral("status_instruction")); + statusBar()->addPermanentWidget(status_instruction_); + // Follow the active tool: its instruction() is the ONE source for "what does + // this tool do", shown both here and as the viewport corner hint (issue #103 + // — during an interaction the user's eyes are on the viewport). Tools used to + // emit the same sentence transiently on activate(); they no longer do, so the + // transient channel carries only results and state-dependent guidance. + const auto show_instruction = [this] { + const Tool* tool = tool_manager_.active(); + const QString text = tool == nullptr ? QString() : tool->instruction(); + status_instruction_->setText(text); + viewport_->set_hint(text); + }; + connect(&tool_manager_, &ToolManager::active_changed, this, show_instruction); + show_instruction(); // the startup tool statusBar()->addPermanentWidget(status_entities_); auto* version_label = new QLabel(tr("kernel %1") diff --git a/editor/src/app/main_window.hpp b/editor/src/app/main_window.hpp index 59b68d0..49b050e 100644 --- a/editor/src/app/main_window.hpp +++ b/editor/src/app/main_window.hpp @@ -161,9 +161,14 @@ class MainWindow : public QMainWindow { QDockWidget* library_dock_; QDockWidget* properties_dock_; QDockWidget* diagnostics_dock_; - QDockWidget* profile_dock_; + QDockWidget* editor2d_dock_; + class Editor2DHost* editor2d_host_ = nullptr; QMenu* recent_menu_ = nullptr; QLabel* status_hover_; + /// Persistent per-tool instruction line (what click/drag/modifiers do right + /// now). Distinct from the transient status_message channel, which keeps its + /// 5 s showMessage lane for state-dependent guidance and results. + QLabel* status_instruction_ = nullptr; QLabel* status_entities_; /// Main toolbar — kept so the guided tour can locate an action's button to diff --git a/editor/src/app/shortcut_registry.cpp b/editor/src/app/shortcut_registry.cpp new file mode 100644 index 0000000..a22dcc3 --- /dev/null +++ b/editor/src/app/shortcut_registry.cpp @@ -0,0 +1,201 @@ +#include "app/shortcut_registry.hpp" + +#include +#include +#include + +namespace roadmaker::editor::shortcuts { + +namespace { + +// The map. Order here is the order on the page. +constexpr std::array kTable{ + Entry{.id = Id::NewScene, + .category = "File", + .description = "New scene", + .standard = QKeySequence::New, + .documented = "Ctrl+N", + .note = "\u2318N on macOS"}, + Entry{.id = Id::Open, + .category = "File", + .description = "Open an OpenDRIVE (.xodr) file", + .standard = QKeySequence::Open, + .documented = "Ctrl+O", + .note = "\u2318O on macOS"}, + Entry{.id = Id::Save, + .category = "File", + .description = "Save", + .standard = QKeySequence::Save, + .documented = "Ctrl+S", + .note = "\u2318S on macOS"}, + Entry{.id = Id::SaveAs, + .category = "File", + .description = "Save as…", + .standard = QKeySequence::SaveAs, + .documented = "Ctrl+Shift+S", + .note = "\u21e7\u2318S on macOS"}, + Entry{.id = Id::Quit, + .category = "File", + .description = "Quit", + .standard = QKeySequence::Quit, + .documented = "Ctrl+Q", + .note = "\u2318Q on macOS, where the platform menu owns it"}, + + Entry{.id = Id::Undo, + .category = "Edit", + .description = "Undo", + .standard = QKeySequence::Undo, + .documented = "Ctrl+Z", + .note = "\u2318Z on macOS"}, + Entry{.id = Id::Redo, + .category = "Edit", + .description = "Redo", + .standard = QKeySequence::Redo, + .documented = "Ctrl+Shift+Z", + .note = "Ctrl+Y also works on Windows; \u21e7\u2318Z on macOS"}, + + Entry{.id = Id::ToolSelect, + .category = "Tools", + .description = "Select/Move tool", + .primary = Qt::Key_Q}, + Entry{ + .id = Id::ToolMove, .category = "Tools", .description = "Move tool", .primary = Qt::Key_M}, + Entry{.id = Id::ToolCreateRoad, + .category = "Tools", + .description = "Create Road tool", + .primary = Qt::Key_C}, + Entry{.id = Id::ToolEditNodes, + .category = "Tools", + .description = "Edit Nodes tool", + .primary = Qt::Key_N}, + Entry{.id = Id::ToolLaneProfile, + .category = "Tools", + .description = "Lane Profile tool", + .primary = Qt::Key_L}, + Entry{.id = Id::ToolElevation, + .category = "Tools", + .description = "Elevation tool", + .primary = Qt::Key_E}, + Entry{.id = Id::ToolCreateJunction, + .category = "Tools", + .description = "Create Junction tool", + .primary = Qt::Key_J}, + Entry{.id = Id::ToolSplit, + .category = "Tools", + .description = "Split tool", + .primary = Qt::Key_K}, + Entry{.id = Id::ToolDelete, + .category = "Tools", + .description = "Delete tool", + .primary = Qt::Key_X}, + + Entry{.id = Id::FrameSelection, + .category = "View", + .description = "Frame the selection (the whole scene when nothing is selected)", + .primary = Qt::Key_F}, + Entry{.id = Id::FrameCursor, + .category = "View", + .description = "Frame on the point under the cursor (keeps the zoom)", + .primary = Qt::Key_V}, + Entry{.id = Id::ViewPerspective, + .category = "View", + .description = "Perspective projection", + .primary = Qt::Key_P}, + Entry{.id = Id::ViewOrthographic, + .category = "View", + .description = "Orthographic projection", + .primary = Qt::Key_O}, + Entry{.id = Id::ViewNorth, + .category = "View", + .description = "Look from the north", + .primary = Qt::KeypadModifier | Qt::Key_8, + .alternate = Qt::Key_8, + .note = "Numpad; the top-row digit is the alternate"}, + Entry{.id = Id::ViewSouth, + .category = "View", + .description = "Look from the south", + .primary = Qt::KeypadModifier | Qt::Key_2, + .alternate = Qt::Key_2, + .note = "Numpad; the top-row digit is the alternate"}, + Entry{.id = Id::ViewWest, + .category = "View", + .description = "Look from the west", + .primary = Qt::KeypadModifier | Qt::Key_4, + .alternate = Qt::Key_4, + .note = "Numpad; the top-row digit is the alternate"}, + Entry{.id = Id::ViewEast, + .category = "View", + .description = "Look from the east", + .primary = Qt::KeypadModifier | Qt::Key_6, + .alternate = Qt::Key_6, + .note = "Numpad; the top-row digit is the alternate"}, + Entry{.id = Id::ViewTop, + .category = "View", + .description = "Top-down view, north up", + .primary = Qt::KeypadModifier | Qt::Key_5, + .alternate = Qt::Key_5, + .note = "Numpad; the top-row digit is the alternate"}, +}; + +/// PortableText so the page is platform-stable; the StandardKey rows still +/// render their platform's convention, which is the point of using them. +QString render(const QKeySequence& sequence) { + return sequence.toString(QKeySequence::PortableText); +} + +} // namespace + +std::span table() { + return kTable; +} + +const Entry& entry(Id id) { + const auto it = std::ranges::find(kTable, id, &Entry::id); + Q_ASSERT(it != kTable.end()); // every Id has a row (asserted in the tests too) + return *it; +} + +QList sequences(Id id) { + const Entry& row = entry(id); + if (row.standard != QKeySequence::UnknownKey) { + return QKeySequence::keyBindings(row.standard); + } + QList out; + out.append(QKeySequence(row.primary)); + if (row.alternate != 0) { + out.append(QKeySequence(row.alternate)); + } + return out; +} + +QString markdown() { + QString out; + QString category; + for (const Entry& row : kTable) { + const QString row_category = QString::fromUtf8(row.category); + if (row_category != category) { + category = row_category; + out += + QStringLiteral("\n## %1\n\n| Action | Shortcut | Notes |\n|---|---|---|\n").arg(category); + } + QStringList rendered; + if (row.documented[0] != '\0') { + // A platform-standard binding: the page commits to one spelling (see + // Entry::documented), the Notes cell carries the platform variants. + rendered.append(QStringLiteral("`%1`").arg(QString::fromUtf8(row.documented))); + } else { + const QList keys = sequences(row.id); + rendered.reserve(keys.size()); + for (const QKeySequence& key : keys) { + rendered.append(QStringLiteral("`%1`").arg(render(key))); + } + } + out += QStringLiteral("| %1 | %2 | %3 |\n") + .arg(QString::fromUtf8(row.description), + rendered.join(QStringLiteral(" or ")), + QString::fromUtf8(row.note)); + } + return out; +} + +} // namespace roadmaker::editor::shortcuts diff --git a/editor/src/app/shortcut_registry.hpp b/editor/src/app/shortcut_registry.hpp new file mode 100644 index 0000000..2ec0916 --- /dev/null +++ b/editor/src/app/shortcut_registry.hpp @@ -0,0 +1,96 @@ +#pragma once + +// The shortcut map (P1/GW-1 pass criterion "every binding is discoverable in +// the documented shortcut map"). +// +// One static table is the single source of truth: Actions binds from it, and +// docs/user-guide/shortcuts.md is RENDERED from it. A gtest compares the render +// against the committed page, so a binding that changes without its +// documentation fails CI instead of quietly drifting — which is exactly what +// happened before, when key letters were hand-copied into tooltips. + +#include +#include +#include + +namespace roadmaker::editor::shortcuts { + +/// Stable identifier for one bound action. Adding a value here without adding +/// a row to the table is a compile-time-visible gap (the table is asserted to +/// be complete). +enum class Id { + // File + NewScene, + Open, + Save, + SaveAs, + Quit, + // Edit + Undo, + Redo, + // Tools + ToolSelect, + ToolMove, + ToolCreateRoad, + ToolEditNodes, + ToolLaneProfile, + ToolElevation, + ToolCreateJunction, + ToolSplit, + ToolDelete, + // View + FrameSelection, + FrameCursor, + ViewPerspective, + ViewOrthographic, + ViewNorth, + ViewSouth, + ViewWest, + ViewEast, + ViewTop, +}; + +/// One row of the map. +struct Entry { + Id id; + const char* category; ///< the section it is documented under + const char* description; ///< what it does, for the page + + /// A platform's standard binding (File/Edit conventions). When set it is what + /// Actions binds, so each OS gets its native keys. + QKeySequence::StandardKey standard = QKeySequence::UnknownKey; + + int primary = 0; ///< a Qt::Key (optionally OR-ed with modifiers) + int alternate = 0; ///< 0 = none — e.g. the numpad-less digit for a cardinal + + /// What the PAGE says, for `standard` rows only. + /// + /// It cannot be rendered from `standard`: QKeySequence::keyBindings returns a + /// DIFFERENT list per platform (Redo is Ctrl+Y plus two others on Windows, + /// Cmd+Shift+Z on macOS) and some entries are empty on some platforms (Quit + /// has no binding on macOS — the platform menu owns ⌘Q). Generating from it + /// would make the committed page platform-specific, so the doc gate would + /// pass on the machine that generated it and fail on the other two. This + /// field is the one spelling the docs commit to; the app still binds + /// natively from `standard`. + const char* documented = ""; + + const char* note = ""; ///< platform caveat, or "" — rendered as a Notes cell +}; + +/// Every bound action, in documentation order. +[[nodiscard]] std::span table(); + +/// The row for `id`. Every Id has exactly one row (asserted in the tests). +[[nodiscard]] const Entry& entry(Id id); + +/// The key sequences for `id`, primary first — feed straight to +/// QAction::setShortcuts so the alternate is never dropped. +[[nodiscard]] QList sequences(Id id); + +/// The whole map rendered as the body of docs/user-guide/shortcuts.md. +/// PortableText so the page reads the same on every platform (NativeText would +/// bake in whichever OS generated it). +[[nodiscard]] QString markdown(); + +} // namespace roadmaker::editor::shortcuts diff --git a/editor/src/panels/editor2d_host.cpp b/editor/src/panels/editor2d_host.cpp new file mode 100644 index 0000000..793e0cc --- /dev/null +++ b/editor/src/panels/editor2d_host.cpp @@ -0,0 +1,63 @@ +#include "panels/editor2d_host.hpp" + +#include + +#include "document/selection_model.hpp" + +namespace roadmaker::editor { + +ProfileEditorPage::ProfileEditorPage(Document& document, SelectionModel& selection, QWidget* parent) + : panel_(new ProfilePanel(document, selection, parent)) {} + +QString ProfileEditorPage::title() const { + return QObject::tr("Vertical Profile"); +} + +QWidget* ProfileEditorPage::widget() { + return panel_; +} + +bool ProfileEditorPage::relevant(const SelectionModel& selection) const { + return selection.primary().road.is_valid(); +} + +Editor2DHost::Editor2DHost(const SelectionModel& selection, QWidget* parent) + : QWidget(parent), selection_(selection), tabs_(new QTabWidget(this)) { + setObjectName(QStringLiteral("editor2d_host")); + tabs_->setObjectName(QStringLiteral("editor2d_tabs")); + tabs_->setDocumentMode(true); + + auto* layout = new QVBoxLayout(this); + layout->setContentsMargins(0, 0, 0, 0); + layout->addWidget(tabs_); + + // The host follows the selection to surface the right editor; the PAGES + // subscribe on their own for their content, exactly as they did before they + // were hosted. + connect( + &selection_, &SelectionModel::selection_changed, this, &Editor2DHost::raise_relevant_page); +} + +void Editor2DHost::register_page(std::unique_ptr page) { + QWidget* widget = page->widget(); + const QString title = page->title(); + pages_.push_back(std::move(page)); + tabs_->addTab(widget, title); +} + +void Editor2DHost::raise_relevant_page() { + const int current = tabs_->currentIndex(); + if (current >= 0 && static_cast(current) < pages_.size() && + pages_[static_cast(current)]->relevant(selection_)) { + return; // already on a useful tab — don't yank it away mid-edit + } + for (std::size_t i = 0; i < pages_.size(); ++i) { + if (pages_[i]->relevant(selection_)) { + tabs_->setCurrentIndex(static_cast(i)); + return; + } + } + // Nothing relevant: leave the tab where it is rather than pick arbitrarily. +} + +} // namespace roadmaker::editor diff --git a/editor/src/panels/editor2d_host.hpp b/editor/src/panels/editor2d_host.hpp new file mode 100644 index 0000000..e482817 --- /dev/null +++ b/editor/src/panels/editor2d_host.hpp @@ -0,0 +1,93 @@ +#pragma once + +// The 2D Editor pane (P1/GW-2 step 7): a tabbed host for the editors that work +// in a flat, non-perspective view of one entity — the vertical profile today, +// the cross-section and the Signal Phase Editor (GW-4 step 4, p4-s5) later. +// +// The host is deliberately thin. A page owns its own selection subscription and +// its own commands exactly as it did standing alone; the host only decides +// which tab is worth looking at. That keeps pages testable on their own and +// means adding one is a page class plus a register_page() call. + +#include +#include +#include +#include + +#include "panels/profile_panel.hpp" + +namespace roadmaker::editor { + +class Document; +class SelectionModel; + +/// One pluggable editor in the 2D Editor pane. +class Editor2DPage { +public: + Editor2DPage() = default; + virtual ~Editor2DPage() = default; + + Editor2DPage(const Editor2DPage&) = delete; + Editor2DPage& operator=(const Editor2DPage&) = delete; + Editor2DPage(Editor2DPage&&) = delete; + Editor2DPage& operator=(Editor2DPage&&) = delete; + + /// The tab label. + [[nodiscard]] virtual QString title() const = 0; + + /// The widget to host. Owned by the page; the host reparents it into the + /// tab bar and never deletes it out from under the page. + [[nodiscard]] virtual QWidget* widget() = 0; + + /// Whether this page has anything to say about the current selection. The + /// host raises a relevant tab when the selection changes; it never hides an + /// irrelevant one, because a user who chose a tab should keep it. + [[nodiscard]] virtual bool relevant(const SelectionModel& selection) const = 0; +}; + +/// Hosts the existing ProfilePanel unchanged. The adapter lives here rather +/// than in ProfilePanel so the panel stays unaware it is hosted at all — which +/// is why editor/tests/test_profile_panel.cpp needed no changes. +class ProfileEditorPage : public Editor2DPage { +public: + ProfileEditorPage(Document& document, SelectionModel& selection, QWidget* parent = nullptr); + + [[nodiscard]] QString title() const override; + [[nodiscard]] QWidget* widget() override; + + /// Relevant whenever a road is selected — the profile edits z(s) of a road. + [[nodiscard]] bool relevant(const SelectionModel& selection) const override; + + [[nodiscard]] ProfilePanel* panel() { return panel_; } + +private: + ProfilePanel* panel_; +}; + +/// Tabbed container for Editor2DPages. +class Editor2DHost : public QWidget { + Q_OBJECT + +public: + explicit Editor2DHost(const SelectionModel& selection, QWidget* parent = nullptr); + + /// Adds `page` as a tab (the host takes ownership). Registration order is + /// tab order. + void register_page(std::unique_ptr page); + + [[nodiscard]] int page_count() const { return tabs_->count(); } + + [[nodiscard]] QString current_title() const { return tabs_->tabText(tabs_->currentIndex()); } + + /// Raises the first registered page that is relevant to the selection. A + /// no-op when the current tab is already relevant — switching tabs out from + /// under someone mid-edit would be worse than showing a stale one. + void raise_relevant_page(); + +private: + const SelectionModel& selection_; + QTabWidget* tabs_; + std::vector> pages_; +}; + +} // namespace roadmaker::editor diff --git a/editor/src/tools/create_junction_tool.cpp b/editor/src/tools/create_junction_tool.cpp index 2436135..5fd4093 100644 --- a/editor/src/tools/create_junction_tool.cpp +++ b/editor/src/tools/create_junction_tool.cpp @@ -16,8 +16,6 @@ CreateJunctionTool::CreateJunctionTool(Document& document, QObject* parent) void CreateJunctionTool::activate() { reset_session(); - emit status_message(tr("Create Junction — click 2+ road ends, or 1 end + a road body to tee " - "into it; Enter generates, Esc cancels")); } void CreateJunctionTool::deactivate() { @@ -292,4 +290,9 @@ void CreateJunctionTool::emit_count_status() { .arg(static_cast(ends_.size()))); } +QString CreateJunctionTool::instruction() const { + return tr("Click 2+ road ends, or one end and a road body to tee into it · Enter generates · Esc " + "cancels"); +} + } // namespace roadmaker::editor diff --git a/editor/src/tools/create_junction_tool.hpp b/editor/src/tools/create_junction_tool.hpp index d21f7df..2d86061 100644 --- a/editor/src/tools/create_junction_tool.hpp +++ b/editor/src/tools/create_junction_tool.hpp @@ -44,6 +44,8 @@ class CreateJunctionTool : public Tool { [[nodiscard]] std::size_t selected_count() const { return ends_.size(); } + [[nodiscard]] QString instruction() const override; + private: /// The road end nearest `cursor` within the snap radius (endpoints only), /// resolved to a RoadEnd (start/end by proximity). diff --git a/editor/src/tools/create_road_tool.cpp b/editor/src/tools/create_road_tool.cpp index e27515c..e6ea8a6 100644 --- a/editor/src/tools/create_road_tool.cpp +++ b/editor/src/tools/create_road_tool.cpp @@ -51,10 +51,7 @@ void append_polyline_of(PreviewGeometry& geometry, const ReferenceLine& line) { CreateRoadTool::CreateRoadTool(Document& document, QObject* parent) : Tool(parent), document_(document) {} -void CreateRoadTool::activate() { - emit status_message(tr("Click to place waypoints — Enter or double-click creates the road, " - "Backspace removes the last point, Esc cancels")); -} +void CreateRoadTool::activate() {} void CreateRoadTool::deactivate() { reset_session(); @@ -274,4 +271,9 @@ PreviewGeometry CreateRoadTool::preview() const { return geometry; } +QString CreateRoadTool::instruction() const { + return tr("Click to place waypoints · Enter or double-click creates the road · Backspace removes " + "the last · Esc cancels"); +} + } // namespace roadmaker::editor diff --git a/editor/src/tools/create_road_tool.hpp b/editor/src/tools/create_road_tool.hpp index 1a7ec53..5dc9d74 100644 --- a/editor/src/tools/create_road_tool.hpp +++ b/editor/src/tools/create_road_tool.hpp @@ -53,6 +53,8 @@ class CreateRoadTool : public Tool { /// arms the tool here so the user continues from the drop location. void begin_at(double world_x, double world_y); + [[nodiscard]] QString instruction() const override; + private: /// One placed waypoint plus the continuation heading of the snap that /// produced it (set for road-end and tangent snaps, nullopt otherwise). diff --git a/editor/src/tools/delete_tool.cpp b/editor/src/tools/delete_tool.cpp index 759103e..aada343 100644 --- a/editor/src/tools/delete_tool.cpp +++ b/editor/src/tools/delete_tool.cpp @@ -9,9 +9,7 @@ namespace roadmaker::editor { DeleteTool::DeleteTool(Document& document, QObject* parent) : Tool(parent), document_(document) {} -void DeleteTool::activate() { - emit status_message(tr("Click a road to delete it — Ctrl+Z restores")); -} +void DeleteTool::activate() {} bool DeleteTool::mouse_press(const ToolEvent& event) { if (!(event.buttons & Qt::LeftButton)) { @@ -32,4 +30,8 @@ bool DeleteTool::mouse_press(const ToolEvent& event) { return true; } +QString DeleteTool::instruction() const { + return tr("Click a road to delete it · Undo restores it"); +} + } // namespace roadmaker::editor diff --git a/editor/src/tools/delete_tool.hpp b/editor/src/tools/delete_tool.hpp index 59a7f0a..faa0572 100644 --- a/editor/src/tools/delete_tool.hpp +++ b/editor/src/tools/delete_tool.hpp @@ -23,6 +23,8 @@ class DeleteTool : public Tool { [[nodiscard]] bool mouse_press(const ToolEvent& event) override; + [[nodiscard]] QString instruction() const override; + private: Document& document_; }; diff --git a/editor/src/tools/edit_nodes_tool.cpp b/editor/src/tools/edit_nodes_tool.cpp index 94d1c24..43d39dd 100644 --- a/editor/src/tools/edit_nodes_tool.cpp +++ b/editor/src/tools/edit_nodes_tool.cpp @@ -37,10 +37,7 @@ bool has_param_poly3(const Road& road) { EditNodesTool::EditNodesTool(Document& document, SelectionModel& selection, QObject* parent) : Tool(parent), document_(document), selection_(selection) {} -void EditNodesTool::activate() { - emit status_message(tr("Drag a node to move it; click a midpoint marker to insert a node; " - "click a node, then Delete removes it — Esc cancels")); -} +void EditNodesTool::activate() {} void EditNodesTool::deactivate() { if (drag_.has_value()) { @@ -344,4 +341,9 @@ PreviewGeometry EditNodesTool::preview() const { return geometry; } +QString EditNodesTool::instruction() const { + return tr("Drag a node to move it · click a midpoint marker to insert one · click a node then " + "Delete removes it · Esc cancels"); +} + } // namespace roadmaker::editor diff --git a/editor/src/tools/edit_nodes_tool.hpp b/editor/src/tools/edit_nodes_tool.hpp index f0aff8c..998b22f 100644 --- a/editor/src/tools/edit_nodes_tool.hpp +++ b/editor/src/tools/edit_nodes_tool.hpp @@ -66,6 +66,8 @@ class EditNodesTool : public Tool { return active_; } + [[nodiscard]] QString instruction() const override; + private: struct MarkerHit { RoadId road; diff --git a/editor/src/tools/elevation_tool.cpp b/editor/src/tools/elevation_tool.cpp index e6d3904..fd98c0b 100644 --- a/editor/src/tools/elevation_tool.cpp +++ b/editor/src/tools/elevation_tool.cpp @@ -15,10 +15,7 @@ namespace roadmaker::editor { ElevationTool::ElevationTool(Document& document, SelectionModel& selection, QObject* parent) : Tool(parent), document_(document), selection_(selection) {} -void ElevationTool::activate() { - emit status_message(tr("Elevation — click a road node, then set its height in the Properties " - "panel; the grade re-fits as a smooth curve")); -} +void ElevationTool::activate() {} void ElevationTool::deactivate() { set_active(std::nullopt); @@ -120,4 +117,9 @@ PreviewGeometry ElevationTool::preview() const { return geometry; } +QString ElevationTool::instruction() const { + return tr("Click a road node, then set its height in the Properties panel or the 2D Editor · the " + "grade re-fits smoothly"); +} + } // namespace roadmaker::editor diff --git a/editor/src/tools/elevation_tool.hpp b/editor/src/tools/elevation_tool.hpp index 6cf52c3..40cd418 100644 --- a/editor/src/tools/elevation_tool.hpp +++ b/editor/src/tools/elevation_tool.hpp @@ -47,6 +47,8 @@ class ElevationTool : public Tool { return active_; } + [[nodiscard]] QString instruction() const override; + signals: /// Emitted whenever active_node() changes (the Properties panel re-syncs). void active_node_changed(); diff --git a/editor/src/tools/lane_profile_tool.cpp b/editor/src/tools/lane_profile_tool.cpp index 311e4c5..f1143e9 100644 --- a/editor/src/tools/lane_profile_tool.cpp +++ b/editor/src/tools/lane_profile_tool.cpp @@ -7,10 +7,7 @@ namespace roadmaker::editor { LaneProfileTool::LaneProfileTool(SelectionModel& selection, QObject* parent) : Tool(parent), selection_(selection) {} -void LaneProfileTool::activate() { - emit status_message( - tr("Lane Profile — click a lane to edit its cross-section in the Properties panel")); -} +void LaneProfileTool::activate() {} bool LaneProfileTool::mouse_press(const ToolEvent& event) { if (!(event.buttons & Qt::LeftButton)) { @@ -24,4 +21,8 @@ bool LaneProfileTool::mouse_press(const ToolEvent& event) { return true; // LMB belongs to the tool even on a miss (M2 button map) } +QString LaneProfileTool::instruction() const { + return tr("Click a lane, then edit its type, width, and road mark in the Properties panel"); +} + } // namespace roadmaker::editor diff --git a/editor/src/tools/lane_profile_tool.hpp b/editor/src/tools/lane_profile_tool.hpp index bda9d2e..9cc9902 100644 --- a/editor/src/tools/lane_profile_tool.hpp +++ b/editor/src/tools/lane_profile_tool.hpp @@ -22,6 +22,8 @@ class LaneProfileTool : public Tool { [[nodiscard]] bool mouse_press(const ToolEvent& event) override; + [[nodiscard]] QString instruction() const override; + private: SelectionModel& selection_; }; diff --git a/editor/src/tools/select_tool.cpp b/editor/src/tools/select_tool.cpp index eb0bb87..aed27c0 100644 --- a/editor/src/tools/select_tool.cpp +++ b/editor/src/tools/select_tool.cpp @@ -33,14 +33,10 @@ SelectTool::SelectTool(Document& document, SelectionModel& selection, QObject* p : Tool(parent), document_(document), selection_(selection) {} void SelectTool::activate() { - if (move_mode_) { - emit status_message(tr("Move tool — hover shows the 4-arrow cursor; drag a road or a prop to " - "move it, or click to select it and use the transform gizmo (Esc " - "cancels)")); - return; - } - emit status_message(tr("Click to select — Shift adds, Ctrl toggles; drag a road body to move the " - "whole road, a node handle to bend it, or empty space for a rubber band")); + // No status_message here: the guidance for "what does this tool do" is + // instruction(), which MainWindow shows in the persistent status line and the + // viewport hint. Emitting it transiently as well put two wordings of the same + // sentence in the status bar at once. } void SelectTool::deactivate() { @@ -656,4 +652,15 @@ PreviewGeometry SelectTool::preview() const { return geometry; } +QString SelectTool::instruction() const { + // One tool, two modes — the instruction follows the mode, as the old + // activate() message did. + if (move_mode_) { + return tr("Drag a road or prop to move it · click to select it and use the transform gizmo · " + "Esc cancels"); + } + return tr("Click to select · ⇧ adds, Ctrl toggles · drag a road body to move it, a node handle " + "to bend it, or empty space for a rubber band"); +} + } // namespace roadmaker::editor diff --git a/editor/src/tools/select_tool.hpp b/editor/src/tools/select_tool.hpp index 38b40a8..68e526b 100644 --- a/editor/src/tools/select_tool.hpp +++ b/editor/src/tools/select_tool.hpp @@ -79,6 +79,8 @@ class SelectTool : public Tool { [[nodiscard]] bool banding() const { return band_current_.has_value(); } + [[nodiscard]] QString instruction() const override; + signals: /// A double-click on a road body inserted a bend node (already committed as /// one command) and asks the app to switch to Edit Nodes and grab it — so a diff --git a/editor/src/tools/split_tool.cpp b/editor/src/tools/split_tool.cpp index 6f114ba..a951a9f 100644 --- a/editor/src/tools/split_tool.cpp +++ b/editor/src/tools/split_tool.cpp @@ -18,9 +18,7 @@ constexpr double kMarkerRadius = 1.2; // cut-cross half-size [m] SplitTool::SplitTool(Document& document, SelectionModel& selection, QObject* parent) : Tool(parent), document_(document), selection_(selection) {} -void SplitTool::activate() { - emit status_message(tr("Click a road to split it at the cut marker — Esc to cancel")); -} +void SplitTool::activate() {} void SplitTool::deactivate() { if (hover_.has_value()) { @@ -123,4 +121,8 @@ PreviewGeometry SplitTool::preview() const { return geometry; } +QString SplitTool::instruction() const { + return tr("Click a road to cut it in two at the marker · Esc returns to Select"); +} + } // namespace roadmaker::editor diff --git a/editor/src/tools/split_tool.hpp b/editor/src/tools/split_tool.hpp index b2124a3..2d3c283 100644 --- a/editor/src/tools/split_tool.hpp +++ b/editor/src/tools/split_tool.hpp @@ -34,6 +34,8 @@ class SplitTool : public Tool { /// A cross marker at the hovered cut point (lines). [[nodiscard]] PreviewGeometry preview() const override; + [[nodiscard]] QString instruction() const override; + private: /// The road + station the cursor currently points at (updated on hover). struct CutHover { diff --git a/editor/src/tools/tool.hpp b/editor/src/tools/tool.hpp index 4100841..cc1481a 100644 --- a/editor/src/tools/tool.hpp +++ b/editor/src/tools/tool.hpp @@ -111,8 +111,19 @@ class Tool : public QObject { [[nodiscard]] virtual PreviewGeometry preview() const { return {}; } + /// What this tool's click/drag/modifiers do right now — the PERSISTENT + /// status-bar instruction line (P1/GW-1). It answers "what can I do with + /// this tool", so it states the interaction, not the shortcut that reached + /// it (docs/user-guide/shortcuts.md owns keys) and not a transient result + /// (status_message owns those). Empty = nothing to say. + [[nodiscard]] virtual QString instruction() const { return {}; } + signals: void preview_changed(); + + /// Transient, state-dependent guidance or a result ("Merged", a refusal). + /// Distinct from instruction(): this one comes and goes, that one persists + /// for as long as the tool is active. void status_message(const QString& text); /// Requests a viewport cursor shape (first cursor mechanism, kept minimal): diff --git a/editor/tests/CMakeLists.txt b/editor/tests/CMakeLists.txt index d46249c..b4bd21f 100644 --- a/editor/tests/CMakeLists.txt +++ b/editor/tests/CMakeLists.txt @@ -38,6 +38,7 @@ add_executable(roadmaker_editor_tests test_delete_tool.cpp test_diagnostics_model.cpp test_document.cpp + test_editor2d_host.cpp test_document_commands.cpp test_edit_nodes_tool.cpp test_junction_regen.cpp @@ -67,6 +68,7 @@ add_executable(roadmaker_editor_tests test_slot_widget.cpp test_scene_tree_model.cpp test_selection_model.cpp + test_shortcut_registry.cpp test_soak_smoke.cpp test_theme.cpp test_toast_queue.cpp @@ -77,7 +79,10 @@ add_executable(roadmaker_editor_tests target_compile_definitions(roadmaker_editor_tests PRIVATE RM_SAMPLES_DIR="${CMAKE_SOURCE_DIR}/assets/samples" - RM_ASSETS_DIR="${CMAKE_SOURCE_DIR}/assets") + RM_ASSETS_DIR="${CMAKE_SOURCE_DIR}/assets" + # The shortcut-map gate reads the committed page and compares it to + # what the registry renders (test_shortcut_registry.cpp). + RM_DOCS_DIR="${CMAKE_SOURCE_DIR}/docs") target_link_libraries(roadmaker_editor_tests PRIVATE roadmaker_editor_lib diff --git a/editor/tests/test_editor2d_host.cpp b/editor/tests/test_editor2d_host.cpp new file mode 100644 index 0000000..a29b0d7 --- /dev/null +++ b/editor/tests/test_editor2d_host.cpp @@ -0,0 +1,175 @@ +// The 2D Editor pane's host (P1/GW-2 step 7). The host's whole job is tab +// management: pages keep their own selection subscriptions and their own +// commands, which is why test_profile_panel.cpp needed no changes to keep +// passing once the panel was hosted. + +#include + +#include +#include +#include +#include +#include + +#include "document/document.hpp" +#include "document/selection_model.hpp" +#include "panels/editor2d_host.hpp" + +namespace roadmaker::editor { +namespace { + +const std::filesystem::path kSample = std::filesystem::path(RM_SAMPLES_DIR) / "t_junction.xodr"; + +struct Harness { + Document document; + SelectionModel selection{document}; +}; + +std::vector all_roads(const Document& document) { + std::vector roads; + document.network().for_each_road([&](RoadId id, const Road&) { roads.push_back(id); }); + return roads; +} + +/// A page that is relevant only when told to be — lets the raise logic be +/// tested without dragging real editors in. +class FakePage : public Editor2DPage { +public: + FakePage(QString title, bool relevant) : title_(std::move(title)), relevant_(relevant) {} + + [[nodiscard]] QString title() const override { return title_; } + + [[nodiscard]] QWidget* widget() override { return &widget_; } + + [[nodiscard]] bool relevant(const SelectionModel&) const override { return relevant_; } + + void set_relevant(bool relevant) { relevant_ = relevant; } + +private: + QString title_; + bool relevant_; + QWidget widget_; +}; + +TEST(Editor2DHost, RegistersPagesAsTabsInOrder) { + Harness h; + Editor2DHost host(h.selection); + EXPECT_EQ(host.page_count(), 0); + + host.register_page(std::make_unique(QStringLiteral("First"), false)); + host.register_page(std::make_unique(QStringLiteral("Second"), false)); + + EXPECT_EQ(host.page_count(), 2); + EXPECT_EQ(host.current_title(), QStringLiteral("First")); +} + +TEST(Editor2DHost, RaisesTheFirstRelevantPage) { + Harness h; + Editor2DHost host(h.selection); + host.register_page(std::make_unique(QStringLiteral("Irrelevant"), false)); + host.register_page(std::make_unique(QStringLiteral("Relevant"), true)); + + host.raise_relevant_page(); + EXPECT_EQ(host.current_title(), QStringLiteral("Relevant")); +} + +// A user who picked a tab keeps it: yanking it away mid-edit because the +// selection moved would be worse than showing a stale one. +TEST(Editor2DHost, KeepsACurrentTabThatIsStillRelevant) { + Harness h; + Editor2DHost host(h.selection); + host.register_page(std::make_unique(QStringLiteral("A"), true)); + host.register_page(std::make_unique(QStringLiteral("B"), true)); + + auto* tabs = host.findChild(QStringLiteral("editor2d_tabs")); + ASSERT_NE(tabs, nullptr); + tabs->setCurrentIndex(1); + ASSERT_EQ(host.current_title(), QStringLiteral("B")); + + host.raise_relevant_page(); + EXPECT_EQ(host.current_title(), QStringLiteral("B")) << "both are relevant — don't switch"; +} + +TEST(Editor2DHost, LeavesTheTabAloneWhenNothingIsRelevant) { + Harness h; + Editor2DHost host(h.selection); + host.register_page(std::make_unique(QStringLiteral("A"), false)); + host.register_page(std::make_unique(QStringLiteral("B"), false)); + + auto* tabs = host.findChild(QStringLiteral("editor2d_tabs")); + tabs->setCurrentIndex(1); + host.raise_relevant_page(); + EXPECT_EQ(host.current_title(), QStringLiteral("B")) << "no arbitrary pick"; +} + +// The migration: the profile editor works hosted exactly as it did standing +// alone, and is relevant precisely when a road is selected. +TEST(ProfileEditorPage, HostsTheProfilePanelAndFollowsRoadSelection) { + Harness h; + ASSERT_TRUE(h.document.load(kSample).has_value()); + + auto page = std::make_unique(h.document, h.selection); + ProfileEditorPage* raw = page.get(); + EXPECT_EQ(raw->title(), QStringLiteral("Vertical Profile")); + ASSERT_NE(raw->panel(), nullptr); + EXPECT_FALSE(raw->relevant(h.selection)) << "nothing selected yet"; + + const RoadId road = all_roads(h.document).front(); + h.selection.select({.road = road}); + EXPECT_TRUE(raw->relevant(h.selection)); + // The panel kept its OWN subscription — the host never forwards selection. + EXPECT_EQ(raw->panel()->road(), road); + + Editor2DHost host(h.selection); + host.register_page(std::move(page)); + EXPECT_EQ(host.page_count(), 1); + EXPECT_EQ(host.current_title(), QStringLiteral("Vertical Profile")); +} + +TEST(Editor2DHost, SelectionChangeRaisesTheRelevantPage) { + Harness h; + ASSERT_TRUE(h.document.load(kSample).has_value()); + Editor2DHost host(h.selection); + host.register_page(std::make_unique(QStringLiteral("Never"), false)); + host.register_page(std::make_unique(h.document, h.selection)); + ASSERT_EQ(host.current_title(), QStringLiteral("Never")); + + // The host subscribes itself — no explicit raise call here. + h.selection.select({.road = all_roads(h.document).front()}); + EXPECT_EQ(host.current_title(), QStringLiteral("Vertical Profile")); +} + +// Why the dock was RENAMED rather than reusing "dock.profile": restoreState +// matches saved geometry to docks by objectName. A returning user's settings +// still carry a dock.profile entry — placed and sized for the old Profile +// panel — and reusing the name would let that stale entry capture the new, +// differently-shaped 2D Editor pane. An unknown name is ignored instead, so the +// default placement wins. (MainWindow itself needs a GL viewport and cannot be +// built offscreen, so this pins the Qt behaviour the decision rests on.) +TEST(Editor2DDock, StaleProfileLayoutDoesNotCaptureTheRenamedDock) { + // A layout saved by the OLD build: a bottom dock called dock.profile, hidden. + QByteArray stale_state; + { + QMainWindow old_window; + old_window.setCentralWidget(new QWidget(&old_window)); + auto* old_dock = new QDockWidget(QStringLiteral("Profile"), &old_window); + old_dock->setObjectName(QStringLiteral("dock.profile")); + old_window.addDockWidget(Qt::LeftDockWidgetArea, old_dock); // deliberately NOT bottom + stale_state = old_window.saveState(); + } + + // The new build: same settings blob, a dock named dock.editor2d. + QMainWindow window; + window.setCentralWidget(new QWidget(&window)); + auto* dock = new QDockWidget(QStringLiteral("2D Editor"), &window); + dock->setObjectName(QStringLiteral("dock.editor2d")); + window.addDockWidget(Qt::BottomDockWidgetArea, dock); + + window.restoreState(stale_state); + + EXPECT_EQ(window.dockWidgetArea(dock), Qt::BottomDockWidgetArea) + << "the stale dock.profile entry must not drag the 2D Editor to the left"; +} + +} // namespace +} // namespace roadmaker::editor diff --git a/editor/tests/test_shortcut_registry.cpp b/editor/tests/test_shortcut_registry.cpp new file mode 100644 index 0000000..53c319f --- /dev/null +++ b/editor/tests/test_shortcut_registry.cpp @@ -0,0 +1,102 @@ +// The shortcut map is a single source of truth: Actions binds from it and +// docs/user-guide/shortcuts.md is rendered from it. These tests are what keeps +// those three in step — before P1, key letters were hand-copied into tooltips +// and nothing noticed when they drifted. + +#include + +#include +#include +#include +#include +#include + +#include "app/actions.hpp" +#include "app/shortcut_registry.hpp" + +namespace roadmaker::editor { +namespace { + +using shortcuts::Id; + +TEST(ShortcutRegistry, EveryIdHasExactlyOneRow) { + QSet seen; + for (const shortcuts::Entry& row : shortcuts::table()) { + EXPECT_FALSE(seen.contains(static_cast(row.id))) + << "duplicate row for id " << static_cast(row.id); + seen.insert(static_cast(row.id)); + EXPECT_STRNE(row.description, "") << "every row must describe itself for the page"; + } + // The last enumerator; every value below it must be covered. + for (int i = 0; i <= static_cast(Id::ViewTop); ++i) { + EXPECT_TRUE(seen.contains(i)) << "shortcuts::Id " << i << " has no table row"; + } +} + +TEST(ShortcutRegistry, SequencesPutThePrimaryFirstAndKeepTheAlternate) { + const QList cardinals = shortcuts::sequences(Id::ViewNorth); + ASSERT_EQ(cardinals.size(), 2) << "the numpad-less alternate must not be dropped"; + EXPECT_EQ(cardinals.at(0), QKeySequence(Qt::KeypadModifier | Qt::Key_8)); + EXPECT_EQ(cardinals.at(1), QKeySequence(Qt::Key_8)); + + const QList select = shortcuts::sequences(Id::ToolSelect); + ASSERT_EQ(select.size(), 1); + EXPECT_EQ(select.at(0), QKeySequence(Qt::Key_Q)) << "Select rebound to Q in p1-s2"; +} + +// Two actions sharing a key would leave one silently dead. The numpad/top-row +// pairs are the same ACTION's two bindings, so they are not a conflict. +TEST(ShortcutRegistry, NoDuplicateActiveBindings) { + QSet seen; + for (const shortcuts::Entry& row : shortcuts::table()) { + for (const QKeySequence& key : shortcuts::sequences(row.id)) { + const QString text = key.toString(QKeySequence::PortableText); + EXPECT_FALSE(seen.contains(text)) + << "'" << text.toStdString() << "' is bound twice (" << row.description << ")"; + seen.insert(text); + } + } +} + +// The rebind that made room for frame-on-cursor. Worth pinning by name: it is +// the one binding change a user would notice. +TEST(ShortcutRegistry, VIsFrameOnCursorAndSelectIsQ) { + EXPECT_EQ(shortcuts::sequences(Id::FrameCursor).at(0), QKeySequence(Qt::Key_V)); + EXPECT_EQ(shortcuts::sequences(Id::ToolSelect).at(0), QKeySequence(Qt::Key_Q)); +} + +// Actions must BIND what the table says — otherwise the page documents one +// thing and the app does another. +TEST(ShortcutRegistry, ActionsBindWhatTheTableDocuments) { + QUndoStack stack; + Actions actions(stack); + + EXPECT_EQ(actions.tool_select->shortcuts(), shortcuts::sequences(Id::ToolSelect)); + EXPECT_EQ(actions.frame_cursor->shortcuts(), shortcuts::sequences(Id::FrameCursor)); + EXPECT_EQ(actions.frame_selection->shortcuts(), shortcuts::sequences(Id::FrameSelection)); + EXPECT_EQ(actions.view_orthographic->shortcuts(), shortcuts::sequences(Id::ViewOrthographic)); + EXPECT_EQ(actions.view_top->shortcuts(), shortcuts::sequences(Id::ViewTop)); + EXPECT_EQ(actions.tool_delete->shortcuts(), shortcuts::sequences(Id::ToolDelete)); +} + +// The gate: the committed page must be what the table renders. A binding +// changed without regenerating the page fails HERE rather than shipping a lie. +// RM_DOCS_DIR is a compile define (editor/tests/CMakeLists.txt). +TEST(ShortcutRegistry, MarkdownMatchesCommittedShortcutsPage) { + const std::filesystem::path page = + std::filesystem::path(RM_DOCS_DIR) / "user-guide" / "shortcuts.md"; + std::ifstream file(page); + ASSERT_TRUE(file.is_open()) << "missing " << page.string(); + std::stringstream buffer; + buffer << file.rdbuf(); + const QString committed = QString::fromStdString(buffer.str()); + + const QString generated = shortcuts::markdown(); + EXPECT_TRUE(committed.contains(generated)) + << "docs/user-guide/shortcuts.md is out of date with the shortcut table.\n" + "Regenerate its tables from shortcuts::markdown():\n\n" + << generated.toStdString(); +} + +} // namespace +} // namespace roadmaker::editor From c76ece3d0d1e136c568665de85992eb83df8c15e Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Wed, 15 Jul 2026 14:28:20 -0700 Subject: [PATCH 2/2] fix(editor): store shortcut bindings as QKeyCombination, not int MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows CI caught this; clang did not. `Qt::KeypadModifier | Qt::Key_8` yields a QKeyCombination, so storing it in an `int` field went through QKeyCombination::operator int() — deprecated since Qt 6.0. MSVC reports the deprecation (C4996) and CI builds with /WX, so the editor lib failed to compile on Windows while macOS and Linux were green. The field type was simply wrong, so fix the type rather than silence the warning. A plain Qt::Key still converts implicitly, so unmodified rows read as before; `alternate` now spells "none" as Qt::Key_unknown instead of 0. --- editor/src/app/shortcut_registry.cpp | 2 +- editor/src/app/shortcut_registry.hpp | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/editor/src/app/shortcut_registry.cpp b/editor/src/app/shortcut_registry.cpp index a22dcc3..a32e9f1 100644 --- a/editor/src/app/shortcut_registry.cpp +++ b/editor/src/app/shortcut_registry.cpp @@ -162,7 +162,7 @@ QList sequences(Id id) { } QList out; out.append(QKeySequence(row.primary)); - if (row.alternate != 0) { + if (row.alternate.key() != Qt::Key_unknown) { out.append(QKeySequence(row.alternate)); } return out; diff --git a/editor/src/app/shortcut_registry.hpp b/editor/src/app/shortcut_registry.hpp index 2ec0916..a464e85 100644 --- a/editor/src/app/shortcut_registry.hpp +++ b/editor/src/app/shortcut_registry.hpp @@ -60,8 +60,16 @@ struct Entry { /// Actions binds, so each OS gets its native keys. QKeySequence::StandardKey standard = QKeySequence::UnknownKey; - int primary = 0; ///< a Qt::Key (optionally OR-ed with modifiers) - int alternate = 0; ///< 0 = none — e.g. the numpad-less digit for a cardinal + /// The binding, as QKeyCombination — NOT int. `Qt::KeypadModifier | Qt::Key_8` + /// already yields a QKeyCombination, and storing it in an int would go through + /// QKeyCombination::operator int(), which Qt deprecated in 6.0 (MSVC flags it + /// and CI's /WX turns that into a build failure). A plain `Qt::Key` converts + /// implicitly, so unmodified rows still read as `.primary = Qt::Key_Q`. + QKeyCombination primary = Qt::Key_unknown; + + /// A second binding for the same action — the numpad-less digit on a cardinal. + /// Qt::Key_unknown = none. + QKeyCombination alternate = Qt::Key_unknown; /// What the PAGE says, for `standard` rows only. ///