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

Added support and integration with 'ammonext' physics. #10

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added support and integration with 'ammonext' physics. #10

wants to merge 3 commits into from

Conversation

alkavan
Copy link
Contributor

@alkavan alkavan commented Mar 23, 2017

It seems ammo physics is not so easy to integrate correctly, what do you think about physics branch?

  • Added example Sphere component PhysicsSphere
  • Added example Plane component PhysicsPlane

* Added example Sphere component PhysicsSphere
* Added example Plane component PhysicsPlane
Copy link
Member

@sasha240100 sasha240100 left a comment

Choose a reason for hiding this comment

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

Thanks for suggestion! See my comments


import {MeshComponent} from '@whs/core/MeshComponent';

export class PhysicsPlane extends Plane {
Copy link
Member

Choose a reason for hiding this comment

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

Is there a need for PhysicsPlane, PhysicsSphere ... ?
I mean that they are used only once in code.

Copy link
Member

Choose a reason for hiding this comment

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

BasicComponent was created to show how to organize a first simple component (similarly to React).

constructor(params = {}) {

params.modules = [
new PlaneModule({
Copy link
Member

Choose a reason for hiding this comment

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

That will overwrite all modules array. I'd recommend using this.applyModule() after calling super(...).

this.applyModule(
  new PlaneModule({
    mass: 0
  })
);

Copy link
Member

Choose a reason for hiding this comment

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

But, anyway, that's just my suggestion of how to do it properly. For very specific component you can ignore that. But if you want to make your components universal - use the code above.

@@ -28,6 +28,10 @@ const config = {
},
{
test: /\.(glsl|frag|vert)$/, loader: 'glslify-loader', exclude: /node_modules/
},
{
test: /\.wasm$/,
Copy link
Member

Choose a reason for hiding this comment

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

It seems we don't use wasm in this example. If you want to use wasm build - read this and get those files.
You need ammoloader.js and ammo.wasm files.

new WorldModule({
   wasmBuffer: 'ammo.wasm',
   ammo: 'ammoloader.js'
})

See this code.

@alkavan
Copy link
Contributor Author

alkavan commented Mar 24, 2017

I've made a commit being more explicit about everything, better for example.
However I did not realize the Ammo.js integration (with THREE) is still in this state.

I saw your repository at https://github.com/sasha240100/ammojs-build-files
I see now how I missed some parts. My whole point was to use the WebAssembly example.
Will do another commit with proper wasm integration using your builds on that repo.

@sasha240100
Copy link
Member

@alkavan There is no need for wasm (no extra optimization on loading time yet)

* And improving physics demo with light and jumping ball.
@alkavan
Copy link
Contributor Author

alkavan commented Mar 26, 2017

@sasha240100 I've updated the PR. Removed wasm dependency, and Improved the demo.
I'm gonna try loading the wasm on my project first, I see it's still too experimental perhaps.

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