object animation keeps cycling when out of viewport

melonjs / melonJS

a fresh, modern & lightweight HTML5 game engine

https://melonjs.org

MIT License

5.86k stars 644 forks source link

object animation keeps cycling when out of viewport #205

Closed obiot closed 11 years ago

obiot commented 11 years ago

Just noticed that : https://github.com/melonjs/melonJS/blob/master/src/renderable/sprite.js#L555 And i'm wondering if this is something we want or not. personally I would say no, and I believe we forgot to add the test on inViewport and it has been added :)

parasyte commented 11 years ago

Good catch!! Definitely change the this.visible check to this.inViewport.

I believe doing as little as possible for objects outside of the viewport is important for performance reasons. And I still hate that objects are updated even when not in the viewport ...

obiot commented 11 years ago

actually I'm having second thoughts on this one :) :) :)

I was just thinking that when using a onComplete callback, this might break thing (although it was for sure not working as well at the time of the visible flag. A typical use case would be to display a "dying" animation and destroy the object once the animation is over.

fix could be to check, on top of inViewport if a callback is set ?

parasyte commented 11 years ago

That's a good point ... But at the same time I wouldn't assume that an animation with a callback always wants to update outside of the viewport. :)

It might be time to start thinking about a new general boolean property to update outside of the viewport (default=false). Used within the object update loop, you could remove the visible/inViewport check entirely (and other similar checks elsewhere)

// update our object
var updated = (obj.inViewport || obj.alwaysUpdate) && obj.update();

Just an idea?

obiot commented 11 years ago

I like it !

parasyte commented 11 years ago

@obiot Got this working quite quickly! I also updating the Upgrade Guide for it. (And went through the wiki changing links to the new melonjs.github.io locations, while I was there.)

obiot commented 11 years ago

hehe, we definitely need to use that "assign" button when one of us start to work on a ticket, because I was working on it as well :)

few differences on my side :

I actually set alwaysUpdate to true in the renderable object
forced the value as well in ScreenObject (though not necessary)
replaced also the visible boolean test by inViewport in `AnimationSheet.update()

on my side however I have an issue with the entities that stopped being updated when outside of the viewport (despite of the alwaysUpdate flag) :(

anyway, I left to go !

So since we reached end of development for this version, what about posting a beta build on the forum ?

obiot commented 11 years ago

thanks for the wiki as well !

obiot commented 11 years ago

@parasyte

ah interesting, it has the same issue I was observing on my side :

open the platformer example
scroll the screen to partially hide (irst tile) of the first platform (with the blob on it)
wait for the blob to go out of the viewport

And the blob never comes back :( I did not manage to find out why, and I had the same code than you and it was not making any sense..

obiot commented 11 years ago

...... ok forget about my last comment : https://github.com/melonjs/melonJS/blob/master/examples/platformer/entities.js#L218

I cannot believe I wasted one hour debugging this stuff and completely missed that !

parasyte commented 11 years ago

I actually used the enemies in platformer example to verify that the patch worked! With alwaysUpdate defaulting to false, I expect objects to stop updating immediately after leaving the viewport. That would make the blob never come back into view. ;) But yeah, because of that check for !inViewport, it would still act the same.