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

Adding onHit event for renderList items #33

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Adding onHit event for renderList items #33

wants to merge 6 commits into from

Conversation

IanTyf
Copy link
Collaborator

@IanTyf IanTyf commented Jul 7, 2021

Added capability for renderList items to have onHit callbacks. Can now treat these objects as buttons, or use the callback function to manipulate the object itself. Also supports grouping objects together, which would be helpful when having a complex object like a human figure or a desk.

Added a demo scene to show how it works.

hittestDemo.mp4

@IanTyf IanTyf requested a review from snowymo July 7, 2021 05:59
@snowymo snowymo added the enhancement New feature or request label Jul 8, 2021
@snowymo
Copy link
Owner

snowymo commented Jul 8, 2021

Awesome work. I may need some to test if it works for multiuser when we synchronize all the input events via the server. For example, when user A is moving the controller to hit the object, what will user B see from the other side?

@snowymo snowymo self-assigned this Jul 8, 2021
@IanTyf
Copy link
Collaborator Author

IanTyf commented Jul 13, 2021

At the moment user B won't see any changes to the object that is hit by user A. I'm not exactly familiar with how inputs are synchronized here so it's not implemented yet. But I thought this could still be a useful tool for some of the demos, even though it's only local to the user.

@snowymo
Copy link
Owner

snowymo commented Jul 14, 2021

You mean user B can see user A's controller moves but no consequence of the hit?

@IanTyf
Copy link
Collaborator Author

IanTyf commented Jul 20, 2021

I just made some changes and now the hit events should be synchronized correctly, aka user B will see the hit events when user A hits something.
Mostly the change is to add an extra parameter for the input event handler functions since I need to know which user is performing the input action, in order to do hittesting/raytracing from the corresponding controller.

@snowymo
Copy link
Owner

snowymo commented Jul 20, 2021

Glad to know it is working. Can you put a video here?

Also, I quickly went through your changes. It seems now we know the id of the input event. So functions like OnPress has id as parameters. However, I don't know if ids is necessary. It seems to me that ids contains buttons states for each user. I remember we have buttonState recording that https://github.com/futurerealitylab/immersive-presentation/blob/hitTest/js/render/core/renderListScene.js#L19. Are they close?

@IanTyf
Copy link
Collaborator Author

IanTyf commented Jul 21, 2021

Oh yes I forgot to change the variable name. ids is a terrible name for what it's representing. It contains button states for each user's controllers, but the state is simply "pressed", "dragged", or "released", as those would be sufficient for this situation, whereas I think buttonState holds the button state for EVERY button on the left and right controller of the local user, so it's like an array of true and false. In other words, ids is more similar to window.isPressed, window.isDragged, etc, but for every user.

I'll change the variable name and maybe restructure it a bit and push again, as well as put up a video once I got home!

@snowymo
Copy link
Owner

snowymo commented Jul 22, 2021 via email

@IanTyf
Copy link
Collaborator Author

IanTyf commented Jul 28, 2021

hitTestSyncDemo.mp4

@snowymo
Copy link
Owner

snowymo commented Aug 21, 2021

@IanTyf Would you mind resolving the conflicts so I can accept this PR?

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

Successfully merging this pull request may close these issues.

2 participants