Evaluate replacement for me.Entity.bounds

[parasyte]

This seems kind of tricky. There has been a lot of discussion about this in the past, mostly in #560. But it seems like maintaining the bounds rectangle separately from the entity rectangle is problematic. See this commit as an example of how it goes wrong: 104b6f4 The entity geometry was silently working for most objects, but fails for TMX Polygon and Poly Line objects. 😕

@obiot What are the reasons for creating a bounds object inside the Entity, instead of using the entity geometry itself as the bounding box?

I know the entity geometry was once used as the renderable geometry. Then that changed when renderable was broken into its own object. And then the collision box object was removed, but it appears the bounds object has just taken its place.

[obiot]

actually the preliminary reason was that me.Entiy is derivated from me.Renderable which itself derivates from me.Rect.

and for now me.Entity was set to the renderable size, since the other key question is also to figure out what what should be used to figure out if an entity is visible or not (within the viewport), as if the entity body is smaller than the sprite itself (which could happen), the entity could disappear from the screen, while part of the sprite is still theorically in the viewport.

[parasyte]

I see.

There's something else kind of concerning, on that note. Let's say there's a hypothetical entity whose renderable and shape bounding boxes are really very different. The renderable bounding box tests whether it's in the viewport, and the shape bounding box tests for collisions (simplified.) We also have a case where objects outside of the viewport are not considered for collision checks:

melonJS/src/physics/collision.js

Line 463 in a65c959

if (objB.inViewport || objB.alwaysUpdate) {

You can imagine the issue: it's possible for an entity's renderable to be outside of the viewport while its shape is inside (and intersecting with other stuff). This is why the above commit fixes collision detection; because it uses the shape bounding box to set the inViewport flag, instead of the renderable bounding box.

Should we then instead set the Entity geometry to be the rectangle union of both bounding boxes? (renderable and shape) And then go back to the original code for the inViewport test?

[obiot]

the union sounds like a good idea actually, i kind of like it :P

[parasyte]

Cool. After that. What do we do about the bounds object? (I want to remove it 😉) Renderable bounds are on renderable and shapes bounds are on body. Seems like we don't need the entity.bounds at all.....

[obiot]

fair enough, the entity rect can be used as the union of the body bounds and the renderable size :)

should be transparent anyway, as far as we keep the getBounds() function

[agmcleod]

Given how we've changed the way bounds works a bit, is this still relevant?

[parasyte]

Oh yes. Something still needs to be done. :) The spec may have to evolve with recent changes, but for sure we need something better than shoving more rectangles onto entities to get everything working...

[parasyte]

For future reference:

Entity:

• getBounds(): return a rectangle that contains all collision shapes, or this when there is no shape
• renderable: a rectangle that contains the visible part of the entity
• this: a rectangle that is a union of the above

Sprite:

• getBounds(): returns this
• this: a rectangle that contains the visible part of the sprite

[obiot]

I was looking at remove _bounds from the entity object, and directly use the entity object geometry for the bounds object, but due to the following lines, i'm not sure we can :
https://github.com/melonjs/melonJS/blob/master/src/entity/entity.js#L257-L263
https://github.com/melonjs/melonJS/blob/master/src/entity/entity.js#L241-L247

as _bounds also serves as a "cache" where all final values (like initial position + body relative position + movements + ancestor absolute position) is included.

so as far as it goes for this ticket, what I would "only" do so far is to change the following line
https://github.com/melonjs/melonJS/blob/master/src/entity/entity.js#L263
to :

this._bounds.resize(w, h);
this.resize(w,h).union(this.renderable);

[obiot]

also, are these two functions not somehow redundant :

1. UpdateBoundsPos : https://github.com/melonjs/melonJS/blob/master/src/entity/entity.js#L257-L263
2. onBodyUpdate : https://github.com/melonjs/melonJS/blob/master/src/entity/entity.js#L241-L247

and especially when you look at the parent function for UpdateBoundsPos that almost looks like the onBodyUpdate one :
https://github.com/melonjs/melonJS/blob/master/src/renderable/renderable.js#L242-L248

[parasyte]

Yes, very similar. It could probably be unified. Not sure if it will be a huge gain. ;P

The _bounds cache makes sense due to the current rectangle implementation. The problem with it is really the fact that all rectangles are positioned by their upper-left corner. Because of this, a union is likely to end up changing the position, which will have too many edge cases with unknown consequences.

To make positioning independent of a rectangle's corner, the code will probably have to evolve in the same direction @Giwayume started to explore for #99. That means, the position vector itself does not provide information about a rectangle's left or top boundaries, but instead you have to use the left and top getter methods for that; pos is a point vector, and the shape is anchored to it with the anchorPoint unit vector.

With that, changing the shape (and anchorPoint) does not affect the position. It would be easier to reason about. And at that point, the entity itself can become the boundary cache. Whether we actually need a union between bounds and renderable is still a valid question; ideally we just won't have cases where these rectangles differ by much. Secondary to that, cache the union or assume it's going to be fast enough to compute the union without needing a cache...

[obiot]

design changes, so breaking changes and 4.0.0

[obiot]

so with the two last commits, it's now possible to add a physic body to any renderable object, Sprite, container, or whatever object inheriting from me.Renderable.

which means that me.Entity could slowly go to die....as for simple case me.Sprite is enough, and for more advanced use cases (with multiple renderable), ones could use a me.Container.

[obiot]

here is an example :
06c64f7

[obiot]

that makes the whole thing much more modular/flexible, as it does not rely anymore on a specific object type for physic/collision.

[obiot]

see #1008