Brain?

The Repo on GitHub

What shall we do today, Brain? Same as every day, Pinkie. Try to improve the world.

We’ll review the list of things we might do, and then we’ll do something.

• Facilitate removal of methods like `step` and `take` from DirectConnection
• Continue to refine protocol between DC, Cohort, and Bot;
• Finish addition of bots
• Enhance Cohort to add bots;
• Use Cohort in game;
• NEW: Bot’s accessors seem too broad
• NEW: Look for methods in Bot that belong elsewhere
• Deal with errors, including avoiding exceptions and passing error info in the knowledge;
• Deal with fact that Location cannot be JSONized;
• Clean up execute in World, possible new object.
• [Possible] Move to better separation between Bot and World in tests.
• [Possible] Make it easier to create the little action dictionaries;

It seems that the handwriting is on the wall, and we’re as fresh as we can be on the Cohort, so let’s see if we can finish Bot addition and then, maybe even today, use the Cohort in the game.

If I’m not mistaken, the World is ready for a so-far-unused request, ‘add_bot’.

class World:
    def execute_action(self, entity, verb, action):
        match verb:
            case 'add_bot':
                bot_id = self.add_bot(action['x'], action['y'], action['direction'])
                self.ids_used.add(bot_id)
            case 'step':
                ...

So, presumably, if we were to add ‘add_bot’ actions to our requests, we’d get some bots. Let’s review Cohort: it has a bot creation method and accepts a Bot on creation, used in testing:

class Cohort:
    def __init__(self, bot=None):
        self.bots = {}
        if bot:
            self.bots[bot.id] = bot

    def create_new_bot(self, bot_id):
        from client.bot import Bot
        new_bot = Bot(0, 0)
        new_bot.id = bot_id
        self.bots[bot_id] = new_bot
        return new_bot

It seems to me that, in actual use, a client would create a Cohort, telling it how many bots it wants in the world. The Cohort would issue that many ‘add_bot’ actions to its list. (Hm, no reason why ‘add_bot’ couldn’t accept an integer parameter: might be more convenient for everyone.)

N.B.

What we see here is actually important, I believe. As we work with and think about the code, we see aspects that might be improved. In my case, these often come as a little discovery. I wasn’t thinking about how to improve the ‘add_bot’ capability, the idea just popped into my head. Sometimes we do it immediately. Sometimes we make a note of it. Sometimes we do it later. Sometimes we don’t do anything at all about it. This time, I’ve made a note.

Let’s have a look a how these methods are used, because we need to walk the line between keeping tests working and making Cohort work properly in actual use.

I think that create_new_bot is only used by Cohort itself:

    def update(self, results):
        for result_dict in results:
            bot_id = result_dict['eid']
            bot = self.by_id(bot_id)
            bot.update(result_dict)

    def by_id(self, bot_id):
        try:
            return self.bots[bot_id]
        except KeyError:
            return self.create_new_bot(bot_id)

    def create_new_bot(self, bot_id):
        from client.bot import Bot
        new_bot = Bot(0, 0)
        new_bot.id = bot_id
        self.bots[bot_id] = new_bot
        return new_bot

Yes. When we get an id back that we do not recognize, we must create a bot with that id. The only expected way for that to happen is that the World will send us an update for every bot we asked to have created. If the World were to send us an id that we didn’t request, we’ll create a Bot for it anyway. Free Bot, no reason to complain about that.

N.B.

One might imagine keeping track of how many bots one expected and refusing to accept a bot one did not order. My own view is that they are like french fries: always accept them, even if we didn’t order them.

I think I’ll mark that method private, with an underbar, just as a note to myself. (Who else? No one else is here …)

Commit: mark method private.

The tests do create a Cohort with a given Bot, because they are testing things in operation, so the program would have bots to work on and the tests want to be able to interrogate them. And when we create the Cohort with no parameter, we get no Bots. So that’s good. Let’s test-drive the creation: it should be nearly trivial.

It turns out that we have a test for a Bot just showing up:

class TestCohort:
    def test_adding_via_surprise_knowledge(self):
        cohort = Cohort()
        WorldEntity.next_id = 100
        new_bot = WorldEntity.bot(5, 6, Direction.EAST)
        dict = new_bot.as_dictionary()
        cohort.update([ dict ])
        client_bot = cohort.bots[new_bot.id]
        assert client_bot.x == 5
        assert client_bot.y == 6

So that’s nice. We need a new test. I write this much and get kind of stuck:

    def test_adding_bots(self):
        cohort = Cohort()
        cohort.add_bots(5)

The Cohort is intended to be called when it is time to create messages:

    def create_message(self):
        message = []
        for bot in self.bots.values():
            actions = bot.get_actions()
            for verb in actions:
                action = self.create_action(bot, verb)
                message.append(action)
        return message

    def create_action(self, bot, verb):
        match verb:
            case 'drop':
                return {'entity': bot.id, 'verb': verb, 'holding': bot.holding}
            case _:
                return {'entity': bot.id, 'verb': verb}

I am assuming¹ that our socket code, when we have it, will repeatedly call create_message on the Cohort. So we really can’t send the add requests over until create_message is called. But when it is called, it creates a new message list. I think what we’ll do is just remember how many bots we need to create and create them on the next call to create_message. That should suffice until we know more.

So the test:

    def test_adding_bots(self):
        cohort = Cohort()
        cohort.add_bots(5)
        message = cohort.create_message()
        assert len(message) == 5
        for msg in message:
            assert msg['verb'] == 'add_bot'

I think we can make that work.

class Cohort:
    def __init__(self, bot=None):
        self.bots = {}
        if bot:
            self.bots[bot.id] = bot
        self._bots_to_add = 0

    def add_bots(self, number):
        self._bots_to_add += number

    def create_message(self):
        message = []
        for _ in range(self._bots_to_add):
            action = {'entity': 0, 'verb': 'add_bot'}
            message.append(action)
        for bot in self.bots.values():
            actions = bot.get_actions()
            for verb in actions:
                action = self.create_action(bot, verb)
                message.append(action)
        return message

The test is green. We’re requesting 5 add_bots. Let’s commit, then refactor. Commit: Cohort remembers how many …

Oh! This will never do! We don’t zero the count: it’ll create more and more Bots every time around! I’m glad I started to write that message.

Enhance the test:

    def test_adding_bots(self):
        cohort = Cohort()
        cohort.add_bots(5)
        message = cohort.create_message()
        assert len(message) == 5
        for msg in message:
            assert msg['verb'] == 'add_bot'
        assert cohort._bots_to_add == 0

Test now fails. Fix code:

    def create_message(self):
        message = []
        for _ in range(self._bots_to_add):
            action = {'entity': 0, 'verb': 'add_bot'}
            message.append(action)
        self._bots_to_add = 0
        for bot in self.bots.values():
            actions = bot.get_actions()
            for verb in actions:
                action = self.create_action(bot, verb)
                message.append(action)
        return message

Dandy. Commit: Cohort remembers how many bots need creating, issues ‘add_bot’ actions, zeroes count.

Now refactor, Composed Method pattern:

    def create_message(self):
        message = []
        self.add_desired_bots(message)
        self.get_existing_bot_actions(message)
        return message

    def add_desired_bots(self, message):
        for _ in range(self._bots_to_add):
            action = {'entity': 0, 'verb': 'add_bot'}
            message.append(action)
        self._bots_to_add = 0

    def get_existing_bot_actions(self, message):
        for bot in self.bots.values():
            actions = bot.get_actions()
            for verb in actions:
                action = self.create_action(bot, verb)
                message.append(action)

Green, naturally. Commit: refactor to composed method.

I think this should work round trip. Just for fun, instead of writing a test, let’s install the Cohort into the Game class, That will be a better example of what it’s like to use it, so we’ll learn a bit. And, of course, there is a decent chance that we’ll discover a problem.

Here’s how Game starts up now:

if __name__ == "__main__":
    world = World(40, 40)
    build_random_blocks()

    game = Game(world)
    connection = DirectConnection(world)
    for _ in range(20):
        client_bot =  connection.add_bot(10, 20)
        game.add_bot(client_bot)
    game.on_execute()

And in operation:

    def on_execute(self):
        if self.on_init() is False:
            self._running = False
        while self._running:
            pygame.time.delay(100)
            for event in pygame.event.get():
                self.on_event(event)
            self.clear_screen()
            self.draw_grid()
            for _ in range(10):
                self.run_one_bot_cycle()
            self.draw_world()
            pygame.display.update()
        self.on_cleanup()

    def run_one_bot_cycle(self):
        for client_bot in self.client_bots:
            connection = DirectConnection(world)
            client_bot.do_something_only_for_tests(connection)

I don’t really remember much about PyGame’s details, but what we have here seems to work, so we’ll just adjust it judiciously.

In the run_one_bot_cycle, we’ll want to use the Cohort to get a message and send it to the DC, which will update everyone.

if __name__ == "__main__":
    world = World(40, 40)
    build_random_blocks()

    game = Game(world)
    connection = DirectConnection(world)
    cohort = Cohort()
    cohort.add_bots(20)
    game.on_execute()

And in run_one_bot_cycle:

    def run_one_bot_cycle(self):
        message = Cohort.create_message()
        connection.run_request(cohort, message)

Nothing for it but to try it, I guess. I expect trouble.

I get it. First, I capitalized Cohort above. Second, the add_bot expects parameters.

Perhaps I should review the code:

    def execute_action(self, entity, verb, action):
        match verb:
            case 'add_bot':
                bot_id = self.add_bot(action['x'], action['y'], action['direction'])
                self.ids_used.add(bot_id)

OK, there’s an issue. If we just ask for 20 bots, where should they go? And our Cohort doesn’t even know anything about the world.

Reflection

We haven’t really done a lot of work, and the work we’ve done is a bit naff. But the code is all committed, and I don’t want to back down and reset. We’ll change world a bit, to default to setting bots at 10, 20, EAST if they have no parameters. Then we’ll put in parameters separately.

    case 'add_bot':
        # bot_id = self.add_bot(action['x'], action['y'], action['direction'])
        bot_id = self.add_bot(10, 20, 'EAST')
        self.ids_used.add(bot_id)

Try game again.

  File "/Users/ron/PycharmProjects/hbiots_reclone/shared/direction.py", line 21, in __eq__
    return self.x == other.x and self.y == other.y
                     ^^^^^^^
AttributeError: 'str' object has no attribute 'x'

We seem to have a string showing up where we expect … what? A direction, I think.

There is a defect in our add bot logic, in that the World add_bot method expects a direction, not a string. And it doesn’t check.

I can fix this by not passing ‘EAST’, because add_bot defaults correctly.

With that in place, the game works correctly. Commit: game works, World slightly hacked to accommodate game expectations and defect in dealing with direction. Cohort needs a way to specify where bots should go.

Now I really feel the need to slow down.

Calming Reflection

There are a number of ways we could go. One might be that the World always really decides where Bots should be placed, with a provision for testing that allows us to place them in specific positions. All we’d really have to do would be to remove the parameters from the ‘add_bot’ request and we could place them in World.

A quick fix would be to accept the parameters if they are provided and otherwise default them in World as we are with our patch.

I do think it would be useful to be able to request positions for one’s Bots. I was thinking of a style of solution where, on the client side, we allow the bots to share information. There is certainly nothing that could prohibit that. So we might start our bots on a grid in the world, separated out, each exploring a subset of the world, and building up a “big picture” of what’s out there, and then make strategic decisions about what to do.

Back to Work

First we have to fix the defect. If we are to accept the direction strings on ‘add_bot’, we need to convert them. Let’s put the code back as it belongs and make it work properly.

class World:
            case 'add_bot':
                bot_id = self.add_bot(action['x'], action['y'], action['direction'])
                # bot_id = self.add_bot(10, 20)
                self.ids_used.add(bot_id)

    def add_bot(self, x, y, direction = Direction.EAST):
        entity = WorldEntity.bot(x, y, direction)
        self.map.place(entity)
        return entity.id

The add_bot method is used in a lot of tests. So let’s change the add command. Start here:

    case 'add_bot':
        bot_id = self.add_bot(action['x'], action['y'], action['direction'])
        self.ids_used.add(bot_id)

Extract variable, posit² a new method:

    case 'add_bot':
        direction_string = action['direction']
        direction = self.get_direction(direction_string)
        bot_id = self.add_bot(action['x'], action['y'], direction)
        self.ids_used.add(bot_id)

We have this method:

    def set_direction(self, world_bot, direction_name):
        match direction_name:
            case 'NORTH':
                world_bot.direction = Direction.NORTH
            case 'EAST':
                world_bot.direction = Direction.EAST
            case 'WEST':
                world_bot.direction = Direction.WEST
            case 'SOUTH':
                world_bot.direction = Direction.SOUTH
            case _:
                pass

Does Direction have no help for us here? It has not. It should have.

class TestDirection:
    def test_from_name(self):
        assert Direction.from_name('WEST') == Direction.WEST
        assert Direction.from_name('NORTH') == Direction.NORTH
        assert Direction.from_name('EAST') == Direction.EAST
        assert Direction.from_name('SOUTH') == Direction.SOUTH
        assert Direction.from_name('fred') == Direction.EAST

    @classmethod
    def from_name(cls, string):
        match string:
            case 'NORTH':
                return cls.NORTH
            case 'EAST':
                return cls.EAST
            case 'SOUTH':
                return cls.SOUTH
            case 'WEST':
                return cls.WEST
            case _:
                return cls.EAST

Now back in World:

class World:
    case 'add_bot':
        direction_string = action['direction']
        direction = Direction.from_name(direction_string)
        bot_id = self.add_bot(action['x'], action['y'], direction)
        self.ids_used.add(bot_id)

And also:

    def set_direction(self, world_bot, direction_name):
        # no change on unrecognized name
        if direction_name in ['NORTH', 'EAST', 'SOUTH', 'WEST']:
            world_bot.direction = Direction.from_name(direction_name)

So now the defect is fixed properly. Green. Commit: fix defect where ‘add_bot’ set direction incorrectly.

Unfortunately, if I’m not mistaken, the Game is now broken, because Cohort isn’t passing in any information to the ‘add_bot’ message. Let’s default it in Cohort as a first step.

    def add_desired_bots(self, message):
        for _ in range(self._bots_to_add):
            action = {'entity': 0,
                      'verb': 'add_bot',
                      'x': 10,
                      'y': 20,
                      'direction': 'EAST'
                      }
            message.append(action)
        self._bots_to_add = 0

Game should work now. And it does. Commit: add_bot command defaults to 10, 20, EAST, Game works now

We have a long article, two hours in, and a few useful changes. We can sum up.

Summary

We might have been better off to spike the changes, given what happened. I was misled by the tests, and I freely grant that I didn’t even look carefully enough at the ‘add_bot’ command in World to notice the required parameters.

Another mistake, in retrospect, was to try to get the game working without a round trip test first. The test would have failed, or taught me how to do add_bot correctly, one or the other. However, the Game taught that same lesson.

We also had a defect where we were not converting direction strings to directions in ‘add_bot’ and add_bot. Unquestionably, more robust tests would have found that issue as well.

I’ve been leaning away from round trip tests, but, now that I think of it, we could test the request and the methods it calls with direct tests on World. We might have written the test correctly and then still done the Cohort change incorrectly, but it seems less likely.

I am still trying to go too fast. The steps are small, but the supporting tests seem not to be robust enough. That may reflect just that I’m trying to go too fast, or we could say that the tests we have and the new ones we’re writing are not supporting the speed we’re trying to go. Could be either, or both. The way to bet is that it’s both, and recommit to slowing down and testing more carefully.

Such commitments are hard to follow, at least for me. I have no useful advice, other than to try to find ways of calling one’s attention to the situation, pulling one out of the racing mode.

I’ve updated the to do list:

• Continue to refine protocol between DC, Cohort, and Bot;
• Finish addition of bots
• Enhance Cohort to add bots;
• Use Cohort in game;
• Adjust adding bots to allow for provision of starting bot info;
• [Possible] Allow for random setting in world?
• Improve testing around Cohort
• Review PyGame. Are we using it correctly?
• let ‘add_bot’ accept parameter for number to create;
• NEW: Bot’s accessors seem too broad
• NEW: Look for methods in Bot that belong elsewhere
• Deal with errors, including avoiding exceptions and passing error info in the knowledge;
• Deal with fact that Location cannot be JSONized;
• Clean up execute in World, possible new object.
• [Possible] Move to better separation between Bot and World in tests.
• [Possible] Make it easier to create the little action dictionaries;

Bottom line, we made decent progress, in that Cohort is now doing the job we intend for it, but it’s not doing it in quite the way we now realize we need. So there is more work to do.

The tests are green and the game works. There are a few patches in place, but that is the nature of our work.

Let me elaborate on that.

We do some work every day. The work day ends at some time, and if we are wise, we stop working when we are tired enough to start making mistakes. Well, even more mistakes than usual.

We cannot expect to be done and perfect at the end of every session. Now we could hold all our updates until we are done and perfect. But that is emphatically not our way. We want all the work we do to be in the system, not off in a branch somewhere. So we work such that we can commit our code without breaking the system. Sometimes a feature will be represented incompletely, but completely enough that the current tests and app work. (Often, we might put some changes behind a feature flag to keep them from affecting the app. That is not our practice here, because we have no users and no other programmers who might be affected.)

It is inevitable that we’ll commit code that is only partially complete and not quite good enough, almost every day. We commit (ha) to commit code that keeps the system running, and we commit to improve the code continually until it meets our standards.

When adding a new capability, as we did today, we expect to add something thin, just barely useful, and then to enhance what it does and how it does it until it is “Good”. We are in the midst of that process for adding bots via Cohort. We’ll continue that process, learning as we go. I predict that tomorrow we’ll be happier with what we have. Monday at the latest, depending what we do tomorrow.

See you then!