Famous / engine

MIT License
1.75k stars 250 forks source link

The documentation of Node.requestUpdate is confusing #446

Closed hashemian closed 9 years ago

hashemian commented 9 years ago

The documentation of Node.requestUpdate reads as:

/**
 * ...
 *
 * @param  {Object} requester   If the requester has an `onUpdate` method, it
 *                              will be invoked during the next update phase of
 *                              the node.
 *
 * @return {Node} this
 */
Node.prototype.requestUpdate = function requestUpdate (requester) {

So I expect this function to receive an object with onUpdate function, and onUpdate will be invoked on the next update cycle.

But all components use this differently, they pass the ID of already-registered component as requester, and the ID is stored in Node._updateQueue, and not the component itself. Also Node.update() treats all Node._updateQueue as IDs.

This gets more confusing when you see FamousEngine.requestUpdate has very similar implementation, but it receives Node and not Path for example.

I think the comment in Node.requestUpdate has to be updated to make it clear this is the ID of the component and node the component itself.

alexanderGugel commented 9 years ago

Good point. Just submitted #447, but I'm going to spend some more time on making sure the current documentation is factually correct, e.g. requester also needs to be an id as returned by Node#addComponent, not the component itself.

michaelobriena commented 9 years ago

landed