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

feat: sensor support integration #141

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

feat: sensor support integration #141

wants to merge 9 commits into from

Conversation

Neosoulink
Copy link
Collaborator

@Neosoulink Neosoulink commented Oct 11, 2024

Summary

This PR comes with the sensor intersection support!

Enable users to use custom colliders as sensors:

<CuboidCollider
   :args="[10, 3, 0.5]"
   :position="[0, 3, 3]"
   activeCollision
   sensor
   @intersection-enter="onIntersectionEnter"
   @intersection-exit="onIntersectionExit"
/>

Logs

  • Fix debug lines bounding-box availability
  • Add new vNode extended types
    • TresVNodeObject
    • TresVNode
  • Add a TresObject to collider components
  • Add sesnor demo
  • Implement intersection feature

Record

Screen.Recording.2024-10-11.at.11.35.00.PM.mov

Documentation addition

Sensor

The Sensor feature allows events to be triggered when there's an intersection or in other words, when the collider is traversed by another collider.

The traversed Collider (or the collider that will trigger events), is the sensor and should set the activeCollision and sensor properties to true.
By passing the above properties, the collider will no longer be affected by the physics law and will now start triggering the following intersection events:

  • @intersection-enter: When another collider starts to traverse the sensor
  • @intersection-exit: When another collider leave the sensor

I.e:

<RigidBody type="fixed">
  <CuboidCollider
    :args="[10, 3, 0.5]"
    :position="[0, 3, 3]"
    activeCollision
    sensor
    @intersection-enter="onIntersection1Enter"
    @intersection-exit="onIntersectionExit"
  />

  <CuboidCollider
    :args="[10, 3, 0.5]"
    :position="[0, 3, -3]"
    activeCollision
    sensor
    @intersection-enter="onIntersection2Enter"
    @intersection-exit="onIntersectionExit"
  />
</RigidBody>

This PR will resolve #107

@Neosoulink
Copy link
Collaborator Author

⚠️ IMPORTANT NOTE:

I'm getting the unsupported properties warning when using custom colliders with intersection events:

[Vue warn]: Extraneous non-emits event listeners (intersectionEnter, intersectionExit) were passed to component but could not be automatically inherited because component renders fragment or text root nodes. If the listener is intended to be a component custom event listener only, declare it using the "emits" option. 
  at <Anonymous name="cuboid-sensor" args= Array(3) position= Array(3)  ... > 
  at <RigidBody type="fixed" > 
  at <Physics debug="" > 
  at <App>

Screenshot 2024-10-11 at 11 40 11 PM

I understand this is coming from my emitIntersection utility, where I'm trying to get the correct collider component:

export const emitIntersection = (
  source: sourceTarget,
  target: sourceTarget,
  started: boolean,
) => {
  const collisionType: collisionType = started ? 'enter' : 'exit'
  const colliderNode = (source.object as any)?.__vnode?.children?.[1]?.children?.find((child: any) => child?.component?.exposed?.instance?.value === source.context.collider)

  colliderNode?.component?.emit?.(`intersection-${collisionType}`, { source, target })
}

I don't have a better solution right now to solve the issue, but I'll try to dig in to see if I can improve the integration.

cc: @JaimeTorrealba

@alvarosabu
Copy link
Member

I understand this is coming from my emitIntersection utility, where I'm trying to get the correct collider component:

export const emitIntersection = (
  source: sourceTarget,
  target: sourceTarget,
  started: boolean,
) => {
  const collisionType: collisionType = started ? 'enter' : 'exit'
  const colliderNode = (source.object as any)?.__vnode?.children?.[1]?.children?.find((child: any) => child?.component?.exposed?.instance?.value === source.context.collider)

  colliderNode?.component?.emit?.(`intersection-${collisionType}`, { source, target })
}

I don't have a better solution right now to solve the issue, but I'll try to dig in to see if I can improve the integration.

Hi @Neosoulink this is probably happening because those new intersection-* emits are not declared in the emit definition here https://github.com/Tresjs/tres/blob/af704c1e8e146608d34543325af67b0b43703f20/src/components/TresCanvas.vue#L79

Is it colliderNode.component a Tres component?

@Neosoulink
Copy link
Collaborator Author

Neosoulink commented Oct 18, 2024

Is it colliderNode.component a Tres component?

Hi @alvarosabu,
No, but it uses the Tres component to get the Collider component, colliderNode.component is the current Collider component triggering the intersection event.

Note: Probably next week I'll create new types instead of using any

@alvarosabu
Copy link
Member

Is it colliderNode.component a Tres component?

Hi @alvarosabu, No, but it uses the Tres component to get the Collider component, colliderNode.component is the current Collider component triggering the intersection event.

Note: Probably next week I'll create new types instead of using any

Hi @Neosoulink if you are in control of the Collider Component, I think you need define the emits using defineEmits https://vuejs.org/api/sfc-script-setup#defineprops-defineemits to get rid of that error.

