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

[proposal]hooks #55

Closed
hookex opened this issue Mar 25, 2020 · 4 comments
Closed

[proposal]hooks #55

hookex opened this issue Mar 25, 2020 · 4 comments

Comments

@hookex
Copy link
Contributor

hookex commented Mar 25, 2020

@brianzinn
Do you think we need to add some other hooks?

For example:

useClick

useClick(onClick: Function<event>): [ref]

https://github.com/hookex/bviz/blob/master/src/hooks.ts

useHover

useHover(over: Function<event>, out: Function<event>): [ref, isHovering]

https://github.com/hookex/bviz/blob/master/src/hooks.ts

...

@brianzinn
Copy link
Owner

Most definitely - great use of action manager and very useful! The only thing I don't know is if we should be returning a method to de-register before the mesh disposes (like useBeforeRender hook does). Hopefully babylonjs takes care of that automatically when the mesh is removed from the scene.

@brianzinn
Copy link
Owner

When the mesh is disposed (which happens when it is removed) the action manager is disposed:
https://github.com/BabylonJS/Babylon.js/blob/master/src/Meshes/abstractMesh.ts#L1603

So, under normal conditions likely wouldn't need this. One issue may arise when referencing a mesh in parent hierarchy (like a forwardRef), as it will keep registering actions if the effect is re-created. Does this look right?

useEffect(() => {
        if (ref.current) {
            // what you have now...
            const registeredAction: Nullable<IAction> = mesh.actionManager.registerAction(
               // ...
            );

            return () => {
                // called when component is unmounted?
                mesh.actionManager!.unregisterAction(registeredAction!)
            }
        }
    }, [ref.current])
}

@brianzinn
Copy link
Owner

Also, I will add it separately, but it needs a guard clause - can check the metadata. GUI have a different hover:

onPointerEnterObservable.add(() => { /* ... */ }

If you make a PR I will accept with or without those extra changes. Thanks Hooke!

@hookex
Copy link
Contributor Author

hookex commented Mar 26, 2020

Thanks for your remind. I will add this code.
Over the next few days, I will integrate my application with spring and event hooks, then submit PR.

brianzinn added a commit that referenced this issue Mar 31, 2020
…d ActionManager vs 2D `onPointerEnterObservable`. #55
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

No branches or pull requests

2 participants