gladiusjs/gladius-core — pull request #199 by dmose (2012-05-24 23:24:01+00:00)
Title: Wip core

Here are a bunch of spaces test dperit and I worked out, some paired, some not. One of them currently fails in an odd way, more on that in a bit.

I moved the spaces constructor next to the prototype in the spaces.js file, as that's standard style and where folks would generally expect to look.

Questions that came up while writing these tests:

The engine has its own RealTimeClock. Is this likely to be as accurate as date.now() or the new high-perf timers? Does that matter?

For the tests, we ended up reaching inside Entity somewhat. It wasn't clear to us why entity._children was a private member. Is there some reason components people shouldn't be able to inspect it?

It's currently possible to create a completely empty entity by supplying no arguments to the constructor. Is there a use case for this, or should it throw?

find(All)With is a little terse as far as names go. Maybe findWithComponentType?

Would it make sense for add() and remove() to return the values they were passed to make for easy chaining?

Right now, removing an entity that's not in a space doesn't do anything or notify the caller. Should it (for example) throw?

These tests nail down a deterministic return order for the various find and findAll (items should be returned in the order added), in part because those were the easier tests to write, but also under the theory that a deterministic API is likely to be less surprising to people who read docs closely (most people).

Finally, doing new Space() without arguments works in the examples which include the engine, because by some black magic, the space constructor ends up with a clock local variable that's non-empty. dperit and i don't see how this is possible. The unit test for that, however, fails, because the code behaves the way we would expect from reading it. Insight here would be much appreciated.

From reading the comments in the Space constructor(), that unit test should actually be testing not just the the clock is an instanceof Clock, but probably that it's an instance of the system clock. Would you agree?
If so, it's not entirely clear how one would test that without depending on the engine. Thoughts?