Skip to content

Conversation

@nearnshaw
Copy link
Member

No description provided.

@nearnshaw nearnshaw requested a review from a team as a code owner February 19, 2025 20:01
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 19, 2025

Deploying adr with  Cloudflare Pages  Cloudflare Pages

Latest commit: e448293
Status:🚫  Build failed.

View logs

@robtfm
Copy link

robtfm commented Feb 27, 2025

perhaps rather than requiring the trigger area to have a MeshCollider alongside it, we could add mesh and collision_mask fields to this new component (and maybe rename it to MeshTrigger).

then we can specify the default collision layers differently.

we could also update the gltf reader to specify that _trigger name means it's a MeshTrigger, in parallel to current the _collider -> MeshCollider?

that would avoid all changes on existing components / behaviours and would give us good default behaviour for the new component.

Copy link

@robtfm robtfm left a comment

Choose a reason for hiding this comment

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

  • we should ensure that triggers are triggered even if the object "passes through" the target in a frame. this would require special handling for tweens. or explicitly say that this doesn't trigger (which would be a shame)
  • we should report the position of the trigger as well (at first point of contact) for the case when the entity with the trigger has a tween
  • if an entity has collision=physics and trigger=player set to the same mesh, should we fire the trigger when player touches? from the example it appears so, this should probably be explicitly mentioned either way

Comment on lines +64 to +66
- `position`: The position of the entity that triggered the trigger
- `rotation`: The rotation of the entity that triggered the trigger
- `scale`: The scale of the entity that triggered the trigger
Copy link

Choose a reason for hiding this comment

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

i suggested using a transform directly here, but i'm not sure we can actually do that since we don't have a PbTransform message type...

Copy link
Member Author

Choose a reason for hiding this comment

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

The same would apply to the fields triggeredEntityPosition and triggeredEntityRotation that I just added
If we change the 3 above fields for a Transform, then we should do the same with the triggered entity

nearnshaw and others added 2 commits August 6, 2025 10:53
Co-authored-by: Mateo Miccino <mateomiccino@gmail.com>
Signed-off-by: Nicolas Earnshaw <nearnshaw@decentraland.org>
Signed-off-by: Nicolas Earnshaw <nearnshaw@decentraland.org>
Copy link
Member

@kuruk-mm kuruk-mm left a comment

Choose a reason for hiding this comment

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

Great job 🙌

nearnshaw and others added 5 commits October 1, 2025 13:31
Co-authored-by: Pravus <pravusjif@gmail.com>
Signed-off-by: Nicolas Earnshaw <nearnshaw@decentraland.org>
Co-authored-by: Pravus <pravusjif@gmail.com>
Signed-off-by: Nicolas Earnshaw <nearnshaw@decentraland.org>
Co-authored-by: Pravus <pravusjif@gmail.com>
Signed-off-by: Nicolas Earnshaw <nearnshaw@decentraland.org>
Co-authored-by: Pravus <pravusjif@gmail.com>
Signed-off-by: Nicolas Earnshaw <nearnshaw@decentraland.org>
Co-authored-by: Pravus <pravusjif@gmail.com>
Signed-off-by: Nicolas Earnshaw <nearnshaw@decentraland.org>
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.

5 participants