-
Notifications
You must be signed in to change notification settings - Fork 21
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
Controller Haptics #76
base: master
Are you sure you want to change the base?
Conversation
…store this in the cachedViewModel.
… which will require me to boot up my headset for testing.
…Type can probably be collapsed into WeaponType, but it's a larger change/testing in order to do that.
… them by different roles so that we can have different dominant/nondominant rumbles.
…r we're gripping or not
…ariables will automatically take effect.
Wow great work, I know this is a highly requested feature and I can't wait to give this a test! |
Thanks, but I can't stress enough that it's built on top of the shoulders of giants. Your work is so good, that someone like myself without any vr or game experience was able to come in and quickly figure out how to add features. This is unheard of in the real world! 👏👏👏
Yes, I only have knuckles to test with. No, because I don't actually understand how the json files are being delivered to clients. The Not sure where these are coming from. Do you manually make the release by copying these files (from somewhere other than the repo) and then zip it up to deliver to clients? If so, maybe I can take a stab at building a release script to automatically do this. On top of that, in order to get haptics to work, I added the action to
That was my additional thoughts. The
If there's no distribution script, I can start to add that in so that haptics and other json files are automatically zipped up ready for client release. We can probably built it into github actions to autorelease moving forward. Couple other notes:
|
Nice work, I haven't had a chance to look over this properly yet but from what I've seen its looking great.
|
So in theory other controller users could copy the haptics part of the knuckles(right) json file to the bindings files for the other controllers it seems, if I understand this correctly? Also, is there a reload haptic event? I think some people were asking for that at one point (I'm not demanding it just if it's something "easy" relatively speaking to the other massive work done here it might be considered at the same time). |
I’m gonna integrate with the python script early next week so that haptics works for all controllers.On Dec 8, 2024, at 6:22 AM, teddybear082 ***@***.***> wrote:
So in theory other controller users could copy the haptics part of the knuckles(right) json file to the bindings files for the other controllers it seems, if I understand this correctly?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
|
… conditional haptics debug flag.
Update manifest.py to include new haptic actions. Update makerelease to include new haptics.json
I've updated makerelease.bat to include haptics automatically. I've also added the vibration actions to the manifest.py file and refactored ScopedWeaponType to use WeaponType instead. |
I gave this a shot, but it seems this location doesn't seem to correspond to the way we would need to deal with haptics for the plasma pistol. One issue, is that WeaponHandler.Prefire doesn't actually get called during charging, only when the weapon is actually fired. For the plasma pistol, this is the "release" of the charge. To work around this, I had to hook into the main input hook and start the logic there. Needs a lot of work, and is going to be hacky and error prone, but with some finesse it should provide a decent enough experience I think. Branch for Plasma pistol: https://github.com/TheKrisSodroski/HaloCEVR/tree/plasmaPistol |
…need to track isntead.
…ude. Since we're dealing with shooting bullets, Frequency greater than 1 doesn't seem to be the right choice.
@TheKrisSodroski I want to give these changes a test but when I try and build the files I get these errors, do I need to do anything different from normal? |
Oops. Forgot to checkin the vs proj file with the new compiled files. I've updated that so it should build correctly now. You will need to update the haptics/controller json files into your halo directory. You can do this by running the makereleasse.bat file or just copying the files in the Bindings folder to the correct locations (haptics should be located next to the inject logs file). The work on haptics is basically done I think from my side. I've implemented the plasma pistol as best as I could. Having tested it for a couple hours, even though it's not perfect, it felt good enough that I forgot I was even testing it! Some issues:
Other notes:
|
Thanks for fixing that @TheKrisSodroski, I can build the files now but when actually launching Halo I get an error. I get a DirectX 9.0b needs to be installed error when launching halo, and the inject.log reads only
So i'm not sure if it's crashing somewhere whilst trying to load the hapticsConfig? The haptics.json is placed in the "Halo/VR" folder. With weapon holsters I'm happy to add haptics to them after this PR goes in, I've had a look over your code and it looks good but just left a few minor comments. |
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.
A few minor comments, but getting issues with launching the game so I can't actually test the haptics.
@@ -85,12 +86,15 @@ class Game | |||
|
|||
bool bNeedsRecentre = true; | |||
bool bUseTwoHandAim = false; | |||
bool bIsMouse1Down = false; |
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.
Could we call this 'bIsFiring'? I feel like that would be more descriptive for how it is used.
}; | ||
virtual void TriggerHapticVibration(ControllerRole role, float fStartSecondsFromNow, float fDurationSeconds, float fFrequency, float fAmplitude) = 0; | ||
|
||
//virtual IVRInput* GetVrInput() { return NULL; } |
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.
Can we remove this comment?
"startSeconds: " | ||
<< fStartSecondsFromNow | ||
<< "\n" | ||
"duration: " |
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.
It seems like theres too much indentation for these lines.
HaloCEVR/WeaponHapticsConfig.cpp
Outdated
|
||
void WeaponHapticsConfigManager::LoadConfig() | ||
{ | ||
std::string hapticsConfig = "VR/haptics.json"; |
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.
Is it possible to make the path "VR/OpenVR/haptics.json"? Just so it's in the folder with the other json files.
return plasmaPistolSettings.isCharging; | ||
} | ||
|
||
void WeaponHapticsConfigManager::WeaponFired(WeaponType Weapon) |
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.
'Weapon' doesn't seem to be used, should we be using it or can it be removed?
Logger::log << "[WeaponHapticsConfig]: Weapon fired" << std::endl; | ||
#endif | ||
|
||
if (plasmaPistolSettings.isCharging == true) |
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 there be a check to see your current weapon is a plasma pistol before this?
That issue should be resolved now. Was missing a comma in the Bindings/haptics.json file. I'll start working on your feedback this weekend. Happy testing! |
Thanks @TheKrisSodroski I managed to test these changes and wow they feel great! The Assault Rifle in particular now feels like a proper gun and not a pea shooter!
Keep up the good work I'm very excited for these improved haptics :) |
Try this, it should be some help Also, on an unrelated note, having separate json configs for each weapon got me thinking: I wonder if at some point in the future it would be worth expanding these files beyond haptics to make things more data-driven so that custom weapons can be better supported. |
Wow amazing work! Whenever this is ready for testing if someone can share the compiled version I am happy to |
I'll be implementing the review items this week. Should have another version out later this week and I'll compile it up for you. |
I just happened to compile this and it works for me, let me know if it doesn't work or wait for Kris's update. |
Just reporting back - I tried the DLL shared by slipperfish above and I can feel the haptics but they are very low power (almost like the type of haptic you would get from picking up an item rather than firing a gun). On Quest 3 + VirtualDesktop. Would playing around with the "amplitude" value in the haptics.json work potentially to increase the haptics? Figured I would share either way. Great work! |
Yes, I feel that as well. What's odd is that the amplitude is at it's maximum value according to the api documentation (between 0f and 1f, and we have 1f as the value). There's also an undocumented parameter that's necessary. Not sure what they're supposed to do, but I found a random sample on the internet that seemed to allow me to call the api.
I might try and use "TriggerHapticPulse" instead, but I believe it's being deprecated. |
Have you tried adjusting the frequency? I believe the docs say it has a range of 0-360 and from a quick scan through the files you have it at 1 by default |
I'm going to get a chance to do some additional testing today. I've added proxies for TriggerHapticPulse as well to see if that's what developers are generally using for weapon haptics. |
…settings for using pulse instead of vibration.
…tting charging to true
Hey, I've not really been keeping up with this PR. Beyond fixing the merge conflict from a recent change of mine, is this ready to merge into master? Or are there still some issues that need resolving first? |
With the holidays, it's been tough doing some last testing. I'll be getting to it this weekend to make some final changes, but I think it should be good to go early next week. |
Alright, no worries. Just wanted to make sure I wasn't neglecting a completed feature |
This commit contains the base code to have haptics working (Resolves #5).
I've checked in
knuckles_right.json
and updatedactions.json
to include the required entries for haptics. I'm unsure how actions.json (or the knuckles_right/left) is delivered to the end user, so I committed the files in here for now as an example.Haptics are controlled through
haptics.json
file, which defines the haptics for both single handed, and dual handed haptics for each weapon. If this file isn't found, haptics are disabled by using a default WEaponHaptic object with everything zeroed out.I have not tweaked the values in this json file: any help or suggestions would be appreciated.
Code recognizes whether user is left handed or right handed, and can assign different values for dominant vs non-dominant vibrations.
Notes: