Skip to content
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

Feature/set grab target #124

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

InfiniteLee
Copy link
Contributor

I have run npm run test:machinima, and this patch passes all machinima tests: yes

Optionally explicitly specify which entity to grab when grab-start is fired. This is useful for things like "spawners" (imagine a box spawner that when you grab, spawns a new box in your hand, leaving the original spawner in place).

@wmurphyrd
Copy link
Member

The way this works is that you modify the incoming button press event with the new target before it goes to super-hands, right?

I'm trying to think of how to incorporate this as a fully documented and supported api feature so that others could put it to use as well, and my concern is that this conflicts with the general goal of super-hands to abstract away any dealings with low-level controller and collider events.

Would you be open to exploring a refactor that accomplishes this entirely within super-hands though custom reaction components and/or new methods in the super-hands core?

@InfiniteLee
Copy link
Contributor Author

This is how it's used in hubs right now: https://github.com/mozilla/hubs/blob/master/src/components/super-spawner.js
Effectively, when entity A (e.g. a spawner, that does not have a grabbable component so it can't actually be grabbed) receives a grab-start event from an entity with a super-hands, it emits the grabStartButtons event with targetEntity (set to Entity B) in the details on the same super-hands entity. There is some added complexity with hubs because we create that "Entity B" during the initial grab and have to wait for it to be fully initialized.

I could see a path where this spawner behavior is added as a reaction component, but I also see this functionality being used in other ways. There is general usefulness in being able to retarget what you are trying to grab. E.g. how "Job Simulator" lets you remotely grab objects on the ground by hovering your hand above them (could be defined as a larger grab sphere/cylinder around the object that retargets the hand on grab). So perhaps its a "retarget" component?

Regardless, while I am happy to explore other approaches (I do agree this current approach does feel weird, but it was the most obvious given the current API), I am somewhat concerned about trying to have too much functionality all in one place. I feel that the more you try to encompass all the functionality at a high-level, the more likely you'll start to run into interference with other libraries and making it more difficult for developers to add their own more complex functionality. (E.g. should it really be a super-hands component that handles the above described "spawner" behavior? Probably not. Similar concerns with adding in the ownership transfer from networked-aframe.)

All that said, I really don't know what the right approach is, but would be happy to talk about any ideas you might have.

@netpro2k
Copy link

The part that feels most weird about the current implementation to me is that we are having to syntehsise an event to do a programatic grab. Instead I would expect to just be able to call a funciton on a super-hand to have it programatically grab soething. This gets at the core of what targetEntity is trying to do and makes this implementation far less weird. I also imagine this might be generally useful in other cases where for whatever reason we want to put something in the players hand.

I would also second the idea that super-hands should strive to do 1 job well, managing the interactions between a set of hands and a set of objects. Everything else (like networking) should be handled by other components that work with the core library. I have noticed a general trend in the aframe community of trying to do too much "out of the box" and though it makes it slightly easier to get started it actually makes things harder once you move past the "slap together a quick glitch demo" phase.

@wmurphyrd
Copy link
Member

It seems the API gap here is the ability for a reaction component to modify the super-hands state. i.e. your spawner could have a component that listened for 'grab-start', but then told super-hands to register the newly created entity as the carried entity instead of itself.

This would also aid in the networking solution as the grabbed entity needs the ability to clear the active grab when ownership is lost.

@netpro2k I appreciate this input. It's tempting to cram in networking because the built-in reaction components (which were originally intended to be more template/inspiration than workhorse) have grown so complex that asking library users to recreate them to add in networking seems like too much. Perhaps the API change above could also allow for external components to act as an intermediary such that networking could be easily integrated without replacing grabbable.

Do you have plans to publish standalone components to enable networked interactions with super-hands?

@InfiniteLee
Copy link
Contributor Author

We do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants