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(body): add isTrigger option #79

Closed
wants to merge 3 commits into from
Closed

feat(body): add isTrigger option #79

wants to merge 3 commits into from

Conversation

age2pierre
Copy link

Hello,
I did this small fork to add the "sensor" option to bodies, the original feature-request is referenced here => schteppe#146 and all the credit for the actual code goes to => schteppe#424 ;)
I've committed the dist/ folder to test without publishing the package, but I can drop it from the PR if needed.

@marcofugaro
Copy link
Member

Thanks for the PR @age2pierre, I think this feature could be useful for many users.

I've committed the dist/ folder to test without publishing the package, but I can drop it from the PR if needed.

Please exclude it from the PR, otherwise, there will be a lot of conflicts if we build on master 😬

Some considerations:

  1. I think isTrigger would be a better name for it. sensor is used in a 2d game engine environment, while 3d game engines use trigger: triggers in Unity, triggers in Unreal. Bullet calls them GhostObjects 🤷‍♂️.
  2. It would be better to have a trigger.html example instead of adding the code in the collisions.html. The trigger would need to be rendered with a wireframe style, just like wireframeMaterial but green or yellow:
this.triggerMaterial = new THREE.MeshBasicMaterial({ color: 0x00ff00, wireframe: true })
  1. The example should show the usage of at least the beginContact and endContact events. Unity has also the OnTriggerStay event, but maybe it's better to do that in a future PR since it's a new event for cannon.

src/objects/Body.ts Outdated Show resolved Hide resolved
@age2pierre
Copy link
Author

@marcofugaro I've updated based upon your review and rebased upon master.
I've kept the demo still very minimal, tell me if you fancy something fancier :)

@age2pierre age2pierre changed the title feat(body): add sensor option feat(body): add isTrigger option Mar 5, 2021
@marcofugaro
Copy link
Member

Thanks! I adjusted the demo a little bit, opened a new PR with also your commits since I wasn't allowed to edit this one: #83

@marcofugaro marcofugaro closed this Mar 6, 2021
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.

2 participants