Repository navigation
Input: Unify native key queries and shared acquisition - #292
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (18)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe changes add reusable input state and resource-hold tracking, a Win32 message-based input implementation, and physical-key query helpers. They also update script keybinds, register input tests, and add native input documentation. ChangesInput support
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Win32Message
participant WindowInput
participant ButtonState
Win32Message->>WindowInput: ProcessEvent keyboard or mouse message
WindowInput->>ButtonState: Set key and button state
WindowInput->>ButtonState: Clear edges during Update
Merge Risk: ⚪ Minimal · up to The shared input changes have no established merge-blocking issue. Real Windows layout and gameplay validation remain outstanding, but do not currently demonstrate a regression. Adopters must rebuild against the changed native interface. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected paths preserve foreground-input restrictions, gameplay gating and resource-owned callbacks. No introduced security vulnerability was established. Downstream window routing, threading and native cleanup integration remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 12.20% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 82 functions across 19 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit taps a key with care Comment |
Multiplayer clients duplicate window key/button state and resource-owned control holds, while scripted keys and native controls use different query paths. This provides shared acquisition and ownership primitives and makes
IInputthe common key-query contract:input->IsKeyDown(FW_KEY_SPACE)works for every adapter, with native layout/device conversion inside the backend.Changes
IInputon the existingFW_KEY_*macros and uppercase US physical letter/digit positions. KeepIsKeyDown,IsKeyUp,IsKeyPressedandIsKeyReleasedas the consumer query methods. RequireIsAvailable()to report fresh acquisition state and remove the publicMapKeymethod.Win32Inputfor foreground-aware held queries and ordinary cursor operations,WindowInputfor message-based edges, andPollingInputfor frame-based edges. Games retain native cursor/control-lock policy. UI features can own a livePollingInputsource when native gameplay acquisition is paused and use exactly the same query API.KeySnapshotfor sampled edge storage and acquisition recovery. Unavailable snapshots publish no active input and reseed on recovery. Convert scriptedKeypolling to the registeredIInputsource and keep its existing callback ownership and gameplay gate.ResourceHoldsstorage so resource cleanup can release only the native holds that owner acquired.FrameworkInputTeststarget, registering the same modules in the existing framework test suite.Validation
FrameworkInputTestsbuild and unit run: 12 cases passed.builds\build.batMSVC x64 Debug input unit build/run under Wine, through the repository container wrapper: 30 cases passed, one real multi-layout case skipped because English/Russian/AZERTY layouts were unavailable. The runner's aggregate total is 31 because it counts skips as passes; that case is not claimed as verified coverage.git diff --checkpassed.Integration
This changes the native virtual interface. All adapters must rebuild against these headers; there is no compatibility layer. The review commit is
689c27139ba8e0123e33cfb54f869d27f15c7001. Merge Framework first, then move the adoption pins to the resulting merged commit before release.IInputprovider and gameplay consumers; review pin included.Native engine action-map rebinding and context priorities stay with game features. Complete chat UI replacement, relative-mouse abstractions and CEF/IME routing extraction remain separate migrations.
Summary by CodeRabbit