The Repo on GitHub

GeePaw and I addressed a serious design error. I think we wound up in a decent place. Today will tell.

Early on in the development of this program, I made a simple decision: code, be it test or our demo, would create a instance of Bot, an object that makes sense for simulating a bot, and we’d register it with the World. The World would store the Bot in a collection, later in the Map, and everything would proceed from there.

This turns out to have been a mistake, surely the biggest mistake in the program so far. Why? I’ll tell you why: because now the World has an object pointer to an object that belongs on the client side of things. Unless we wanted to develop fully distributed objects, if we even could, that connection cannot be permitted to exist in client-server mode. Yes, I had every intention of deferring client-server, but this particular decision didn’t have to be made that way, and it has been at the core of my difficulties getting closer to client-server over the past handful of sessions.

GeePaw Hill joined me for a session yesterday afternoon. At first we thought we might add another command to the DirectConection, but then, at his urging, we decided to break that link once and for all. More or less. Mostly. Pretty much.

We began, if I’m not mistaken, by changing the protocol for tests. They often said something like this:

    world = World(10, 10)
    bot = Bot(5, 5)
    world.add(bot)

We changed them to look like this:

    world = World(10, 10)
    bot = world.addBot(5, 5)

And in World, we had;

class World:
    def add_bot(self, x, y, direction = Direction.EAST):
        bot = Bot(x, y, direction)
        self.add(bot)
        return bot

Everything worked at that point and shortly we changed to this:

class World:
    def add_bot(self, x, y, direction = Direction.EAST):
        bot = Bot(x, y, direction)
        self.add(bot)
        returned_bot = Bot(x, y, direction)
        returned_bot.id = bot.id
        returned_bot.world = self
        return returned_bot

This completely breaks the connection between the world’s copy of the bot and the test or game’s copy. That being the case, tests were bound to break if they put a bot into the world and drove it around doing things, because the world would update one or the other but not both copies.

You’ll note that I didn’t say “would update its copy and not the other”. It wasn’t quite that simple, because depending on the operation, sometimes the World would fetch its own copy (a good thing) but sometimes it would be passed the client-side copy, not a good thing because now World has an object pointer into client space.

Nine tests broke. We had expected millions, although I had seen that same number, 9, a day or so before. So we set out to fix the tests. We’d find a test that said something like this:

    def test_wandering(self):
        world = World(10, 10)
        bot = world.add_bot(5, 5)
        bot.do_something()
        loc = bot.location
        assert loc != Location(5, 5)

And change it to this:

    def test_wandering(self):
        world = World(10, 10)
        client_bot = world.add_bot(5, 5)
        client_bot.do_something()
        real_bot = world.map.at_id(client_bot.id)
        client_bot._knowledge = real_bot._knowledge
        loc = client_bot.location
        assert loc != Location(5, 5)

We tell the client bot to do whatever we want to test, typically calling do_something but often calling some other Bot method. Then we explicitly copy the knowledge of the World bot over to the client bot. That does two good things:

  • First, it emulates the final client-server mode, where the Connection will fetch the results from the World and hand them out to the client objects;
  • Second and equally important, it means that changes made directly from world to client are wiped out, so that any direct connections remaining have no effect.

Combined, these two effects give us pretty decent confidence that the two sides are playing fairly now, keeping their information to themselves.

A couple of tests required a bit more tweaking, because they hammered special Bot values, like the chance of changing direction, and we had to make sure that both bots had that same setting. I think changing direction was the only case of that. I’m not worried about that for the longer term, because we’ll be making direction changing into a command, which means that the World won’t need to know about it.

Note
The oddest thing about all this is that, as things stand now, the world is still executing client code, running the state machine and so on, but that it doesn’t really matter. Since it has its own copy of the Bot, our tests are still valid. As we move to passing commands, the World will simply stop calling Bot methods.

