melonjs / melonJS

a fresh, modern & lightweight HTML5 game engine
https://melonjs.org
MIT License
5.87k stars 645 forks source link

melonJS 1.1.0 #480

Closed obiot closed 10 years ago

obiot commented 10 years ago

Hi Guys.

Time to decide what we want to have in the next version of melonJS.

Here is my wishlist for the big ones :

On my side, the next stuff I would like to start working on is to add a copy of Pixi in the vendors folder and replace basic object with the Pixi ones (like shapes for examples) in order to gradually allow to use the pixi renderer. This would then allow to have more (if i can say) "standardized" object for later work on collision.What do you think ?

Who is then in for what ? :)

let's make version 1.1.0 rocks the world !

ellisonleao commented 10 years ago
DuckyCrayfish commented 10 years ago

Good list! I've got a pretty good idea for a goal for the modulation xD

agmcleod commented 10 years ago

When i worked on the inheritance changes/style that jay proposed, I really started to realize how our deeply nested inheritance is an issue. Would love to see a more modular approach on classes referencing other objects as supposed to inheriting all the time.

parasyte commented 10 years ago

@agmcleod :+1: The deep inheritance is a very big issue. And I imagine it contributes greatly to the object size "limit" of 30 properties. That should be pretty easy to instrument, too.

parasyte commented 10 years ago

@agmcleod I just started up the platformer example and manually counted the number of properties on the me object ... there are _over 100!_ All accesses to our global namespace use the slower hash table lookup. :disappointed: A lot of this can be fixed with better namespace organization.

For example, over half of the properties directly relate to handling TMX maps, and of those, only 10 are functions which should be grouped under me.tmx: like me.tmx.Layer, me.tmx.Tileset, etc. The majority of TMX properties are just constants which should be scoped variables, or grouped under me.tmx.CONST if they need to be globally referenced by something (?)

agmcleod commented 10 years ago

Yeah definitely need to separate responsibility there. Can potentially have medium/middleman object or something for working entities with maps as well.

setthase commented 10 years ago

@parasyte - problem you mentioned should be fixed (or partialy fixed) if melonJS will be structured using CommonJS or AMD modules

obiot commented 10 years ago

Hi all,

The end of the month is nearly there, and here comes the time to decide what we do with the 1.1.0 release, so far, what we had :

so the big question being : Do we freeze the feature list, and attempt at delivering the above by end of July, or is there anything else we should include and eventually delay the release to make it happen ?

(note: the new 1.24 version of Howler will be integrated of course)

aaschmitz commented 10 years ago

Hi @obiot a suggestion: it would be interesting to rename other objects in the same way done with me.ObjectEntity (to me.Entity) like the source code cleaning/re-organization?

me.SpriteObject -> me.Sprite me.ObjectContainer -> me.Container me.ObjectSettings -> me.EntitySettings???

The same time renaming / separating some source files:

base.js -> renderable.js sprite.js -> separate and create the "animationsheet.js"

Thanks!

obiot commented 10 years ago

for the second ones, most certainly, but for me.SpriteObject and the others, I'm less sure to be honest. I'm not saying i'm against, but me.Entity was motivated by the fact that we redesigned how the entity object is, but for these ones there is not major changes.

As for me.ObjectSettings, if I would change it would be for me.TiledSettings

@parasyte @agmcleod what is your opinion ?

agmcleod commented 10 years ago

I like idea of renaming them, as it keeps it more simple. Sprite or SpriteObject is fine by me, but i think ObjectContainer to just Container would be a nice change.

Also renaming/splitting out the animationsheet for sure, as that bugs me :).

The ObjectSettings could be something like TiledObjectSettings since they are called objects in Tiled itself. TiledSettings i feel would be confusing, as that could be more global for a tiledmap itself.

parasyte commented 10 years ago

We should probably keep the settings objects private (there's another one for the particle emitter). Having them referenced in documentation makes them attractive to bad design patterns.

I've seen code in the forum that actually manipulates the object directly, and then passed it to the entity constructor. Of course, this is not great if the constructor ever holds a reference to the settings object for any reason, because the object can just be manipulated again later when the next entity is created. This kind of bug is very hard to find.

agmcleod commented 10 years ago

Was going to suggest that as well actually, but I was not sure if it needed to be publicly accessible.

agmcleod commented 10 years ago

For the 1.1.0 release, i would kind of like to get the canvas renderer refactor in at least. But i think i should change around the me.Font class first, to instead call a function on the renderer instead of accepting a canvas context. To better prep it for the webgl implementation.

obiot commented 10 years ago

good good. so I'll rename them all :)

Object Renaming :

me.SpriteObject -> me.Sprite
me.ObjectContainer -> me.Container
me.ObjectSettings -> me.TiledObjectSettings

@parasyte about making me.TiledObjectSettings private, I would actually agree, but the issue is that it is also somehow used for documentation purpose....

Source files renaming/splitting:

base.js -> renderable.js
sprite.js -> separate and create the "animationsheet.js"
obiot commented 10 years ago

@agmcleod having the new canvas renderer refactor would be great. that would allow to make the new API official, even if for this particular release we flag the webGL support as experimental.

so as soon as the canvas renderer is stable, please do not hesitate to push it back into master :):):)

aaschmitz commented 10 years ago

Nice :+1:

obiot commented 10 years ago

damn I really like the shape that melonjs is taking; SAT, quadtree, webgl, it's really becoming somehow something much more professional :)

parasyte commented 10 years ago

At the very least, let's mark the settings object with @const and @private in documentation, so that it is clear that users should not modify it.

obiot commented 10 years ago

about me.TiledObjectSettings, ultimately it should even be moved somwhere else, under that TMX sub-namespace that was proposed in another ticket here. So something like `me.TMX.ObjectSettings

obiot commented 10 years ago

FYI, I just went through all the opened tickets for 1.1.0 and moved all the ones where nothing were started to 1.2.0.

Feel free to pick one back from the 1.2.0 milestone if you get excited about one of them, but I think that if we can finish half of the ones for 1.1.0, that will be good already !

agmcleod commented 10 years ago

Alright fixed up the conflicts and merged in my branch :D.

Question, should i remove the parameter for the renderer type since it's still canvas only?

me.video.init('screen', me.video.CANVAS, 800, 600, true, 'auto')
obiot commented 10 years ago

awesome ! Amazing addition Aaron !

obiot commented 10 years ago

I think we definitely need to roll-out a beta with this one, with so many changes behind the scene :P Also, if the WebGL one is there but not complete i would rather put "alpha/experimental support" in the documentation, rather than "to be done" (what do you think?)

btw: can you also make sure that the wiki is reflecting these changes ?

agmcleod commented 10 years ago

I put all my changes in the upgrade guide already.

For the WebGL, i'm wondering if we should just flat out ignore it in the docs. Because there isnt much there yet for it to even be considered an alpha.

Once I get some of the functionality in there, then yes i think it's a good idea :)

agmcleod commented 10 years ago

On the docs, the Error classes need to be fixed.

docsproblem

parasyte commented 10 years ago

Hmmm... template issue? I hate jsdoc...

agmcleod commented 10 years ago

Yeah. Kami seems to use YUIDoc. Not sure if it's any better, but noticed it when its block comments were conflicting :)

parasyte commented 10 years ago

I started on a brand new documentation platform, because all the available ones suck. Currently mine sucks too. ;P But here's an article I wrote about it: http://blog.kodewerx.org/2014/05/trials-of-documenting-javascript-source.html And here's an example output: http://parasyte.github.io/node-capstone-docs/

agmcleod commented 10 years ago

I like your post. But i think it's a mess in JS land because writing java-doc like documentation has been going away, especially in dynamic languages. While RDoc exists, and you get it for free, i dont always find libraries using it. In ruby, most gems that i use regularly and refer to their documentation, I actually go onto their github and read the readme, wiki, and sometimes source code. At work, we actively keep our clean through SRP. We also tend to name things well, to the point where writing RDoc comments would be mostly a waste of time. We try to make it so the code IS the documentation. Larger projects like rails do use a more formal documentation, methods usually documented inline with code. http://api.rubyonrails.org/

Now given we're writing a library in a very dynamic language that is near impossible to REPL, it makes it tricky to tell a user how to use the methods available, what methods are available, and what the wholesome API is. It's a bit simpler when you just grab a ruby gem for creating test factories, or a gem to put in twitter bootstrap. It documents the main methods on how to call stuff, and that's it. The API is typically small. In MelonJS we have nearly 20 items under namespaces, and 30-40 under the classes. It's a fairly sizable API, where covering it in the wiki would be a nightmare. Especially when we talk about versioning.

I wonder with all the javascript libraries, how many are actually big enough to have this issue. jQuery does their own documentation, and they have done a fairly good job at maintaining it. A lot of plugins and such out there get covered by a read me just fine. I haven't used it much myself, but i imagine this applies as well with node modules for web apps, similar to how it applies to many ruby gems that i use.

Not sure how this comment turned into a novel, but this thread brings an interesting discussion on documentation and the current ecosystem around it.

parasyte commented 10 years ago

BTW, you guys have done awesome work on melonJS in the last month. I haven't done a thing! :laughing:

The reason I want to stick with JavaDoc style comments is kind of obvious. There are a LOT of them spread throughout our codebase! There's really no reason we can't switch to another documentation style. What I like about JavaDoc is that it provides a lot of contextual/semantic information. For example, the parameter list and return value; they are distinct but both contain a data type and a description.

If I were to replace JavaDoc, I would want something that provides the same level of detail, even if the overall structure is completely alien. I can imagine documentation comments that use markdown exclusively, or even JSON! The latter would be interesting from the point of view that it could mimic Python's doc comments, and provide documentation from within the node.js REPL or browser console:

function foo(bar, zab) {
    // Do something ...
    return (bar in zab || (Array.isArray(zab) && zab.indexOf(bar) >= 0));
}

foo.__doc__ = {
    "about"    : "Does something awesome.",
    "describe" : "Lorem ipsum ...",
    "params"   : {
        "bar"     : [ "String", "Some text..." ],
        "zab"     : [ "Array", "Object", "Some other thing to work with!" ],
    },
    "returns"  : [ "Boolean", "True if bar the foo baz." ],
    "example"  : [
        "Is `goats` in Array?",
        function () {
            return foo("goats", [ "apples", "oranges", "goats", "cherries" ]);
        },

        "Is `tanks` in Object?",
        function () {
            return foo("tanks", { "planes" : 4, "cars" : 3, "trains" : 42, "tanks" : 17 });
        },
    ],
};

In the REPL, it looks like this:

> foo
{ [Function: foo]
  __doc__:
   { about: 'Does something awesome.',
     describe: 'Lorem ipsum ...',
     params: { bar: [Object], zab: [Object] },
     returns: [ 'Boolean', 'True if bar the foo baz.' ],
     example:
      [ 'Is `goats` in Array?',
        [Function],
        'Is `tanks` in Object?',
        [Function] ] } }
> foo.__doc__.params
{ bar: [ 'String', 'Some text...' ],
  zab:
   [ 'Array',
     'Object',
     'Some other thing to work with!' ] }
> foo.__doc__.example[1].toString()
'function () {\nreturn foo("goats", [ "apples", "oranges", "goats", "cherries" ]);\n}'
> foo("goats", [ "apples", "oranges", "goats", "cherries" ])
true
> foo("tanks", { "planes" : 4, "cars" : 3, "trains" : 42, "tanks" : 17 })
true
> foo(undefined, [])
false

One issue (and I'm sure there are many!) is that JavaScript is a dynamic language, and using dynamic strings and objects in doc objects would make it impossible for a parser to generate documentation. But with a static object like the example above, a parser could easily spot the __doc__ = assignment and apply the JSON to its internal node graph that represents the application. With the addition of markdown to style the output, I think you would have a fairly impressive means of documenting JavaScript.

FTR, I haven't seen anything like this in the wild. Possibly because it could get difficult to write documentation like this? But it could be pretty expressive, nonetheless. Documentation could be broken up for large classes, while remaining easy to parse with external tools:

var MyEntity = me.Entity.extend({
    "init" : function (x, y, w, h) {
        this.__doc__ = {
            "type"   : "Class",
            "params" : {
                "x"      : [ "Number", "X-coordinate" ],
                "y"      : [ "Number", "Y-coordinate" ],
                "w"      : [ "Number", "Width" ],
                "h"      : [ "Number", "Height" ],
            },
            "members" : {},
        };

        // Call super constructor
        this._super(new me.Vector(x, y), w, h);

        this.__doc__.members.color = [
            "me.Color='red'", "Primary entity color. Makes it all pretty!"
        ];
        this.color = new me.Color("red");

        this.__doc__.members.altColor = [
            "me.Color='black'", "Secondary color. Used for extra spice."
        ];
        this.altColor = new me.Color("black");
    }
});

The tools just need to grep the source for __doc__, really. It's rather trivial to apply the additional members keys throughout this example. It's standard JavaScript, so it runs as-is. And a pre-processor/build script can strip them out for minification.

It's kind of hard to read the samples in GitHub Flavored Markdown, but with a syntax highlighting plugin, it would be way better.

Anyway, just one more idea (out of infinite possibilities).

agmcleod commented 10 years ago

@obiot something i noticed in the container example is its x position seems to be doubling now. This might not be a bug, but it's a change i noticed and wanted to flag it.

Basically the pos.x and bounds.x are both 275, so you get an x value over 500:

var x = ~~(this.pos.x + bounds.offset.x + (
                this.anchorPoint.x * (bounds.width - this.renderable.width)
            ));

From the entity code.

agmcleod commented 10 years ago

@parasyte definitely a neat approach, but i think that style is very very intrusive to the code itself. I think writing the docs itself with that style would be easy, but having it as comments keep it separate from the code itself, allowing our eyes to just focus on the code, or the docs very easily.

Just to clarify as well, I agree that with our code base java-doc style comments make sense, and i don't think we should change them. I was simply putting my thoughts out there on why there may be tools lacking for JS.

parasyte commented 10 years ago

@agmcleod Would you agree that __doc__ objects would be less intrusive with syntax highlighting that styled them more like comments?

agmcleod commented 10 years ago

@parasyte might provide an easier separator for the eyes. Would that complicate parsing though?

parasyte commented 10 years ago

Oh no, syntax highlighting is separate. It would be a plugin to install in your editor.

obiot commented 10 years ago

@agmcleod about this :

var x = ~~(this.pos.x + bounds.offset.x + (
                this.anchorPoint.x * (bounds.width - this.renderable.width)
            ));

bounds.offset is actually an internal property that specify the offset between the entity/body position and the body collision shape, so the value should not be doubled (if it is, it's a bug).

now the current code is a bit confusing, as what I would like is for the body pos property to become the reference position for an object (which would make sense imho), but we still have some code that needs some cleanup, like here : https://github.com/melonjs/melonJS/blob/master/src/physics/body.js#L280

also I was thinking of redeclaring the pos property for entity with something like the follownig, that would greatly simplify everything :

    Object.defineProperty(me.Entity.prototype, "pos", {
        get : function () {
            return this.body.pos;
        },
        configurable : true
    });
obiot commented 10 years ago

@parasyte about last month, your spirit was there inspiring us all :):):)

and don't worry there is still a lot to do for the next 1.2.0, but also a few things for this one (like the quadtree stuff that is not yet fully working i think)

obiot commented 10 years ago

and back on the documentation topic, it's really a very interesting topic, but it might be a good idea to use the specific ticket (#497), as this one will then get "lost" once we will release 1.1.0, and we will lost all the relative discussion :P

obiot commented 10 years ago

i think it's ready for a 1.1.0 beta release now, except anyone has something that need to be done before that (before tomorrow? )

obiot commented 10 years ago

done : https://groups.google.com/forum/#!topic/melonjs/bukQ7KiVSYA and https://twitter.com/melonJS/status/496216426718302209

obiot commented 10 years ago

Shall we already now create a 1.1.0 branch (or do we wait for the beta period to be over)? btw, do we keep the 1.0.x branch once 1.1.0 is official ?

agmcleod commented 10 years ago

1.1.0 branch would be good if anyone wants to start on 1.2.0 features and merge them in.

For the 1.0.x branch, i think it really depends if we want to keep maintaining fixes for 1.0.x or not.

parasyte commented 10 years ago

@obiot You don't want a 1.1.x branch until 1.1.0 has been tagged. Also, you won't need a branch at all unless we end up doing maintenance releases. ;)

@agmcleod has a good point about the older maintenance branches. Git branches are very lightweight, so there is no storage/capacity concern. And even if you delete the branch, the commits will still be available in history, and accessible through the tags. The only reason to delete the branches, IMHO, is to keep the branch list clean. But for sanity reasons, we might just keep the current and previous branches around, and clean up the others.

During the transition phases like this, I think it's prudent to continue working on new features in feature branches, and consider master in code freeze. After release has been tagged, then it will be safe to unfreeze master and merge in any feature branches. This is all in effort to minimize maintaining multiple branches. (Feature branches are handled much differently than a mainline branch with multiple authors.)

Just my 2 :dollar:

agmcleod commented 10 years ago

There's always going to be maintenance releases. If we some how don't have to patch a 1.1.1, i'd be amazed.

aaschmitz commented 10 years ago

Hi guys, would be interesting to separate the file state.js (https://github.com/melonjs/melonJS/blob/master/src/state/state.js) into two files, one state.js and other screenobject.js? And maybe rename the me.SreenObject to me.Screen or me.Scene? Thanks!

parasyte commented 10 years ago

I like it, but renaming to Scene could be confusing, because that makes it sound like part of the scene graph (terminology we don't use in documentation, but is popular in other engines.)

Because all the code in state.js is relevant to the main Finite State Machine, I think ScreenObject should be renamed to something like StateNode. Then you don't have to split it into its own file. :smiley:

There's also a question of whether to move the main loop outside of the state object, and have one global main loop instead of switching between multiple loops ...

parasyte commented 10 years ago

Another name that would work is Phase, which is short, and descriptive. And as long as it doesn't cause other confusion (with phaser.io) it would be a good choice.

obiot commented 10 years ago

Hi guys, would you mind having look at the upgrade guide to see if we are not missing anything outstanding (and update it if you spot anything, including my bad english)

https://github.com/melonjs/melonJS/wiki/Upgrade-Guide

thanks !

parasyte commented 10 years ago

I added the Error classes a week ago. Didn't spot anything else that had changed which wasn't also in the upgrade guide. But then, I also did not thoroughly investigate.