The Repo on GitHub

Bot’s behavior is pretty cohesive now. But its member variables vary variously. Should we improve that? Once again we almost work on that … and then something else comes up.

Bot’s member variables look like this now:

class Bot:
    id = Forwarder('_knowledge')
    location = Forwarder('_knowledge')

    def __init__(self, x, y, direction=Direction.EAST):
        self.name = 'R'
        self.direction_change_chance = 0.2
        self._knowledge = Knowledge(Location(x, y), direction)
        self.state = Walking()
        self._old_location = None

The bot’s name never changes: they are all named ‘R’. The direction_change_chance never changes, but in principle the Bot could adjust the value to tune its behavior. The attribute _knowledge itself never changes, but it contains all the information that se get back from the World after we take a turn. None of that changes on our side, but most of it changes on every turn. The id, part of the knowledge, never changes.

The state member changes often, and is reassigned on every turn. The _old_location is set whenever the Bot decides to take a step, and it takes a step on every turn, although a different Bot strategy might not do that.

But wait! There’s more! Inside Knowledge, all the members come from the World, with the exception of the member _energy, which is used by the state machine, and which changes on every turn. None of the other members of Knowledge change except when Knowledge is updated from the World. That happens on every turn as well, but all those members are essentially informative: they’re not really variables from the viewpoint of the Bot. They could be read-only as far as we’re concerned.

Which of the members does the Bot itself actually use? An interesting question indeed. It uses location and _old_location. It refers to direction_change_chance.

Push Down
I discover something that needs a fix. Push down all our mental state and deal with this direct reference to _knowledge:
class Bot:
    def change_direction(self):
        return [self._knowledge.new_direction().name()]

class Knowledge:
    def new_direction(self):
        possibles = [d for d in Direction.ALL if d != self.direction]
        return choice(possibles)

This is why we have the Forwarder class, so we should always use it. The above is the only use of new_direction, and we want the name, not the direction, so let’s first do this:

class Knowledge:
    def new_direction(self):
        possibles = [d for d in Direction.ALL if d != self.direction]
        return choice(possibles)

    def new_direction_name(self):
        return self.new_direction().name()

class Bot:
    def change_direction(self):
        return [self._knowledge.new_direction_name()]

Green. Commit: refactoring new direction.

Now add a forwarder and use it:

class Bot:
class Bot:
    id = Forwarder('_knowledge')
    location = Forwarder('_knowledge')
    new_direction_name = Forwarder('_knowledge')

    def change_direction(self):
        return [self.new_direction_name()]

Green. Commit: use Forwarder for new direction name.

Pop
Pop back to previous task. Don’t forget what you just learned.

That discovery makes me want to take a further look at Bot to see if it has any other references to _knowledge: they should all use Forwarder. I find this one:

    def update(self, result_dict):
        self._knowledge.update(result_dict)

Ah. Here’s an interesting issue: the name of our method and the Knowledge method is the same. If I put a Forwarder named update into Bot, it will bypass our own method. That should work fine now, but if Bot ever does want to do anything with the update on the way by, we’d lose our chance.

By way of experiment, I think I’ll try it.

The tests all run. I decide—I don’t know why—to try the game, and it breaks!

  File "/Users/ron/PycharmProjects/hbiots_reclone/client/cohort.py", line 48, in create_action
    return {'entity': bot.id, 'verb': verb, 'holding': bot.holding}
                                                       ^^^^^^^^^^^
AttributeError: 'Bot' object has no attribute 'holding'
Push Down
Aoogah Aoogah! Enter emergency broken build mode. 😀‼️🆘

I removed that Forwarder yesterday, and there was no error. I’ll put it back and commit to fix the broken build, but we need more testing.

class Bot:
    holding = Forwarder('_knowledge')
    id = Forwarder('_knowledge')
    location = Forwarder('_knowledge')
    new_direction_name = Forwarder('_knowledge')
    update = Forwarder('_knowledge')
Oops
Arguably, I should have committed the game fix here, as it is an emergency. I didn’t. Of course the next commit is about five minutes later, not like the system was down for a lot of extra time.

Game works. The error came from Cohort:

    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}

The test is easy to write although rather invasive:

    def test_can_drop(self):
        # invasive to cover missing forwarder
        cohort = Cohort()
        test_bot = Bot(5, 5, Direction.EAST)
        test_bot._knowledge.id = 101
        test_bot._knowledge._held_entity = 666
        action = cohort.create_action(test_bot, 'drop')
        assert action['verb'] == 'drop'
        assert action['entity'] == test_bot.id
        assert action['holding'] == 666

