Skip to content

Commit b8477f2

Browse files
committed
Implemented inspector-indefinite-drag
1 parent e05ebe5 commit b8477f2

14 files changed

Lines changed: 1327 additions & 17 deletions

File tree

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
# Workflow Coordination: Inspector Indefinite Drag
2+
3+
## Orchestrator
4+
5+
**Feature**: `inspector-indefinite-drag`
6+
**Status**: completed
7+
**Current step**: done
8+
**Loop history**: 1 — implementation-contract-critic rejected contract due to incorrect include paths in section 5a. Looping back to implementation-contract-author for fix.
9+
**Naming correction**: Method renamed from `warp_mouse` to `set_mouse_position` per human feedback. Moved from `Window` to `InputSystem` for API symmetry with `InputSystem::mouse_position()`.
10+
**Initial instructions**: In the editor inspector, when dragging floats or vec props, when the mouse goes outside the editor, the drag is stopped — we should be able to drag indefinitely (Unity/Unreal style).
11+
**Notes**:
12+
13+
### Decision Log (Grill Me)
14+
15+
**Definition of Ready walkthrough (2026-06-14):**
16+
17+
**Clarity & Completeness:**
18+
- [x] Scope clearly defined: Enable indefinite drag-to-scrub on float/vec property handles in the editor inspector using relative mouse mode (cursor hidden, raw mouse deltas). Int, bool, string, Color property types excluded.
19+
- [x] Dependencies identified: `Window::set_mouse_capture()` (already exists, uses SDL_SetWindowRelativeMouseMode), `InputSystem::mouse_delta()` (already exists, accumulates raw xrel/yrel). New: `InputSystem::set_mouse_position()` to restore cursor position on drag end.
20+
- [x] Edge cases described: Mouse release outside window (relative mode captures mouse, button up delivered), ESC during drag (not handled specially — drag naturally ends when mouse button released).
21+
- [x] Expected behavior unambiguous: User clicks drag handle → cursor hidden, relative mode enabled, value changes based on raw mouse delta → user releases → cursor warps back to starting position, relative mode disabled.
22+
23+
**Verification:**
24+
- [x] Verification method: Manual E2E testing (drag past window boundary, verify value continues changing, verify cursor returns to start on release).
25+
- [x] Acceptance criteria specific and measurable: (1) Drag past window edge continues updating value, (2) Drag ends properly when button released outside window, (3) Cursor returns to starting position on release, (4) No interference with other editor functionality.
26+
- [x] Success/failure states described.
27+
28+
**Documentation:**
29+
- [x] Interface changes: `InputSystem` base class gets new `set_mouse_position(int x, int y)` pure virtual method.
30+
- [x] Existing documentation to update: `docs/wiki/editor/editor-panels.md` (drag handle behavior description).
31+
32+
**Technical:**
33+
- [x] Technical constraints: SDL3's `SDL_SetWindowRelativeMouseMode` used via existing `Window::set_mouse_capture()`. New `SDL_WarpMouseInWindow` via `InputSystem::set_mouse_position()`. No build changes needed.
34+
- [x] Risks/unknowns: Relative mouse mode + ImGui interaction verified — ImGui correctly tracks active item state via mouse button events (unaffected by relative mode). `GetMouseDragDelta()` not used during relative mode drag; custom accumulator using `InputSystem::mouse_delta().x` instead.
35+
- [x] Performance: Minimal — one extra `mouse_delta()` call per frame during drag.
36+
37+
**Key decisions:**
38+
- Approach: Use `SDL_SetWindowRelativeMouseMode` (cursor lock) + `InputSystem::mouse_delta()` for raw deltas (Option B — infinite drag, Unity/Unreal style).
39+
- Cursor behavior: Hidden during drag, restored + warped to starting position on release.
40+
- Scope: Float, Vec2, Vec3, Vec4, Quat drag handles only (not int).
41+
- Testing: Skip automated tests for now (manual E2E only).
42+
43+
## spec-author
44+
45+
**Status**: completed
46+
**Summary**:
47+
Spec for indefinite drag-to-scrub on float/vec property handles in the editor inspector. Uses relative mouse mode (SDL_SetWindowRelativeMouseMode) + InputSystem::mouse_delta() to enable infinite drag past window boundaries. Adds InputSystem::set_mouse_position() to restore cursor position on release. Covers draw_axis_widget() and the float editor lambda in inspector_editors.cpp. Excludes int/bool/string/Color editors. 10 acceptance criteria covering behavior, interface changes, and non-regression.
48+
**Artifacts**:
49+
- `.specs/sprint-2026-06/inspector-indefinite-drag/spec.md`
50+
**Questions for human**:
51+
none
52+
**Warnings**:
53+
none
54+
**Blocking issues**:
55+
none
56+
57+
## Human Spec Validation
58+
59+
**Status**: approved
60+
**Summary**:
61+
Spec approved by human. Key changes: `InputSystem::set_mouse_position()` added to InputSystem interface (not Window), relative mouse mode + `InputSystem::mouse_delta()` for indefinite drag.
62+
**Date**: 2026-06-14 18:57
63+
**Feedback**: Approved after naming correction (warp_mouse → set_mouse_position, moved from Window to InputSystem).
64+
65+
## implementation-contract-author
66+
67+
**Status**: completed
68+
**Summary**:
69+
Wrote implementation contract at `.specs/sprint-2026-06/inspector-indefinite-drag/implementation-contract.md`. Contract covers: (1) adding `InputSystem::set_mouse_position()` pure virtual to input_system.h; (2) extending InputSystemSDL3 with `sdl_window_` member, `set_sdl_window()`, and `set_mouse_position()` via SDL_WarpMouseInWindow; (3) no-op override in InputSystemHeadless; (4) wiring `set_sdl_window()` in PlatformSDL3::create_window(); (5) replacing drag logic in `draw_axis_widget()` and the float editor lambda to use relative mouse mode + `mouse_delta().x` instead of `ImGui::GetMouseDragDelta()`, with a `DragState` struct; (6) logging on drag start/end/warp.
70+
71+
15 verifiable Done Criteria (DC-01 through DC-15). Edge cases from spec spec carried forward. Notes one discrepancy: the spec's "set_mouse_capture(true) failed" warning log cannot be implemented because `Window::set_mouse_capture` currently returns `void` — documented in contract for implementer to handle.
72+
73+
**Loopback fix (iteration 2)**: Fixed include paths in section 5a — removed erroneous `engine/` prefix from `#include "engine_service.h"`, `#include "input/input_system.h"`, `#include "platform/platform.h"` to match the engine's public include directory layout (`src/engine/`).
74+
**Artifacts**:
75+
- `.specs/sprint-2026-06/inspector-indefinite-drag/implementation-contract.md`
76+
**Questions for human**:
77+
none
78+
**Warnings**:
79+
- The spec's failure log `BUDDD_LOG_TAGGED_WARN("Editor:Inspector", "set_mouse_capture(true) failed")` (spec line 311) cannot be implemented because `Window::set_mouse_capture` currently returns `void`. Documented in the contract's Required implementation behavior section 8 for the implementer to handle (either leave the log out or extend the Window interface — the latter would expand scope).
80+
**Blocking issues**:
81+
none
82+
83+
## implementation-contract-critic
84+
85+
**Status**: completed
86+
**Summary**:
87+
Re-review after loopback fix: Include paths in section 5a corrected — `"engine_service.h"`, `"input/input_system.h"`, `"platform/platform.h"` no longer have the erroneous `engine/` prefix. All 10 ACs still covered by 15 DCs. No new issues introduced. Contract is sound and ready for implementation. Verdict: accepted.
88+
**Artifacts**:
89+
- `.specs/sprint-2026-06/inspector-indefinite-drag/implementation-contract-critic.md`
90+
**Questions for human**:
91+
none
92+
**Warnings**:
93+
- Fragile line number references throughout the contract — absolute line numbers may not match actual files if edited since contract was written. Descriptive markers would be more robust.
94+
- `<unordered_map>` not explicitly included in `inspector_editors.cpp` (pre-existing — works via transitive include).
95+
- `SDL_WarpMouseInWindow` return value (bool) ignored; `set_mouse_position` is `-> void` per spec — no way to detect warp failures.
96+
- `platform_sdl3.cpp` insertion point line number reference may be off.
97+
**Blocking issues**:
98+
none
99+
100+
## Human Validation
101+
102+
**Status**: approved
103+
**Approver**: Hilderin
104+
**Date**: 2026-06-14 18:57
105+
**Warnings**:
106+
- Spec's failure log for `set_mouse_capture(true)` cannot be implemented due to `void` return type (discrepancy noted, non-blocking).
107+
**Notes**: Approved to proceed with implementation.
108+
109+
## code-implementer
110+
111+
**Status**: completed
112+
**Summary**:
113+
Implemented indefinite drag-to-scrub for float/vec property handles in the editor inspector. Added `InputSystem::set_mouse_position()` pure virtual to the input system interface, with SDL3 implementation via `SDL_WarpMouseInWindow` (with null check) and headless no-op. Extended `InputSystemSDL3` with `sdl_window_` member and `set_sdl_window()` setter. Wired `set_sdl_window()` in `PlatformSDL3::create_window()` after `SDL_CreateWindow`. Replaced `draw_axis_widget()` and float editor lambda drag handling to use relative mouse mode (`Window::set_mouse_capture()`) + `InputSystem::mouse_delta().first` accumulation instead of `ImGui::GetMouseDragDelta()`, using a new `DragState` struct. Added logging on drag start/end/warp. Added necessary includes (`engine_service.h`, `input/input_system.h`, `platform/platform.h`, `window/window.h`).
114+
**Artifacts**:
115+
- `src/engine/input/input_system.h` — added `set_mouse_position` pure virtual
116+
- `src/engine/input/input_system_sdl3.h` — added `set_sdl_window`, `set_mouse_position` override, `sdl_window_` member
117+
- `src/engine/input/input_system_sdl3.cpp` — implemented `set_mouse_position` and `set_sdl_window`
118+
- `src/engine/input/input_system_headless.h` — added `set_mouse_position` override
119+
- `src/engine/input/input_system_headless.cpp` — implemented no-op `set_mouse_position`
120+
- `src/engine/platform/platform_sdl3.cpp` — wired `input_system_.set_sdl_window(sdl_window)`
121+
- `src/editor/inspector_editors.cpp` — replaced drag logic with relative mouse mode, added includes, added `DragState` struct
122+
**Questions for human**:
123+
none
124+
**Warnings**:
125+
- `Window::set_mouse_capture()` returns `void`, so the spec's aspirational failure log (`"set_mouse_capture(true) failed"`) could not be implemented — this was documented in the contract and is a non-issue.
126+
- Added `#include "window/window.h"` beyond what the contract listed, because `Window` is only forward-declared in `engine_context.h` and the code needs a complete type to call `set_mouse_capture()`.
127+
**Blocking issues**:
128+
none
129+
130+
## tester
131+
132+
**Status**: completed
133+
**Summary**:
134+
Completed build verification and full test suite run. All 833 tests pass with zero failures and zero new warnings. Verified all 15 Done Criteria against the implementation. Confirmed that all forbidden files remain unchanged. The feature cannot be automated (requires OS-level relative mouse mode). A detailed manual test procedure is provided in the test report.
135+
**Artifacts**:
136+
- `.specs/sprint-2026-06/inspector-indefinite-drag/test-report.md`
137+
**Questions for human**:
138+
none
139+
**Warnings**:
140+
- The spec's "set_mouse_capture(true) failed" warning log cannot be implemented because `Window::set_mouse_capture()` returns `void`. This pre-existing limitation is documented and non-blocking.
141+
**Blocking issues**:
142+
none
143+
**Manual tests required**:
144+
See `.specs/sprint-2026-06/inspector-indefinite-drag/test-report.md` — 7-step manual test procedure covering: drag past window boundary, release outside window, rotation handles, click-to-type, FreeCameraMovement, int editor, and bool/string/Color editors.
145+
146+
## Manual Test Validation
147+
148+
**Status**: passed
149+
**Instructions**:
150+
7 manual tests covering: drag past window boundary, release outside window, rotation handles, click-to-type, FreeCameraMovement, int editor, bool/string/Color editors.
151+
**Human feedback**:
152+
All 7 tests passed successfully.
153+
**Date**: 2026-06-14 18:57
154+
**Notes**: Feature behaves as expected — cursor hidden during drag, value changes outside window, cursor warps back on release. No interference with other editors or FreeCameraMovement.
155+
156+
## wiki-agent
157+
158+
**Status**: completed
159+
**Summary**:
160+
Updated the operational wiki to document the new indefinite-drag behavior for float/vec inspector property handles. Added the new `InputSystem::set_mouse_position()` method to the Input submodule module-map. Updated the Inspector Property Editors section to describe the relative-mouse-mode drag pattern (cursor hidden, raw deltas from `InputSystem::mouse_delta()`, warp on release).
161+
**Artifacts**:
162+
- `docs/wiki/editor/editor-panels.md`
163+
- `docs/wiki/architecture/module-map.md`
164+
**Changes made**:
165+
- `docs/wiki/editor/editor-panels.md`: Added SPEC-NNNN subsection to the Inspector Property Editors blockquote describing the indefinite-drag behavior (relative mouse mode, cursor hiding, InputSystem::mouse_delta accumulation, warp on release via InputSystem::set_mouse_position).
166+
- `docs/wiki/architecture/module-map.md`: Added `set_mouse_position(int x, int y)` to the `input_system.h` row in the Input submodule table.
167+
**Questions for human**:
168+
none
169+
**Warnings**:
170+
none
171+
**Blocking issues**:
172+
none
173+
174+
---
175+
176+
**Constraints:**
177+
178+
- Use exact heading names as listed above (case-sensitive).
179+
- Use exact field names as listed above (bold markdown `**Field**`).
180+
- Sub-agent sections must appear in the exact order listed above (spec-author → Human Spec Validation → implementation-contract-author → implementation-contract-critic → Human Validation → code-implementer → tester → Manual Test Validation → wiki-agent).
181+
- The `## Human Validation` section must appear between `## implementation-contract-critic` and `## code-implementer`.
182+
- The `## wiki-agent` section must include `**Changes made**` instead of `**Decisions needed**`.
183+
- **Exception**: during loop-backs, the orchestrator may temporarily reset a sub-agent's `**Status**` to "in-progress" to re-invoke them. This overrides the general principle that sub-agents self-manage their own status and is the only case where the orchestrator writes to a sub-agent's status field.
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Implementation Contract Review — Inspector Indefinite Drag
2+
3+
**Re-review (2026-06-14)**: Loopback fix verified. Include paths in section 5a are now correct (`"engine_service.h"`, `"input/input_system.h"`, `"platform/platform.h"`). The previously blocking issue (DC-07 compile failure) is resolved. All 10 ACs remain covered by the 15 DCs. No new issues introduced. **Verdict: accepted** — contract is ready for implementation.
4+
5+
## Blocking issues
6+
7+
Items that must be resolved before the artifact can be accepted.
8+
9+
- [x] **DC-07 include paths will fail to compile** — RESOLVED in loopback fix. The contract (section 5a) specifies adding `#include "engine/engine_service.h"`, `#include "engine/input/input_system.h"`, `#include "engine/platform/platform.h"`. These paths are wrong. The `buddd_engine` target sets its public include directory to `${CMAKE_CURRENT_SOURCE_DIR}` which is `src/engine/`. The existing file already uses this convention (e.g., `#include "log/log.h"` resolves to `src/engine/log/log.h`). The correct includes should be:
10+
- `#include "engine_service.h"` (not `"engine/engine_service.h"`)
11+
- `#include "input/input_system.h"` (not `"engine/input/input_system.h"`)
12+
- `#include "platform/platform.h"` (not `"engine/platform/platform.h"`)
13+
14+
As written, these includes would attempt to resolve to `src/engine/engine/engine_service.h` etc., which does not exist. Fixing this is necessary before the code can compile. (Affects DC-13.)
15+
16+
**Resolution**: This is a contract-level issue — loop back to implementation-contract-author.
17+
18+
## Warnings
19+
20+
Non-blocking concerns for awareness:
21+
22+
- **Fragile line number references** — The contract uses absolute line numbers throughout (e.g., "after line 36", "between line 174 and line 176", "line 146-198"). These line numbers may not match the actual files if they have been edited since the contract was written. The surrounding descriptive text is clear enough for an experienced implementer, but the line numbers could mislead. Consider using descriptive markers (e.g., "after the `// ── State (double-buffered) ──` comment") instead.
23+
24+
- **`<unordered_map>` not explicitly included** — The existing `inspector_editors.cpp` uses `std::unordered_map` (line 84) without a direct `#include <unordered_map>`. It compiles because it is transitively included via `editor.h``scene/world.h` → other headers. The contract extends this usage with a new `std::unordered_map<const void*, DragState>` type. This works by transitive include, but an explicit `#include <unordered_map>` would be more robust. Pre-existing issue, not introduced by this contract.
25+
26+
- **`SDL_WarpMouseInWindow` return value ignored** — In SDL3, `SDL_WarpMouseInWindow` returns `bool` (true on success). The contract's `InputSystemSDL3::set_mouse_position` has `-> void` return type and ignores the SDL3 return value. This is acceptable (matching the spec's `-> void` interface) but means the implementation cannot detect or log warp failures.
27+
28+
- **`platform_sdl3.cpp` insertion point description** — The contract says "between line 174 and line 176" for the `set_sdl_window` call in `create_window()`. In the actual file, these line numbers correspond to the gap between the null-check closing brace and `SDL_SetWindowMinimumSize`. The conceptual location is correct, but the line number reference may shift.
29+
30+
## Required changes
31+
32+
Concrete, actionable changes requested:
33+
34+
1. **Fix include paths** in section 5a: Change `#include "engine/engine_service.h"``#include "engine_service.h"`, `#include "engine/input/input_system.h"``#include "input/input_system.h"`, `#include "engine/platform/platform.h"``#include "platform/platform.h"`.
35+
36+
## Suggested improvements
37+
38+
Optional ideas (not required):
39+
40+
- Add explicit `#include <unordered_map>` in `inspector_editors.cpp` to reduce reliance on transitive includes.
41+
- Replace absolute line number references with descriptive markers throughout the contract.
42+
- Add a note that `InputSystem::create(Backend)` factory (in `input_system.cpp`) will need a `set_sdl_window` wiring update if any test code creates `InputSystemSDL3` directly — though this is unlikely since the factory is the only creation path.

0 commit comments

Comments
 (0)