Several small issues

[Fabian Meisinger]

Hi,

I am currently using melon.js together with pomelo to develop a 2d multiplayer space game.
Over the last few weeks I noticed several things in melon.js that did not work as expected, are missing (I changed/added some of them locally) or just cost me some time.
Until now I mainly worked on the server side code, so before I start digging deeper, I would like to set up a workflow how I can contribute things.

As I never have contributed to an open source project before, I am not sure how I should proceed.
I guess normally one would fork the repo, open an issue and make a pull request for every one of them, or just make an issue if they have no fix?
Or can I just open one issue for all the fixes I made? Do I even need to make issues (I have never used pull requests before)?

Here is a list of the things I found so far:

loader:

• unloadAll cannot work with the current implementation (res vs. name).

camera:

• bounds should be optional (allow for infinite scrolling in all directions)
• move does not match description (moves relative instead of absolute) -> add new method moveTo and update comment
• follow does not work with negative coordinates (at least in an older build, didn't try it with the current dev version).
• on the right and bottom of the screen, sprites disappear/appear once their position is outside/inside the viewport, regardless of their size.
  

input:

• isKeyPressed should always return key state regardless of lock state
• keydown should update lock state instead of isKeyPressed, as this function might never be called and the KEYDOWN event then always shows true for unlocked (why is this inverted?)
• register pointer event should check if rect is defined when setting _float
  

matrix:

• add method to set rotation to specific angle

vector:

• add method to multiply by scalar (like div(n))

state:

• when the screen is a visible object, switching to a different state does not call the onDestroyEvent of the current state before the onResetEvent of the new state is called -> remove children immediately

save:

• delete is a keyword and shows a syntax error in eclipse when used as an object key or function name -> rename delete to remove

general:

• file names should reflect the name of the class they contain (e.g. me.viewport is in camera.js, why not rename it to viewport.js?)
• one class per file would make it easier to find stuff (e.g. state.js contains ScreenObject and state)
• more consistent naming (e.g. ObjectContainer: getEntityByName vs. getChildByProp)
• defer should work like call and apply and take the context as first argument to allow deferred calls on object methods (e.g. this.draw.defer(this, context);)
  

Function.prototype.defer = function defer() {
    return setTimeout(this.bind.apply(this, arguments), 0.01);
};

[Jay Oster]

Hello Fabian,

Good questions. To start, it is probably easiest to create a new branch for each issue, add some commits, and open a pull request for the specific issue that it fixes. Repeat as necessary. We prefer commits and pull requests that are well-isolated. As that makes it easier to understand why a particular change was made, revert pieces if necessary, and track the history of thinking that led to design considerations. We will also do peer reviews in the form of github comments to make sure the team agrees on simple things like code style, and more complex things like algorithm optimizations.

Last thing to mention, and pretty important: we're currently focusing on the dev-1.0.0 branch, and leaving master in a maintenance only (bug fixes) mode. For features and architectural changes, you'll want to branch off the dev-1.0.0 branch.

I won't touch on everything you mentioned, since some of them are clearly bugs, and some could definitely use some work as new features. But here are a few comments I have already:

• Viewport is designed to cover the entire screen canvas. If your viewport is somehow smaller than the screen, you will have issues like the disappearing/reappearing entities. Entities are not supposed to be drawn, nor have AI updates, when outside of the viewport. This is a performance concern, which has highly influenced the design. With some changes, it is be possible to create multiple viewports, but they must all cover exactly 100% of the screen to prevent strange issues like this. And in the end, an implementation like this may be too complex for the average developer.
• There are also some limitations with the bounding mechanism. By default it is bound to the map size. It is possible to extend it into infinity, but not well-documented. May be a more interesting feature to specify boundary regions that can hide secret areas in a map until the followed-entity gets into the area. You could also implement 2D Zelda-like scrolling with this, or Megaman scrolling with non-rectangular levels.
• me.input.isKeyPressed() is the method for checking key state with locking. What you want is me.input.keyStatus()
• While the placement of the key locking code may be unfortunate in some cases, the KEYDOWN event receives the lock state in the form of an edge value. The concept comes from electronics engineering, where voltages can be detected with edge-triggering or level-triggering; pressing a button will change the voltage on its line, but upstream components may only detect the voltage change itself (from zero-to-positive voltage), or they may detect the level (is is zero volts? or is it positive volts?) Codified in melonJS input events, you receive information about the key's edge state, which is only useful when the key locking (edge triggering) is enabled on the key binding. Otherwise you just get the constant level, which is always going to be true. Docs : http://melonjs.github.io/docs/me.event.html#KEYDOWN
• `rect` is a required parameter to me.input.registerPointerEvent(). If not included, the call is invalid and its actions are undefined. Only the last argument to this function is optional.
• We are planning to remove the functionality of using ScreenObject as an object added to the game loop. It causes many silly problems.
• The "delete" keyword is a silly one. If we used proper JSON syntax for object keys, this would not be an issue. "delete" : function (key) { ... } and you may have to call it with me.save["delete"]() if your IDE is so smart that it's stupid.
• I don't want to get into Java-like directory trees. I think it's reasonable to name file by the class they implement, sometimes. But for large classes like input, we want to break them into multiple files. Probably src/input/pointer.js, src/input/keyboard.js, src/input/gamepad.js ... But we're probably not going to break up the me.input class into me.input.pointer, me.input.keyboard, etc.

[Aaron McLeod]

On the class names, until I got used to it, i did find knowing what files had what code took me a bit. The one that really stands out for me is camera.js and the class is me.Viewport. shape.js for rect and such I'm fine by, as well as sprite since animationsheet is quite small. When I write my own games, I tend to do it in a more java style of separate files per class, but that's just more habitual on my part.

--
You received this message because you are subscribed to the Google Groups "melonJS - A lightweight HTML5 game engine" group.
To unsubscribe from this group and stop receiving emails from it, send an email to melonjs+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
Aaron McLeod
http://agmprojects.com

[Fabian Meisinger]

Thank you for the fast reply.
I will look into creating the issues/branches/pull requests tomorrow and continue the discussion there.