It runs when the holding Forwarder is present and not when it is absent. Commit: test checking drop command. covers missing forwarder issue and drop message formatting.

Pop
OK, pop out of emergency mode. Let’s regroup, reflect, reassess, and breathe a sigh of relief.

Reflection

So, yesterday I released a version where the game crashed. It was down to a missing test, plus either I missed a reference to the holding forwarder or never checked. I think it’s the latter: if I recall correctly, I just commented out those lines one at a time to see whether tests failed. None did.

A simple smoke test would have detected the problem yesterday: just starting the game was enough to trigger the error.

You know what? We could probably write a smoke test and run it as part of our tests. It would be a bit tricky: the error we just found didn’t occur until one of the Bots had found a block and a place to put it, so we were probably quite a few moves into the game. I’ll make a card for the idea but we’ll not do it this morning.

So, we had just moved update from a method on Bot to a forwarder to the Knowledge, which was all that the method did anyway. If we ever have need to intercept the update information on the way in, we’ll have to do something … but that seems very unlikely, because we have all the information there is, and we have a Bot method that checks that expectations have been met, so anything that needed doing, we could put there. Not to worry,

We’ve just arranged that Bot isn’t making direct references to Knowledge other than through its forwarders. It does pass the Knowledge to the state machine, of course.

Which brings us back to our earlier topic, the varying uses and chronology of the members, in particular _old_location, saved in Bot, and _energy, saved in Knowledge.

If we mean Knowledge to be all the information we get from World, and no other information, then _energy doesn’t belong there. And if we mean for Bot to save everything that is turn-based off in one or more objects like Knowledge, the _old_location does not belong in Bot.

Our state machine classes, Walking, Looking, and Laden, all accept a single parameter, knowledge, which they refer to to make their decisions. They expect their knowledge to understand a few things: can_drop, can_take, gain_energy, has_block, has_energy, and use_energy.

It’s worth noting that all those notions are fairly abstract higher-level ideas, not simple accessors. They interpret the knowledge rather than just returning a simple value that’s stored there.

That suggests to me that there might be at least two objects hiding in or near Knowledge, one that knows the simple facts, and one that interprets those facts. I take a look at Knowledge and its methods, to decide whether to explore it with you, and I find that, while there’s room for improvement, I think we’re at a stopping point right now. Let’s see what a summary would look like if we do stop:

Summary (tentative)

Added in Post
Promoted below to Official Summary.

One Big Lesson: We need some kind of more extensive but not to expensive “smoke test” that gives us automatic confidence that the game is going to run.

We have made just four commits, two steps to move the new direction logic properly over to Knowledge and cause it to return what we really want, and one for the broken build, which should have been done in two commits but somehow I missed one, and one that I didn’t write about that improved a test that I happened to notice.

I am not impressed with the progress. We have, however, increased the Forwarder count of Bot from two, should have been three, to five, adding new_direction_name and update as well as holding, which should never have been removed. So we have made the code a tiny bit better.

But I am not satisfied. We keep approaching the weirdness of Knowledge, the fact that it contains one member that is not like the others and the fact that Bot contains a member that is very much like the odd one in Knowledge. Both those odd ones “should” be together somewhere, and neither Bot nor Knowledge seems to be the right place. We come close to working out a solution … and then we don’t get there. Other, more important things happen instead.

What does this mean? Does it mean we’ll probably get there next time? Or does it mean that this oddness is not very important and that we should find something much more significant to work on?

I do not know the answers to these questions. I do expect that we’ll learn something interesting if and when we work out a solution for this oddness, and that we’ll get some interesting thoughts out of it. And I also think that the problem itself is not so important. If we had something better to do, on a real project I’d say to do that other thing.

Partly, I think I’m feeling that this project is tapped out, although there is certainly plenty of client-server work that needs to be done, at least in principle. Partly, I think I’m disappointed that the project didn’t turn into the group effort we all contemplated going on. No one’s fault, really, it just didn’t take root as something we wanted to do together. But I am a bit saddened and have a sense of loss. I hope it’s the worst loss I ever suffer, but expect that it won’t be.

I’ll have to think about all this. For now, I am promoting this tentative summary to be the official summary of today’s work.

I hope to see all five of you next time!