ScreenObject.floating is difficult to change.

[parasyte]

I wanted to change my ScreenObject's floating property to false, so that me.game.viewport.shake() would work. Turns out, it's not very easy to set this property!

The visible and floating properties are forcefully set after onResetEvent() is called, meaning I have to change these properties in a deferred function call from onResetEvent : https://github.com/melonjs/melonJS/blob/master/src/state/state.js#L151-L153

These properties should probably be set directly on the ScreenObject class definition, as seen here: https://github.com/melonjs/melonJS/blob/master/src/state/state.js#L106

[obiot]

to be honest, I would like to stop using ScreenObject this way, and keep it for state management only.

it's easy enough to add a basic object for anything is generally done with screen objects, and is way more in line with what we do in melonJS.

[agmcleod]

Working on a smaller project this weekend vs the small platformer I've been doing, i do prefer having the update method of the screen object be the more top level update that I call if i need to customize, over using another object. Just easier for me to remember where I put what in regards to keybindings, and setup for the current screen.

I do think the idea of a screenobject having a floating point or coords for that matter kind of odd.

[petarov]

Wouldn't it make more sense to have me.ScreenObject inherit from me.ObjectContainer? Ideally one would (probably) want to bind objects (sprites, HUD, etc.) to the current scene and the ScreenObject seems like the place to do so, since it is related to the current game state.

[parasyte]

In that case, ScreenObject would just replace me.game.world as the root ObjectContainer. Since you're right about the ScreenObject representing a scene, in the sense of typical 2D APIs. And that's exactly what ObjectContainer implements.

[petarov]

I was thinking more in a kind of Scene Graph direction impl., where the ScreenObject that would inherit from ObjectContainer would actually be added as a child to the me.game.world container. It should already be possible to have nested ObjectContainers from what I see in container.js? When the ScreenObject is being destroyed it's container with all it's children will be removed from me.game.world. That should also trigger the .destroy() call for any subsequent ObjectContainers the ScreenObject container contains.

p.s. the word container is used 7 times in what I wrote above :)

[agmcleod]

Is there a direct case where you would still add objects to the world container over the screen object though? I'm not sure if I see the benefit of doing it that way vs what Jay suggested, as it definitely adds more complexity & confusion.

[petarov]

yes, that's a good question. Maybe adding objects to the world container would only make sense for some sort of persistent objects? That might eliminate the need for .isPersistent flags, but I guess it does increase confusion, nevertheless.

[agmcleod]

Something to keep in mind with it as well is that when you switch levels, objects not set isPersistent=true get removed, so we'd want to make sure that is handled properly with the screen object, and any other objects that would be on the world itself, if we were to change that behaviour.

[obiot]

Would Screen Object replace me.game.world, or would it replace me.game ?

me.game is already some sort of container today, most of the function we had before have disappeared in (moved to) the Object Container, and I would then rather see it that way, as the main entry for the main loop.

what is sure however is that I would like to find something to clear, replace the following code : https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/state/state.js#L205-L218 https://github.com/melonjs/melonJS/blob/master/src/core.js#L1478#-L1531

[parasyte]

@obiot Exactly. Making ScreenObject the root node of the scene graph is the way to go. In this way it's no longer a special class, but it's actually the complete game loop. And rather than optionally adding it to the scene, it's already the root of the scene. In this sense, it should never default to screen coordinates (floating) because its children will inherit that behavior.

It makes perfect sense from a cleanup point-of-view.

As a subclass of ObjectContainer, each ScreenObject will maintain its own list of child nodes, significantly reducing the complexity of cleaning objects between state changes. However, it will be more complicated to persist objects across state changes. So we should be prepared for that.

ScreenObject can retain its onUpdateFrame method, which is the entrypoint for RAF. Except it will call this.update() and this.draw() instead of those methods in the me.game namespace.

[petarov]

@parasyte :+1:

As for persistent objects, why not keep the me.game.world but just make it private, i.e., not directly accessible? Then persistent objects can be added to the parent element of the ScreenObject container, e.g., this.parent.addChild()

Another way would be to have a root ScreenObject that contains all further ScreenObjects. This, however, would just mimic the current me.game.world behavior.

[obiot]

actually i kind of like the idea of keeping me.game.world reference to the actual world, as me.game could then still contains other useful function or reference like the current TMX level, global transformation, etc...

anyway, I'm personally more planning this small code revamping for 1.0.0 (i don't know if we all agree on this) in order to avoid any major regression on what should just be a maintenance version.

[parasyte]

No confusion there! Such changes are definitely 1.0.0-only.

Effectively, me.game.world could be a reference to the ScreenObject I think that's fine. We will need some way to reference it globally, anyway.

But that's really the whole point of this conversation; if the ScreenObject is turned into the root node, then this whole ticket becomes obsolete! :)

[parasyte]

Moved to 1.0.0 for further discussion.

[obiot]

i was giving more thoughts on this one and i'm not sure that having screenObjects as the root node is a good idea.

the main reason being for me that it would then add some complexity, as at each state change the world root node would need to be changed, which would then bring with it some questions and annoying stuff to manage (like what do we do with persistent objects, etc...).

right now I'm personally very happy with me.game as it's almost empty and just contains a few reference to some useful object (camera, current Level) and an entry point for the RaF function.

and although we all do not agree, I still maintain that we should stop offering the possibility to add screenObjects to the main loop, as this can be very easily done through an additional objects, and would allow us to further clean the API (and the mess that we have right now with screenObjects)

[agmcleod]

I agree. I tend to use the screenObject#update a fair bit in my games. But i guess doing so can cause confusion on using the draw.

[obiot]

@parasyte what is your opinion ?

that's actually that hack-ish part that i don't like : https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/state/state.js#L104-L155

[parasyte]

@obiot Let's get rid of it! It has caused a lot of little bugs, and if we can't agree to make the ScreenObject the world container, then at least we can agree that the ScreenObject should not be a child of it.

[agmcleod]

Sounds good to me. Time to refactor my games, buahahahahah :D

[obiot]

my god I actually hate this decision, don't get me wrong I'm very happy we are getting rid of it and ultimately to have a cleaner code, but yes, as Aaron just said it means refactoring for existing games, and countless questions/complains on the forum as well :P

[agmcleod]

Honestly, I think we need to make that tough call. We already run into support issues with it. We need to then ensure the migration guide dictates what needs to be done.

[parasyte]

Make it so. </iron fist of doom>

[obiot]

@parasyte @agmcleod

and this one is done, what is interesting now is to see what is remaining in the screenObject class.... almost nothing :P

i'm closing this ticket, if you guys beleive that we should still consider merging screenobject and container, I'd rather then open a new one.