schteppe / p2.js

JavaScript 2D physics library
Other
2.64k stars 330 forks source link

Added a factory method on Line that creates a line between two points #168

Closed jakobmulvad closed 9 years ago

jakobmulvad commented 9 years ago

This was something I used in my own project to build line segments between two points. I'm not sure if this has any interest but I figured I would make a PR none the less :)

Also in this PR:

schteppe commented 9 years ago

Awesome! I totally see the need for this and it goes along the same line (no pun intended) as box2d.

The only thing I wonder about is the inertia calculation. Since the line doesn't have to cross its local origin any more, it needs another inertia term to compensate. On the other hand, if the line is supposed to be static, the inertia doesn't matter. Was this your intention? Then maybe we should rename it to StaticLine instead, and always put infinite inertia. What do you think?

jakobmulvad commented 9 years ago

This is probably because i don't understand the inertia calculations. I didn't realize the lines needed to cross their own origin. Is this a common requirement for all shapes? My intention was to use it as static lines yes, but to me making a StaticLine shape doesn't sound like a shape but a body (since mass is a body property).

Perhaps a better solution is to add a function on Body much like fromPolygon() that takes an array of points and builds line shapes bewteen them? Then we can have each line cross it's own origin and use an offset with addShape() to place it correctly. Would this solve the problem with inertia?

schteppe commented 9 years ago

@jakobmulvad they don't need to cross their own origin, but the inertia just needs to be calculated with such things in mind. See https://en.wikipedia.org/wiki/Parallel_axis_theorem The p2.Heightfield is a good example of a static shape. It behaves like a "rotationally static shape", just sets the inertia to infinity and don't care about the rest. This is totally okay in my mind :)

I think that .fromPolygon is bad from a code perspective that we should move away from. Would be much better to have a Polygon shape that does the fromPolygon magic behind the scenes. This way, the Body and Shape API is more uniform for all shapes, and new users would understand the body creation pattern better.

Regarding the point array - it would be better to have a StaticLineChain shape for that, like in Box2D. This way, the shape could index all points in a QuadTree for quick AABB intersection lookup. It would also solve the "ghost vertex" problem https://www.youtube.com/watch?v=oP7ZLeyc9HU

This said, maybe the best of all would be to make a StaticLineChain shape? If you want to make a single line, just feed it with two vertices.

function StaticLineChain(vertices){
    ...
}
StaticLineChain.prototype.computeMomentOfInertia = function(mass){
    return Number.MAX_VALUE;
};
jakobmulvad commented 9 years ago

I see. I agree that the fromPolygon pattern is not nice.

The problem with making a new StaticLineChain shape is that you would need to add collision calculations against each other type of shape (I started down this path but gave up when i realised this)

schteppe commented 9 years ago

@jakobmulvad True. More work, but probably can't add QuadTree acceleration without it. Maybe the intersection stuff can be shared anyway somehow.

jakobmulvad commented 9 years ago

But yeah that would be the best solution.

However, I'm afraid that's beyond my level of expertise and scope of this PR. So maybe we just close this PR saying we want to solve it with a new shape later on? You can add it on a wishlist if you have that somewhere (or create an issue for it here on github)

schteppe commented 9 years ago

Will do. Thanks!