Add Rumble Manager and Demo

[lunarcloud]

Supercedes #250 - previous Godot 3 era implementation.
Makes it possible to add rumble to standard functions like #191 suggests (adds a couple)

[lunarcloud]

GDLint failure is due to static variables not yet being implemented ( Scony/godot-gdscript-toolkit#242 )

[lunarcloud]

FYI - I didn't intend to change anything in assets or hands - godot just forced that on me.

[Malcolmnixon]

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: ***@***.***>

[lunarcloud]

Okay, fixed the extraneous changes, pre-squashed and rebased

[lunarcloud]

Going to add doc comments and deal with @Malcolmnixon 's suggestions

[lunarcloud]

We cleaned up the documentation, added poke and button rumble examples, added a "hand aware rumbler" helper

[lunarcloud]

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.
But this all works and is fun as-is, so I might stop meddling until after a review from Bastion

[Malcolmnixon]

I was able to test out the gdlint fix by installing from git-hash using:

pip3 install git+https://github.com/Scony/godot-gdscript-toolkit.git@ad525088fc3d4633a8577d37da1def8addfb32f4

The result was:

.\rumble\rumble_manager.gd:37: Error: Definition out of order in None (class-definitions-order)
.\rumble\rumble_manager.gd:40: Error: Definition out of order in None (class-definitions-order)
.\rumble\rumble_manager.gd:44: Error: Definition out of order in None (class-definitions-order)
.\rumble\rumble_manager.gd:47: Error: Definition out of order in None (class-definitions-order)
Failure: 4 problems found

The fix is to move the static var to be the last var defined - just before the is_xr_class() function.

[lunarcloud]

I was able to [...]
The fix is to[...]

Yup, just re-ran it myself and fixed that issue.

[lunarcloud]

Rebased on top of the latest changes (and the gdlint update is now live)

[lunarcloud]

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)

[lunarcloud]

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)

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.

[DigitalN8m4r3]

@BastiaanOlij holy Macaroni this is still not merged

[lunarcloud]

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.

[BastiaanOlij]

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.

1. The singleton implementation is flawed. When you switch between two scenes, there will briefly be two rumble managers, with the outgoing one remaining the singleton breaking the system. We can change this in three ways:
   a. We can make this a true singleton and autoload the script globally. But this causes problems if the user doesn't add the autoload script. That is something that can be tested on the rumbler logic.
   b. Don't use a singleton but take the same pattern as with the PlayerBody, e.g. use our lookup helper functions to find the rumble manager attached to the XROrigin3D. But this may not be an option when the rumble originates from the environment.
   c. Add the manager to a list when added to the scene tree, remove it from the list when it's removed, have the last added entry in the list be the singleton, keep all data in static variables so they are shared amongst the managers and/or add gates so managers ignore calls when they are not the active singleton.

2. This is probably inherited from the Godot 3 days but we can't hardcode the actions. This needs to be usable with customised action maps.
   a. I would change rumble_on_button.gd to work for a single event. I would thus also rename it to rumble_on_action.gd, have one property for the action name, have one property for the rumble event. Now it can work with any action, including trigger, grip, etc. When you want to trigger rumbles on multiple buttons, you just add multiple child nodes to the controller, one for each action. I would also add an enabled property (default on), so things can be enabled contextually.
   b. HAPTIC_ACTION shouldn't be hardcoded. We don't currently have a way to interrogate the loaded action map, I really should make something for that, because ideally we'd get a list of active haptic actions per controller from the system. That will have to wait until we redo the action map as a more global system. For now I think it's enough for this to just be a property on the manager. We can look at enhancing this when multiple haptic actions become a thing in a later update.

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.

[Malcolmnixon]

Touch-rumbling appears to have been broken with the last round of changes. What's going on is that the XRToolsPoke node has grown new pressed and released events which are fired when the poke touches and releases an object... however these events only report the object being poked.

The pressed event is wired to the XRToolsRumbler.rumble_hand method; which tries to find the hand associated with the node provided... but the node is the object being poked - and has no relationship to the hand to rumble.

One option would be to add an XRToolsRumbler.rumble_pointer which takes an XRToolsPointerEvent. This is also fired from XRToolsPoke and contains the pointer and target. The rumbler can use the pointer to track down which hand to rumble. This would also allow it to work with laser-pointers if desired.

[lunarcloud]

@Malcolmnixon we worked together to rebase and then un-break.

@BastiaanOlij

1. Addressed via solution a.
2. unaddressed, but it makes sense to fix

[Malcolmnixon]

Indeed - we'll want to address the 'haptic' action name at some point soon.

[BastiaanOlij]

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

[zacharysarette]

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!!!