Repository navigation
inconsistent entity/body reference position between shape and tile collision #560
Description
Activity
tagging this to 1.1.0 for now, I'm going to the gym and will think about it while on the treadmill :P
I see some overlap with recent discussion in #548 (comment) which is a red flag for sure. 😃
My gut instinct is to remove some complexity here by dropping the
body.offsetentirely. The job of the body is to provide a minimum bounding rectangle for its set of shapes. Here's a picture from the wikipedia article:The body is the rectangle with the dark outline. And its
shapesarray defines all of the green shapes. You can see, there is no one "shape offset" that can be mapped to abody.offsetproperty; it's a meaningless concept. The shapes themselves have a position vector, which positions them relative to the body.As shapes are added, removed, or modified in the body, its size and position need to be updated to keep the Minimum Bounding Rectangle in sync, but those variables are all that are necessary for it.
If I'm not mistaken, removing this offset is the ultimate solution? You get rid of that
[body.pos + offset]step, which makes the[body.pos + shape.pos]step useful again.one thing i'm not clear about, is where we store the position of an entity, as logically speaking since it's the "position of the enitity" i would be inclined to use the entity position vector, but since body is meant to be the entity body, and since body is used for collision, it would also make sense to use the body position. As when using the latter, i suppose that anyway virtually, the body position is always 0,0 from a shape point of view (based on the above picture)
I think you got it right, in your last statement. The body's position vector will default to
<0,0>. But in the case of a resize (adding shapes) it's possible that the body position will have to change, relative to the entity position.In other words, the body position is only relative to the entity (which has no shape; just a position relative to its container).
Oh, and with that, I can finally have my entities with central anchor points! :D
If I add a circle as the only shape, I expect the circle to have only 2 variables: its position, and a radius. The position is the center of the circle. So if I add a circle to an entity's body with
pos = <0,0>andradius = 10, then I will get an entity whose position is the center point of a 20x20 px circle! Exactly what I want.ok i will rewrite the body to comply with the following :
entity.posto be the reference position of the entitybody.posis relative to the entity position and default to0,0- shapes positions is (are soon) relative(s) to the body position
- body is a rectangle representing the smallest bounding box containing all shapes
I think shapes will actually have to be relative to the entity position, too. The body position is really just a means of extending the body shape to the left and above the entity position (or to the right and below, but that's weird!)
yes, you are right.
What should i do however with the
entity.getBoundsfunction :getBounds : function () { return this.body.getBounds(); },
I suppose I shall also add a private
_boundsobject and ensure it keeps being updated with the entity pos and the body collision box information ?this is used not only by the collision code, but as well in other part of the main game loop when for example checking if the object is within the viewport, etc...
In the case that
body.pos != <0,0>then you will have to recalculate the entity's absolute bounds each time it moves. Whether it makes more sense to calculate this at write-time (entity has moved) or read-time (someone wants to know the entity area) is a good question, though.My gut suggests that write-time will be used fewer times per frame. But if you go that route, user code then has to be mindful about updating the entity bounds if it directly manipulates the position. I think this is a good trade off, because physics engines sometimes have a similar limitation/requirement.
- added a commit that references this issue
on Aug 18, 2014 @parasyte did a first commit, but had to stop for the day :
e058057it's kind of working, but something is off when changing a shape default pos, maybe if you have some free time during the day you can have a quick pass on it, as i'm sure you'll spot a couple of things i wrong, or could do better :)
ah! just tracked down a bug :
https://github.com/melonjs/melonJS/blob/master/src/shapes/poly.js#L207whatever the
posvalue, when calling the updateBounds, the bounds position is always the minimum x/y vertex from the polygon ... I guess the function was not that simple !Shall I translate the final bounding rect by the initial polygon position ?
return this.bounds.translate(this.pos)Good find! Should be more like this? (Offset all vertices by the position vector)
var x, y, right, bottom, px, py; x = right = this.pos.x; y = bottom = this.pos.y; this.points.forEach(function (point) { px = point.x + this.pos.x; py = point.y + this.pos.y; x = Math.min(x, px); y = Math.min(y, py); right = Math.max(right, px); bottom = Math.max(bottom, py); });
That looks kind of ugly, actually. ;) Your translation idea might be better. But then you have to change the initialization slightly, anyway:
var x = Infinity, y = Infinity, right = -Infinity, bottom = -Infinity; this.points.forEach(function (point) { x = Math.min(x, point.x); y = Math.min(y, point.y); right = Math.max(right, point.x); bottom = Math.max(bottom, point.y); }); // ... return this.bounds.translate(this.pos);
The use of
Infinityprevents bugs when all vertices are greater than zero, or all less than zero.11 remaining items
ok guys, i found the "issue", it was not really in melonJS, but more related to the fact that when manually changing a object position, we need to call
updateBounds()(see my last commit)any idea or to improve this, or is this an acceptable "limitation" ?
- the collision detection could also call the entity updateBounds function after the triggering the collision callback ?
- adding a call at the beginning of the body update function could also automatically fix this ?
I think that can be an acceptable limitation. Right now we tend to have the users use updateMovement() with collidable entities.
Otherwise, I think we'd have to implement an observer of some kind.
I'm not sure i really like it actually, as we already have to call me.body.update() as well (the former updateMovement) for regular movement calculation, OR now updateBounds if manually changing the position.
In 1.2.0 it should be all automatic though as we should also take in account collision response to make entities "solid" or stay on top of a collision shape.
On 20 Aug 2014, at 20:05, Aaron McLeod notifications@github.com wrote:
I think that can be an acceptable limitation. Right now we tend to have the users use updateMovement() with collidable entities.
Otherwise, I think we'd have to implement an observer of some kind.
—
Reply to this email directly or view it on GitHub.I already called it an acceptable limitation 3 days ago! But if it's not, then we should consider a new Vector2D extension that implements
xandyas setter functions to automatically correct the bounds.Fine for now, and i see it as a temporary transition while waiting for 1.2.0 :)
Beta3 later today :)
@agmcleod i would say, except if they are ready to be committed or if you can add them very fast, let's keep them for 1.1.1 as improvements or 1.2.0 with other changes as discussed with Jason.
I think it's really time to freeze the 1.1.0 branch now :)
Yeah I'll wait then. I'd rather get it done correctly. Can fix anything that is actually preventing something from working in a 1.1.1
yep, closing this ticket then (finally!)
- added a commit that references this issue
on Sep 1, 2021
Close one, and a new one will open.... :/
Following discussion here :
https://groups.google.com/forum/#!topic/melonjs/QW0zNC7scu8
I'm not sure when to fix this one, as having this mix implementation for shape and tile also complicate things, and since there is somehow a workaround, I would be tempted to defer this one to 1.2.0 ? (where we will have a full shape collision implementation for both entities and the world)
Description :
current entity vs body vs shape is confusing and lead to some inconsistencies
Current impact and issues :
if a shape default offset is different from 0, collision with other shapes won't properly work (the offset will be taken in account 2 times, shifting the collision detection by that amount). Collision with tiles is however working properly.
Workaround :
using anchor point similar effect can be accomplished, for example :
Instead of offsetting the y position of the rectangle to set the collision box at the bottom of the entity renderable
Set anchor point so that the renderable base is aligned with the bottom of the body :