-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor and enhance bHapticsManager architecture - (v1.1.0) #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Refactored `BHapticsConnection` into a singleton for better state management and added event-based device handling. Improved thread safety and logging in initialization and shutdown processes. Replaced static `DeviceEventHandler` with an instance-based implementation supporting `IDisposable`. Enhanced `DeviceRegistration` with thread-safe methods, a `ClearAllRegistrations` function, and improved logging. Upgraded `ModernBHapticsWorkerThread` with dynamic device handling, better thread management, and detailed diagnostics. Added intensity boosting for weak signals in `LegacyCompatibilityLayer` and reduced submission intervals for responsiveness. Modularized patch application in `HapticPlayerPatches`, `HapticMethodPatches`, and `TorsoMapperFix`. Improved error handling and compatibility with the modern bHaptics SDK. Streamlined mod initialization in `bHapticsManager`, added lazy haptics initialization, and ensured proper shutdown handling. Updated `bHapticsManager.csproj` to support additional Resonite paths and removed unused references. General improvements include better maintainability, enhanced modularity, and updated documentation.
|
this is a MASSIVE overhaul and fixes SEVERAL oversights i did not see before, SHOULD be entirely usable now! |
|
doo it to it CoP! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the bHapticsManager architecture to version 1.1.0, implementing significant improvements to connection management, device handling, and thread safety. The changes transform the codebase from a static-based approach to a more modern singleton pattern with proper lifecycle management and event-based device hot-plugging support.
Key Changes:
- Converted
BHapticsConnectionto a singleton pattern with instance-based event handling for device connection/disconnection - Modularized Harmony patches into separate
ApplyPatches()methods for better organization and maintainability - Enhanced
ModernBHapticsWorkerThreadwith dynamic device state tracking, improved shutdown handling, and intensity boosting for weak haptic signals
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| bHapticsManager.csproj | Updated project file to support multiple Resonite installation paths and use $(ResonitePath) variable consistently for all references |
| bHapticsManager.cs | Refactored initialization flow with proper world synchronization, lazy haptics initialization, and structured shutdown handling |
| TorsoMapperFix.cs | Added ApplyPatches() method for modular patch application and cleaned up inline comments |
| RemoteHapticSource.cs | Added DummyWorldElement helper class to provide proper parent reference for world elements |
| Properties/AssemblyInfo.cs | Updated assembly description and copyright format |
| PositionMapper.cs | Added nullable annotation to return type of ConvertDotPointsToMotorArray() |
| ModernBHapticsWorkerThread.cs | Implemented IDisposable, added device state tracking, improved timing with stopwatch-based scheduling, and enhanced shutdown coordination |
| LegacyCompatibilityLayer.cs | Added intensity boosting for weak signals, increased buffer duration, and reduced submission intervals for better responsiveness |
| HapticPlayerPatches.cs | Modularized patch application and simplified patch methods by removing verbose comments |
| HapticMethodPatches.cs | Added ApplyPatches() method and changed logging from Msg to Debug for diagnostic output |
| DiagnosticPatches.cs | Reorganized into nested class structure and updated logging to use Debug instead of Msg |
| DeviceRegistration.cs | Added ClearAllRegistrations(), RegisterDevice(), and UnregisterDevice() helper methods with thread-safe access |
| DeviceEventHandler.cs | Converted from static to instance-based implementation with IDisposable support and simplified event handling |
| BHapticsConnection.cs | Converted to singleton pattern, added device connection events, and improved shutdown process with detailed error handling |
Comments suppressed due to low confidence (1)
bHapticsManager/ModernBHapticsWorkerThread.cs:125
- These 'if' statements can be combined.
if (_workerThread != null && _workerThread.IsAlive) {
if (!_workerThread.Join(TimeSpan.FromSeconds(2))) {
bHapticsManager.Warn("Worker thread did not stop gracefully, interrupting...");
try {
_workerThread.Interrupt();
if (!_workerThread.Join(TimeSpan.FromSeconds(1))) {
bHapticsManager.Warn("Worker thread did not respond to interrupt");
}
}
catch (Exception ex) {
bHapticsManager.Error($"Error interrupting worker thread: {ex}");
}
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
nalathethird
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be adjusted due to comments on your suggestions please.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@nalathethird I've opened a new pull request, #9, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: nalathethird <36301692+nalathethird@users.noreply.github.com>
…timing Co-authored-by: nalathethird <36301692+nalathethird@users.noreply.github.com>
Co-authored-by: nalathethird <36301692+nalathethird@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fix race condition and static field access in WorldFocused event handler
Remove unused _devicePoints dictionary and _lock object from DeviceRegistration
|
legit just making sure i dident miss anything. im not THAT confident on this update cause it was CONSIDERABLY rushed to my standards... so thats why im having this clanker check my work. 💀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public ModernBHapticsWorkerThread(InputInterface inputInterface) { | ||
| _inputInterface = inputInterface; | ||
| _cancellationToken = new CancellationTokenSource(); | ||
| _hapticPointsByDevice = new Dictionary<LegacyBHaptics.PositionType, List<HapticPointData>>(); | ||
| _deviceKeys = new Dictionary<LegacyBHaptics.PositionType, string>(); | ||
|
|
||
|
|
||
| _workerThread = new Thread(WorkerThreadLoop) { | ||
| Priority = ThreadPriority.Highest, | ||
| IsBackground = true, | ||
| Name = "ModernBHapticsWorker" | ||
| }; | ||
| } |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two constructors (lines 45 and 55) with different initialization logic. The first constructor creates a thread but never starts it (no _running = true or Start() call), while the second does. The first constructor appears to be unused code since Start() would need to be called separately but there's no evidence of that pattern in the codebase. Consider removing the unused constructor or documenting why both patterns are needed.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@nalathethird I've opened a new pull request, #14, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@nalathethird I've opened a new pull request, #15, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@nalathethird I've opened a new pull request, #16, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@nalathethird I've opened a new pull request, #17, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@nalathethird I've opened a new pull request, #18, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@nalathethird I've opened a new pull request, #19, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@nalathethird I've opened a new pull request, #20, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
that bitch is REMOVED. its causing loops. ill fix these myself thanks. lmao |
Refactored
BHapticsConnectioninto a singleton for better state management and added event-based device handling. Improved thread safety and logging in initialization and shutdown processes.Replaced static
DeviceEventHandlerwith an instance-based implementation supportingIDisposable. EnhancedDeviceRegistrationwith thread-safe methods, aClearAllRegistrationsfunction, and improved logging.Upgraded
ModernBHapticsWorkerThreadwith dynamic device handling, better thread management, and detailed diagnostics. Added intensity boosting for weak signals inLegacyCompatibilityLayerand reduced submission intervals for responsiveness.Modularized patch application in
HapticPlayerPatches,HapticMethodPatches, andTorsoMapperFix. Improved error handling and compatibility with the modern bHaptics SDK.Streamlined mod initialization in
bHapticsManager, added lazy haptics initialization, and ensured proper shutdown handling. UpdatedbHapticsManager.csprojto support additional Resonite paths and removed unused references.General improvements include better maintainability, enhanced modularity, and updated documentation.