Repository navigation
Entity.pos vs Body.offset #548
Description
Activity
Yep, but let me arrive at the office first , as it too long from my mobile :)
On 12 août 2014, at 09:01, Jay Oster notifications@github.com wrote:
@obiot Any ideas, here?
—
Reply to this email directly or view it on GitHub.FYI, this is an issue directly related to:
- Not including a
getShapefunction in thesettingsfor the Entity constructor - Not setting any specific body shape.
So the default body shape isn't adequate.
- Not including a
so, let me clarify this thing :
entity.posis identical toentity.body.pos, withentity.posserving as the reference one :- see https://github.com/melonjs/melonJS/blob/master/src/physics/body.js#L522 then https://github.com/melonjs/melonJS/blob/master/src/physics/body.js#L625
- I'm not saying this is correct, but i did not know how to better manage this until now, and did not have any feedback until now.
- As a matter of fact I wanted to redefine
posfor Entity and make it point to body.pos (using
``javascript
defineProperty :
Object.defineProperty(me.Entity.prototype, "pos", {
get : function () {
return this.body.pos;
},
configurable : true
});
`
this would as well prevent us from doing all the copy of the pos vector
- As a matter of fact I wanted to redefine
body.offsetis the minimum offset between body pos and all body shapes
(https://github.com/melonjs/melonJS/blob/master/src/physics/body.js#L325)- if no shape are defined for an entity, the entity default size (which is actually the renderable) is used to defined the body bounds : https://github.com/melonjs/melonJS/blob/master/src/entity/entity.js#L185
once again, i'm not saying this is correct, and for sure needs improvements :P
the other issue is that me.Entity inherit from me.Renderable, it would be much more easier if entity was just a basic container for renderables and a physic body, but it would then break all the current entity rendering code...
Isn't there also some duplication with Body inheriting from Rect, and yet that information is not used as the boundary rect?
indeed...... gosh maybe I kind of lost myself within my own code :(
i've been missing you Jason to help me proofing my code !
body.offsetis the minimum offset between body pos and all body shapes
(https://github.com/melonjs/melonJS/blob/master/src/physics/body.js#L325)if no shape are defined for an entity, the entity default size (which is actually the renderable) is used to defined the body bounds : https://github.com/melonjs/melonJS/blob/master/src/entity/entity.js#L185
This second line is actually a big problem, because it means the
body.offsetis no longer used for its intended purpose. In the second line, thebody.offsetduplicate's the entity's position within the world. If we're going to keep this, then it should instead be given the bounds of a new rectangle with position (offset from Entity)<0,0>and width/hight of the entity.Does it make sense to get rid of Entity entirely, and replace it with Container? I get the feeling that's kind of the direction we're going, but haven't yet got there.
I suppose so, but as you were saying I do not thing we are there yet... and Container is kind of a big object, is it not ?
Else what you would then recommend we do to clean my mess, between :
entity.posentity.body.posentity.body.offset(offset between thebody.posand shapes, actually for now since we only have 1 shape this equal tobody.shapes[].pos)entity.body.shapes[].pos(relative to the entity)
That's a lot of vectors... 😒
What are we gaining from the
Entityobject, right now? It just seems to have a bunch of proxy methods forBodyandRenderable.- Ideally,
Bodyhas a position, width, and height. (It does! It extends Rect) This rectangle defines its absolute bounds within the world. Bodyalso contains one or more shapes which give it substance to collide with other things. Right now it only supports a single shape. (That's fine) And as shapes are added, removed, or modified, theBodybounds should be changed. (I believe this is the case, so far? At least when shapes are added and removed.)Renderableshould be drawn relative to the body. E.g.Renderable.posis an offset fromBody.pos.- We should be able to use multiple
Renderables, all drawn relative to theBody.
I don't really see a use for
Entityat all. Body should reference theRenderables, since they are to be drawn relative to theBody. Basically,ObjectEntityshould have been renamed toBody. :)- Ideally,
I don't really see a use for Entity at all. Body should reference the Renderables, since they are to be drawn relative to the Body. Basically, ObjectEntity should have been renamed to Body. :)
gosh you are killing me :):):)
was the the idea behind having body as a "child" property of entity not was as well to lower the amount of properties for the entity object ?
what about then redefining entity pos as I was suggesting before, so that it just point to body.pos ?
defineProperty : Object.defineProperty(me.Entity.prototype, "pos", { get : function () { return this.body.pos; }, configurable : true });not sure how it works though if we do
entity.pos.x = 6;I guess that else a good old 'this.pos = this.body.pos` (after creating the body) works too ?
I like that the intention was to limit the number of properties, but if they both have the same properties, it defeats the purpose. :)
Anyway, I don't think changing the
Entity.poswill address the issue as reported. Fixing it would entail changing this line: https://github.com/melonjs/melonJS/blob/master/src/entity/entity.js#L185 to:this.body.updateBounds(new me.Rect(0, 0, this.width, this.height));
I know, creating an object to be garbage collected. But it fixes the issue with the multiplied draw positions for the default shape.
that's certainly a better way to manage it yes, and this is a one time only garbage collected object per entity creation if Tiled is not used, so I believe we can survive it :)
- added a commit that references this issue
on Aug 12, 2014 @obiot Done! And verified with: http://jsfiddle.net/Lth0bgdx/3/ (I love the melonjs-builds bucket!)
oh great, thank you Jason !
as for entity.pos, do we keep like this for now ? we really need to clear this up as well, but maybe in 1.1.1? (that would be a nice version number btw)
I don't mind leaving it as is for now.
I was trying to create a little test case for the recently reported world boundary collision issue on the forum. But I'm blocked on a different issue! I created a basic Entity with a default position, set the body velocity, and expected it to just work.
However, the renderable is drawn at position Entity.pos + Body.offset, which are both the same value (set to the initial entity position). This seems completely incorrect.
Here's the test: http://jsfiddle.net/Lth0bgdx/ Set a breakpoint on Entity.draw() in melonJS-1.1.0.js: 8945 to see it in action.