-
Notifications
You must be signed in to change notification settings - Fork 79
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
Add Rumble Manager and Demo #557
Add Rumble Manager and Demo #557
Conversation
GDLint failure is due to static variables not yet being implemented ( Scony/godot-gdscript-toolkit#242 ) |
|
Yeah, for the time being you want to commit only the stuff you touch. When
4.2 comes out in another week or so, we'll let it "upgrade" everything...
after trying to get as much stuff merged as possible because it will cause
merge-hell.
…On Mon, Oct 30, 2023, 8:23 PM Sam Sarette ***@***.***> wrote:
FYI - I didn't intend to change anything in assets or hands - godot just
forced that on me.
—
Reply to this email directly, view it on GitHub
<#557 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOHAG66KEUYSAOIA26XNTDYCBAIHAVCNFSM6AAAAAA6W2RTPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBWGI2DMOBVGI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
ca946cd
to
2f3a343
Compare
Okay, fixed the extraneous changes, pre-squashed and rebased |
b55821a
to
fe98d2c
Compare
Going to add doc comments and deal with @Malcolmnixon 's suggestions |
3b823ec
to
b83f990
Compare
We cleaned up the documentation, added poke and button rumble examples, added a "hand aware rumbler" helper |
I think there's an argument to be made that the rumbler not the rumble events definition should be in charge of defining which hand (or both) gets rumbled. |
I was able to test out the gdlint fix by installing from git-hash using:
The result was:
The fix is to move the static var to be the last var defined - just before the |
b83f990
to
f3e880f
Compare
Yup, just re-ran it myself and fixed that issue. |
169bde4
to
1bdf7ef
Compare
7f5b04a
to
e2e051e
Compare
Rebased on top of the latest changes (and the gdlint update is now live) |
I discussed this with @Malcolmnixon in person, but... I'm feeling like the Rumble Event resources shouldn't include the intended hand. We found that in practice, 90% of things want to rumble EVERYTHING or a specific hand (or head (XRCamera?), chest, other XRNode3d) |
d5bdfa4
to
31b6b65
Compare
Okay. No more hand enum inside events. Events are just duration/pausable/magnitude. tracker that gets rumbled is just going to be the XRInterface tracker string, which can be grabbed from any XRNode3D (like left/right XRController3D). So if you really want to rumble some special gear, go wild. |
@BastiaanOlij holy Macaroni this is still not merged |
Sorry I haven't made the meetings to try and push this feature's inclusion. Let me know if there's any issues needing attention. It's the last feature I feel is missing for my XR usage. |
Indeed, long overdue to get his merged and I am sorry it's taking me this long to have an in depth look. That said, more people really need to have a look at this and see if they agree with this approach and leave feedback on this PR. We can't just merge this because we need a rumble approach. People need to understand it. In broad strokes I really like this approach, the rumble manager is a neat way to combine rumbles from different sources and potentially future proofs this. It is really hard to predict what's in the future here, especially because I don't think the industry knows yet, but we can look at what's out there already in the XR space, both in more advanced haptics on controllers and things like haptic vests. My current assumptions here is that we're going to start seeing haptic solutions that work akin to spatial audio. Something in game triggers a haptic pulse at a given location, feedback from pulling a trigger, feedback from a stick you're holding hitting surface, feedback from something hitting the body in a certain location, feedback from a pulse happening near the player. This information is sent to the rumble manager, the rumble manager detects which haptic feedback devices are in range of the pulse, and triggers various haptic feedback events on those devices. That at this point in time our solution is more directly structured around the output, that's a fine starting point. We're not at this future point yet and there is no certainty that is where this will head. But we're at the right base. Technically there are two things we need to change in the approach IMHO for this to be mergeable.
I think there is much more we can do in future updates, but I think if we handle the above, we have a solid foundation. |
Touch-rumbling appears to have been broken with the last round of changes. What's going on is that the The One option would be to add an |
941d07b
to
15feeb0
Compare
@Malcolmnixon we worked together to rebase and then un-break.
|
15feeb0
to
d98e613
Compare
Indeed - we'll want to address the 'haptic' action name at some point soon. |
d98e613
to
ae5ce03
Compare
ae5ce03
to
fd4dc81
Compare
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.
LGTM, I think this is ready to be merged and it can evolve from there. Sorry it took so long to get it over the line
Nice! Now we're ready to RUMBLE!!! |
Supercedes #250 - previous Godot 3 era implementation.
Makes it possible to add rumble to standard functions like #191 suggests (adds a couple)