alandtse / alandtse/imgui-vr-helper

Devbench: headless keyboard injection + client keyboard diagnostics

Đang mở
#47 1 bình luận 1 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C++
Star
0
Fork
2
Merge trung bình
1 ngày 18 giờ
Pull request đã merge (30 ngày)
11

Mô tả

# Devbench: headless-capable keyboard injection + centralized client keyboard diagnostics

## Problem

Bugs inside imgui-vr-helper's own PumpInput/PumpKeyboard/focus pipeline (e.g. a VR-keyboard
seed-capture bug where `ActiveId=0` even though `io.WantTextInput=true` after a wand click)
currently require a human in a headset for every iteration: don headset, navigate to a field,
reproduce the click sequence, report log lines back. 5-10 minutes per iteration.

## What already works today (no new code needed)

- Deterministic wand click at an exact panel UV via `imguivrhelper.input`
(`action="pointer"` then `action="button"`) — goes through the real input-thread queue
(`Input::InjectButton` → `DrainInjected`), works with **zero OpenVR/HMD dependency**.
- `inspect kind=imguivrhelper` already returns registered clients, focused client, wand
intersect/UV, drag state, held masks, lease strips.
- `imguivrhelper.dumppanel` for visual panel verification.

**Important correction from review**: reproducing the `ActiveId=0` bug itself does **not**
need any new keyboard bypass — it needs the *existing* wand-click injection (a real click,
through the real routing path) combined with Gap 2's new diagnostics readout below. Gap 1 is
for a different, complementary purpose: iterating on the keyboard text/seed/diff/Enter-submit
behavior *once a field is confirmed active*, independent of the click-routing bug.

## Gap 1 — VR-keyboard text injection has no headless bypass

`VRKeyboard.cpp` requires a live `vr::IVROverlay` for every operation. Proposed fix: a
`debugKeyboard` override in `Overlay::State`, but corrected for two real defects an
adversarial review caught in the first draft:

1. **Must share `VRKeyboard.cpp`'s existing `g_mutex`**, not a separate mutex — a second lock
guarding overlapping state is a data race with the real input-thread path.
2. **Must be scoped per-client** (mirroring the existing `g_wantClient`/`g_shownClient`
checks), not a bare global flag — an unscoped `closedPending` latch would leak a spurious
"Done" edge into whichever client happens to open a keyboard next.
3. **Seeding must echo into the debug buffer** the same way the real path does
(`g_currentText = g_seed` on activation) — otherwise `PumpKeyboard()`'s diff sees the debug
buffer as empty against a non-empty `m_kbDelivered` and emits a wall of spurious backspaces,
erasing the seed before any synthetic typing happens.
4. **Activating the bypass mid-session must not orphan a real overlay** — if `g_shown` is
already true when `debugKeyboard.active` flips on, force a clean `HideKeyboard()` +
`g_shown=false` first, rather than silently short-circuiting `Tick()` and leaving a real
OpenVR keyboard overlay stuck open in the HMD.

New `imguivrhelper.input` actions once corrected:
- `action="text", client_id=N, text="..."` — sets the synthetic runtime keyboard buffer for
that specific client.
- `action="closekeyboard", client_id=N` — sets that client's one-shot "Done" edge.

Extend `DiagnosticsJson()` with keyboard-shown state (owning client id, shown,
dismissCooldown remaining, delivered-text length) — **read these fields under `g_mutex`**
(or make them atomic), since `DiagnosticsJson()` runs on DevBench's own thread and the current
draft would otherwise read `g_shown`/`g_hideGrace` unsynchronized against the input thread.

## Gap 2 — Client ImGui state (ActiveId, InputTextState, WantTextInput) is invisible to imgui-vr-helper's own process

This data lives in each client's own private ImGui context — imgui-vr-helper.dll has no
pointer into it. `PumpKeyboard()`'s existing debug log already computes everything needed;
it just writes to the client's own SKSE log.

**Revised design** (the first draft's "ship a copy-paste snippet, client links devbench-api
itself" was a false dichotomy an adversarial review correctly flagged — imgui-vr-helper.dll
already links devbench-api and already has a vtable channel to every client):

Add a lightweight call on the existing client→helper interface, e.g.
`IImGuiVRHelperInterface002::ReportKeyboardDiagnostics(client_id, const KeyboardDiagnosticsWire&)`,
that `PumpKeyboard()` calls each time it computes the diagnostics (replacing the current
`SKSE::log::info` call with a structured push through the same channel `SetKeyboardActive`
already uses). imgui-vr-helper.dll stores the latest diagnostics per client id
(mutex/atomic-guarded, matching the rest of `VRKeyboard.cpp`'s synchronization) and folds them
into its **own existing** `inspect kind=imguivrhelper` payload.

This means:
- Zero new per-client devbench-api dependency — every client gets this for free on next SDK
header update, with no opt-in wiring required.
- Diagnostics for every connected client are visible from ONE place (imgui-vr-helper's own
bridge), matching the "shared, client-agnostic" goal the feature was scoped around.

```cpp
struct KeyboardDiagnosticsWire {
bool wantTextInput;
unsigned int activeId;
unsigned int inputTextStateId;
bool idMatch;
int seedLen;
char seed[256]; // fixed-size for a stable, allocation-free ABI across the ADI boundary
bool kbShown;
int kbDismissCooldown;
};
```

**Known inherited risk (not newly introduced by this change)**: `PumpKeyboard()` already reads
`ImGuiContext::InputTextState`, an internal (non-public-API) Dear ImGui struct whose layout can
shift across ImGui versions; `HasTextLenMember` already handles the one known TextLen/CurLenA
split. This plan only exposes an existing computation — it does not add new internal-struct
reads, so it does not change this pre-existing exposure. Worth a one-line comment at the
accessor site so it isn't mistaken for new risk during review.

## Sizing / staging

Two independent, separately shippable PRs against imgui-vr-helper:

1. **PR 1 (Gap 1)**: `debugKeyboard` override (shared-mutex, per-client-scoped, seed-echoing,
clean-takeover-on-activate) + two new `imguivrhelper.input` actions + synchronized
keyboard-shown fields in `DiagnosticsJson()`. No SDK header changes, no client rebuilds.
2. **PR 2 (Gap 2)**: `ReportKeyboardDiagnostics` interface call + per-client diagnostics store
in imgui-vr-helper.dll + fold into `inspect kind=imguivrhelper`. SDK header change is
additive only (replaces an internal log call with a structured push) — no behavior change
for existing clients, no opt-in wiring needed on their side.

Order doesn't matter for correctness (they're independent), but PR 2 has more diagnostic value
per unit effort — it makes the *existing* wand-click injection immediately useful for
bisecting the ActiveId bug, without waiting on PR 1 at all.

## Non-goals

- No general VR testing/record-replay framework — the two new actions compose into devbench's
existing `scenario`/`record`/`replay` primitives for free.
- No cross-DLL memory proxy beyond the one new interface call in Gap 2 (that call is the
intentional, minimal, synchronized bridge — not a raw memory read across the boundary).
- No fix for Modex's own `Hooks.cpp` crash bug (separate, Modex-internal, unrelated repo).
- No physical-HMD simulation (rendering, head pose, controller pose synthesis).
- No devbench-core (shared server) changes — both PRs stay within imgui-vr-helper's existing
`RegisterTool`/`RegisterToolExtension` surface; devbench's `ToolRegistry` needs no changes.

## Critical files
- `src/VRKeyboard.cpp`, `src/VRKeyboard.h`, `src/Overlay.h`, `src/DevBenchBridge.cpp`,
`api/ImGuiVRHelperClientSDK.h`, `src/HelperImpl.cpp`

---
Planned via Plan subagent, adversarially reviewed (agy-bridge / Gemini), and reconciled before
filing. See the four critical/major findings folded into Gap 1/Gap 2 above for what changed
from the first draft.

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.