Finally, in World, we fixed the step method:

    def step(self, bot):
        location = self.bots_next_location(bot)
        self.map.attempt_move(bot.id, location)  # changes world version
        real_bot = self.map.at_id(bot.id)
        self.set_bot_vision(real_bot)
        self.set_bot_scent(real_bot)

This was a case where the client bot was being passed in, so we explicitly switch over to the world-side one. This changed fixed a very confusing situation with our last couple of tests, where half the necessary information was in the world-side bot and half in the client side. This change isolated it to the world side. When the tests copy the information over, it then appears on the client side.

Assessment

The above took us two hours, and we were really rather confused much of the time. As I review the code now, I’m confident that the confusion was mostly over the need to set the direction change flag and over the anomaly in the scent and vision, which were being done client side while other operations were not. The result was that there was no correct information anywhere for those tests.

Reviewing the code now, there are only those two small changes to World, and the changes to the tests, and nothing else in the production code changed at all. So I think that what we have is righteous, and since it is green, I’m going to commit it. Then I’m going to make one more change. Well, at least one, this one to simplify the test changes just a bit.

I could make the change and then commit, I suppose, but I’d rather have the save point. Commit: green. client side and world side now use different bot instances.

My planned change is this.

    def test_nothing_near(self):
        world = World(10, 10)
        client_bot = world.add_bot(5, 5)
        real_bot = world.map.at_id(client_bot.id)
        client_bot.direction_change_chance = 0.0
        real_bot.direction_change_chance = 0.0
        client_bot.vision = None
        client_bot.move()
        real_bot = world.map.at_id(client_bot.id)    # <===
        client_bot._knowledge = real_bot._knowledge  # <===
        assert client_bot.location == Location(6, 5)
        vision = client_bot.vision
        assert ('R', 6, 5) in vision

I want to encapsulate those two lines, which appear a dozen times or more, with a single call to World to do the job.

    def test_nothing_near(self):
        world = World(10, 10)
        client_bot = world.add_bot(5, 5)
        real_bot = world.map.at_id(client_bot.id)
        client_bot.direction_change_chance = 0.0
        real_bot.direction_change_chance = 0.0
        client_bot.vision = None
        client_bot.move()
        world.update_client_for_test(client_bot)  # <===
        assert client_bot.location == Location(6, 5)
        vision = client_bot.vision
        assert ('R', 6, 5) in vision

class World:
    def update_client_for_test(self, client_bot):
        real_bot = self.map.at_id(client_bot.id)
        client_bot._knowledge = real_bot._knowledge

That test goes green. I think I can basically just paste that where all the two-line versions are. That’s readily done, and we are green again. Commit: replace two-line update of client knowledge in tests with single call to world update_client_for_test.

Let’s Plan

What are some things we need to do, still mostly aiming at being just a bit more ready for client-server? Let’s see …

  • Block is still shared between world and client. Client probably makes no real use of Block?
  • Change World so that it can never talk to a client-side bot. Make it rely solely on the id.
  • Give World its own representation of Entities, including just the information it uses or provides.
  • Continue to extend DirectConnection to support all commands.
  • Change of direction should be done by command, not by mashing the _direction value.
  • The word “vision” is used with two meanings, one a list and one a smart list.
  • Check take and drop. World side should be trivial, intelligence is on client side.
  • Devise a simple way of transferring the result info between world and client.
  • Extend client side to support a fleet of bots. Current tests are all focused on one.
  • Consider whether Map storage is too fragile, whether it’s useful enough.
  • World needs type of entity. Name / icon etc is client side … except for making?
  • Do we need a Making App / Shipping App? I still don’t understand that idea well.
  • Organize files into server / client / test [each]

Wow, that’s a lot for off the top of my head!

I think the things that I’d like to address early on are getting more operations into DirectConnection, which may entail beginning to formalize the shape of the World’s output to the client, and which will impact the new update_client_for_test method, which will probably evolve to be the same as used in the connection.

For now, even though it is only 0900, I’ll have a bit of a break. See you next time!