alandtse / alandtse/imgui-vr-helper

Devbench: headless keyboard injection + client keyboard diagnostics

Ouverte
#47 1 commentaire 1 réaction 0 personnes assignées Voir sur GitHub
Langage dominant
C++
Étoiles
0
Forks
2
Merge moyen
1 j 18 h
PR mergées (30 j)
11

Description

# 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.

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Piste de recherche

Start by reading src/VRKeyboard.cpp, src/VRKeyboard.h, src/Overlay.h, and src/DevBenchBridge.cpp to understand the existing keyboard state, input actions, and diagnostics path. Then inspect api/ImGuiVRHelperClientSDK.h and src/HelperImpl.cpp for the client interface and per-client storage. Done means two independently shippable PRs provide the scoped keyboard injection and synchronized diagnostics in inspect kind=imguivrhelper without devbench-core or client opt-in changes.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
cpp
Domaine
ar-vr-xr, tooling
Type d'issue
Fonctionnalité
Difficulté
5/5
Temps estimé
Plus d'une semaine
Activité
Calme
Clarté
Plutôt claire
Accessibilité débutants
38/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.