@Neosoulink
Copy link
Collaborator Author

Hi @Neosoulink if you are in control of the Collider Component, I think you need define the emits using defineEmits https://vuejs.org/api/sfc-script-setup#defineprops-defineemits to get rid of that error.

Okay, let me try this ✅

@JaimeTorrealba
Copy link
Member

@Neosoulink in order to use the same "system" that I use, you should emit the events in the parent, the RigidBody which is a 3DGroup...

I know it is just a draft (of end documentation) But if you check here:
https://github.com/Tresjs/rapier/blob/main/docs/components/collider.md

The solution I came was to emit the event in the parent.

The BaseCollider is not a Tres component and doesn't have the same vNode properties (in fact is just a <slot /> which is empty), but the RigidBody does.

You can check here how I handle it: https://github.com/Tresjs/rapier/blob/main/src/utils/collisions.ts#L8

Except that, this is a nice work, thank you.

PS: Don't forget to submit the documentation, is missing.

@Neosoulink
Copy link
Collaborator Author

@JaimeTorrealba I see now, and it's also joining the @alvarosabu note...
But Sensor events should be triggered from the Collider (yes, it's a <slot />).

In that case, we have to define emits directly in the project,
Something like here: https://github.com/Tresjs/tres/blob/af704c1e8e146608d34543325af67b0b43703f20/src/components/TresCanvas.vue#L79

@Neosoulink
Copy link
Collaborator Author

Hi @alvarosabu @JaimeTorrealba, I just pushed the fix for the properties warning issue.

But I'm not sure if I should add documentation in this PR as well, I mean this PR will get bigger

Otherwise, this PR is ready for review

@@ -86,5 +98,7 @@ onUnmounted(() => {
</script>

<template>
<slot></slot>
<TresGroup>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason to use a <TresGroup> here is what @JaimeTorrealba mentioned here #141 (comment) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about this, a <TresGroup /> makes no sense here... Maybe add a "dummy" object like a "TresObject3D".

Something that can emit an event (a Tres-vue component) but also something that makes sense when you inspect the hierarchy tree in Three.js.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @JaimeTorrealba, I used a Group to match what was done in the RigidBody component,

But in reality, both of them should use Object3D instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I mean, we use TresGroup a lot on cientos for some of the abstractions, so I'm fine either way.

@JaimeTorrealba what does the dummy object would look like?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion is exactly that a "TresObject3D" (we generally used a dummy object3D in instanceMesh) it would solve the issue, also we can add (optionally) some :name="ColliderId" attribute, this way if you look into the threeJs scene makes more sense than a plain TresGroup, right? Or any other data/value that identify as a collider. All these are just suggestions., @alvarosabu.

@Neosoulink IDK if I misunderstand your last comment, but no <RigidBody> receive slots, it makes sense to be a TresGroup

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there's no misunderstanding,
I was just clarifying the topic of using an Object3D.

@alvarosabu
Copy link
Member

Regarding

Hi @alvarosabu @JaimeTorrealba, I just pushed the fix for the properties warning issue.

But I'm not sure if I should add documentation in this PR as well, I mean this PR will get bigger

Otherwise, this PR is ready for review

It's expected to have documentation in the same scope as the PR whenever there is a new feature or bugfix. It's also a way reviewers can understand the code changes quicker as they were a new user.

@alvarosabu alvarosabu added feature needs-documentation p3-significant High-priority enhancement (priority) labels Oct 28, 2024
@Neosoulink
Copy link
Collaborator Author

It's expected to have documentation in the same scope as the PR whenever there is a new feature or bugfix. It's also a way reviewers can understand the code changes quicker as they were a new user.

Got it

@Neosoulink
Copy link
Collaborator Author

I added a minimal senor documentation

cc: @alvarosabu @JaimeTorrealba

started,
)

if (started) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small improvement here.
I know this is the approach of R3F but if you pay attention to my collisionEmisor I handle the state (started true or false) in the function itself, this way we don't have to put an extra if in Physic and also don't have to repeat the emitIntersection function. Also keep consistency in the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it was also my design choice to be able to trigger the starting and the ending.
But yeah, let me correct this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the change @JaimeTorrealba

Copy link
Member

@JaimeTorrealba JaimeTorrealba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can improve this in the future in order to accept automatic colliders too.

But the implementation is stable, thanks for this one

@Neosoulink
Copy link
Collaborator Author

I think we can improve this in the future in order to accept automatic colliders too.

But the implementation is stable, thanks for this one

I spend time looking for a good way to implement it.

Let's say you have your RigidBody and add the sensor property. Will the sensor events (Enter & Exit) be triggered when a collider transverses any collider inside that RigidBody?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature needs-documentation p3-significant High-priority enhancement (priority)
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Sensors
3 participants