inViewport broken since moving SpriteObject to renderable property

[parasyte]

@obiot

Some code within core.js expects to act upon objects that are defined as sprites. The move away from ObjectEntity inheriting from SpriteObject means this code no longer works as expected. One example is the inViewport property. (Also the dirtyRegion feature.)

I suppose a fix would involve a new property isEntity for objects inherited from ObjectEntity, right? I don't really like the idea of having all of these class-specific properties. Another option is a type property which is an array of strings identifying the object class hierarchy.

e.g.

[
    "me.Rect",
    "me.ObjectEntity",
    "me.LevelEntity"
]

With that, the if (this.isSprite) can be replaced by if (this.type.indexOf("me.ObjectEntity") >= 0) Iterating arrays like this is kind of slow, though. :(

[melonjs]

hmmm good point ..

what about just simply move the inViewport property to me.Rect ? I would not really be shocked by doing so, since furthermore entity will then or not draw their renderable component based on their inViewport status, and if they actually have a renderable component.

[parasyte]

As long as we can reasonably expect all "viewable" things to inherit from me.Rect, that sounds like a quick and easy stopgap measure! And anyway, the calculation that determines whether something is in the viewport uses me.Rect.overlaps() :p

[melonjs]

@parasyte all viewable things are supposed to inherit from the new me.Renderable object, which itself inherits from me.Rect so yes :)

note however that a few ones are not yet inheriting from it, like the infamous HUD object/items ;)

[parasyte]

There's a me.Renderable class! I didn't know about it until just now. :)

Since me.ObjectEntity is no longer a "viewable" object, the visible flag should also be moved to me.Rect ? Both flags (visible and inViewport) are duplicated in me.ObjectEntity and me.SpriteEntity

[melonjs]

@parasyte

yep, I added me.Renderable while working on the inheritance stuff between entity and sprite ;)

But I was thinking, shall me.ObjectEntity not rather inherit from me.Renderable as well ? after all it's also some kind of optionally renderable elements (except that it then renders itself another renderable element.

[parasyte]

@obiot True! The same could be done for me.ScreenObject too. And get rid of the fake me.Rect inheritance on that object:

• me.ScreenObject inherits from me.Renderable
• init() calls this.parent(new me.Vector2d(0, 0), 0, 0);
• reset() does var vp = me.game.viewport; this.set(vp.pos, vp.x, vp.y); if this.addAsObject === true
• Remove the rect property
• Remove the getRect() method

[melonjs]

@parasyte wow, when I just read that, I just thought “what it does not inherit from me.Rect ! ” :)

[parasyte]

This is becoming a little more work than I imagined when I discovered the bug. ;) But better to get it all cleaned before release.

[melonjs]

@parasyte
the last one was also bit more tricky that what I initially thought, as just changing Object to me.Renderable broke the library "compilation" (since me.Renderable was declared after me.ScreenObject). So I had to reorganize a few stuff to make it working.

Shall we also move isPersistent into me.Renderable ?

Also, we should also rename the entity directory to something else (like component?), as right now it does not make sense anymore considering what's inside. I could do it now, but then I won't be able to later merge the ticket #8 into the main branch :)

Ultimately, it would be nice to re-organize the whole library using something like require.js, as we would not need to do this kind of hackish ordering requirement (i guess that would deserve its own ticket)

is it me, or the more we do, the more there is to do ? :):):)

[parasyte]

@obiot Wow! core.js is much smaller now! :D

I think isPersistent would be very useful in me.Renderable; it's technically supported for all objects, even though me.ScreenObject and me.HUD_Object are the only classes that define it.

The entity directory was starting to bug me, too. Actually it was more the camera.js file living there. I always felt it belonged in the video directory.

On require.js; I don't have many opinions about it. But I did recently read an interesting article by Mishoo (Uglify-JS author) on the subject. Have a look: http://lisperator.net/blog/thoughts-on-commonjs-requirejs/ I get the impression there's no magic bullet for the dependency issue; either you stick to loading everything in a specific order, or deal with callback horror!

But yeah, I think the problem with refactoring is that it's a lot of work for little visible gain from a user's perspective! ;)

[melonjs]

@parasyte wow, very good article, even made me laugh a couple of times :):):)

so I then will move isPersistent to me.Renderable (but a bit later, as for now I have to work on my paid job), and then return to my TexturePacker Stuff !

[parasyte]

@obiot That pretty much solves the ticket! (yay! teamwork!)

The last patch was a massive change that also impacts some established API. (realwidth and realheight properties on TMX layer objects are replaced by width and height instead! The old width/height in tile measurement is now cols/rows) This affected the platform example, but the fix was super easy (and nice!)

I added this info to the Upgrade Guide.

[melonjs]

@parasyte wow, yep great work you did on this one as well I must say, although I'm a bit scared by the very last one as I'm seeing a loooootss of changes :)

[melonjs]

@parasyte

about this me.ImageLayer.offset is now me.ImageLayer.pos :

I'm not sure I'm really happy about this one to be honest, as I had in mind to use pos (later) to allow positioning an initial image somewhere that at the (0,0) origin position. think about a repeative background much less higher than the current level height that you want to be bottom aligned ?

[parasyte]

@obiot I see. Would that be expected to work with the ratio property? I mean, if you set pos after the me.ImageLayer is created, it seems to have the same effect, right? You get an initial position, then it's adjusted as the viewport scrolls.

[melonjs]

@parasyte actually with or without, could be a fix background (some "header" or "footer" background with some fancy elements) or a repeatitve/parallax one. But yeah, for sure i'll give it a try :)

Else I just reviewed the changes you made and that you renamed width/height to cols/rows and realwidth/realheight to width/height.... and I don't know... I mean having the layers itself in mind, width and height are then coherent compared to the property/variable named used by TIled, on the other hand, this is not coherent with other width/height property used in the engine (viewport widht/height to start with).

in the mean time, this is also internal stuff, and not really visible for the "end-user' so....

[parasyte]

@obiot Do feel free to revert these changes. I felt it made sense to make the me.ImageLayer.width/height measured in pixels. And if that's the case, me.TMXTileLayer and friends should also measure width/height in pixels.

It is a big change and prone to error, though. :(

[melonjs]

@parasyte no no, actually having width (or height) being the width in pixels, fully makes sense at least compared to other element/components of melonJS (entites width are in pixel not tile), it's just that effectively big changes are prone to error :)

makes me things that while we are at breaking stuff, what about all these setTile, getTile function you once "complained" about ?

on my side, i keep progressing on the texturepacker stuff, I should soon have some beta version with animation support :)

[parasyte]

@obiot Oh yeah! I had forgotten about getTile and setTile. I think there's still a ticket for that. On texture packer: sounds great! I should probably familiarize myself with the tool. Currently I don't know anything about it. ;)

[melonjs]

@parasyte

Texturepacker is quite good actually, or more specifically what is good is that you don't really have to worry about your sprite size, etc... just drag&drop them into the software and basically it will gives you back a big PNG and a corresponding atlas with each sprite size and position.

Of course the big big advantage is that you end up with a power of 2 size optimized texture (instead of having all your single PNGs being sized up to match the next pow2 value and waste, specially on mobile devices, considerable amount of memory for nothing), that's quite good, specially when targeting cocoonJS. I don't how much you played with it so far, but checking the console log is kind of scary :)

[melonjs]

i'm closing this one, as I think we are done with it :)