The Repo on GitHub

Let’s see what additional tests and capability we need in our new little batch mode objects. What were yesterday’s notions?

Yesterday, I closed with this:

I think next time we’ll probably improve the output side of this thing, and perhaps deal with the other actions, and maybe even process more than one bot at a time. That’s a lot: we will almost certainly not get all that in one session. But one never knows, do one? See you then!

Recall that for output, so far, we’re just stuffing the requesting object’s as_dictionary into the result:

class World:
    def process(self, world_input):
        from test_world_batch import WorldOutput
        output = WorldOutput()
        for request in world_input:
            requestor_id = request.identifier
            for action in request:
                self.command(action.action, requestor_id, action.parameter)
            output.append(self.fetch(requestor_id))
        return output

I had been thinking that we should provide more structure in the result, some kind of keyed thing with an entry for each request kind of thing. In particular, I was thinking that the receiver would want to know what each packet relates to. But each packet contains the identifier of the requestor, so I think we’ll just let the receiver sort them out by that value. It might even work for creating new Bots: we’d return the original dictionary as set up.

Now, this might not be enough. But it is enough for now. YAGNI, as we used to say in the old days: You Aren’t Gonna Need It. We said that, not because we might not need the thing someday, which, often, we might, but as a reminder that we might not need it. It was generally said in response to someone saying “We might as well do it now, we’re gonna need it …”.

We should have tests for the other command actions, and for more than one bot. Let’s see about that. Our sole significant test looks like this:

    def test_step(self):
        world = World(10, 10)
        bot_id = world.add_bot(5, 5, direction=Direction.EAST)
        request = EntityRequest(bot_id)
        step_action = EntityAction('step', None)
        request.add_action(step_action)
        batch_in = WorldInput()
        batch_in.add_request(request)
        batch_out = world.process(batch_in)
        assert isinstance(batch_out, WorldOutput)
        # invasive just to see how we did
        result = batch_out.results[0]
        assert result['location'] == Location(6, 5)

As we read that, notice the constructor for EntityAction. We had to pass in the None because as defined, EntityAction is a namedtuple with no default values. We could provide those, but let’s wait to see what actual users need. I think they’ll wind up needing more than one parameter, in fact.

So the other commands we understand are ‘drop’ and ‘take’. Oh, and turn. But the process code just extracts the value and calls command, which does the work. We scarcely need to test much here, but for completeness let’s do a bit more.

    def test_more_action(self):
        world = World(10, 10)
        block_id = world.add_block(6, 5)
        bot_id = world.add_bot(5, 5, direction=Direction.EAST)
        request = EntityRequest(bot_id)
        request.add_action(EntityAction('take', None))
        request.add_action(EntityAction('turn', 'SOUTH'))
        request.add_action(EntityAction('step', None))  # 5, 6
        request.add_action(EntityAction('step', None))  # 5, 7
        request.add_action(EntityAction('drop', block_id))  # 5, 8
        batch_in = WorldInput()
        batch_in.add_request(request)
        batch_out = world.process(batch_in)
        result = batch_out.results[0]
        assert result['location'] == Location(5, 7)
        item = world.map.at_xy(5, 8)
        assert item.id == block_id

Definitely a story test but I think that’s what I wanted. We dump in a block, add a bot. have him pick up the block, turn south, step twice, drop the block. We check that he’s in the right spot and that the block is back in the world. Basically worked first time, except that I forgot to put the block id in the drop, and I thought south was -1 not +1. All the issues were in the test not the code.

Also for completeness, let’s do two bots.

    def test_two_bots_return_two_results(self):
        world = World(10, 10)
        bot_a = world.add_bot(5, 5, direction=Direction.EAST)
        bot_b = world.add_bot(7, 7, direction=Direction.WEST)
        request_a = EntityRequest(bot_a)
        request_b = EntityRequest(bot_b)
        batch_in = WorldInput()
        batch_in.add_request(request_a)
        batch_in.add_request(request_b)
        batch_out = world.process(batch_in)
        assert len(batch_out.results) == 2

Green right out of the box. Oh and I forgot to commit the first time. Commit: adding tests for World.process.

What’s Next?

Right, good question. I guess we’d like DirectConnection to create and use these new objects. Right now, DC only processes separate commands and applies a result each time:

class DirectConnection:
    def step(self, bot):
        self.world.command('step', bot.id)
        self.update_client(bot)

    def take(self, client_bot):
        self.world.command('take', client_bot.id)
        self.update_client(client_bot)

    def drop(self, client_bot, block_id):
        self.world.command('drop', client_bot.id, block_id)
        self.update_client(client_bot)

That’s driven by Bot.do_something:

    def do_something(self, connection):
        actions = []
        actions += self.update_for_state_machine()
        self.state = self.state.update(self._knowledge)
        actions += self.state.action(self._knowledge)
        if random.random() < self.direction_change_chance:
            actions += self.change_direction()
        actions += ['step']
        self.perform_actions(actions, connection)

    def perform_actions(self, actions, connection):
        for action in actions:
            match action:
                case 'take':
                    connection.take(self)
                case 'drop':
                    connection.drop(self, self.holding)
                case 'step':
                    self._old_location = self.location
                    connection.step(self)
                case 'NORTH' | 'EAST' | 'SOUTH' | 'WEST':
                    connection.set_direction(self, action)
                case _:
                    assert 0, f'no case {action}'

In do_something, the Bot decides what to do, and in perform_actions it takes that list of things to do and wires in the parameters and asks the connection to do each action.

I think the right plan will be for perform_actions to produce a thing that is analogous to the WorldInput / EntityRequest / EntityAction nest. I think we need to have a design discussion here. Also, remind me that I’ve not yet moved the World objects out of my test file.

If we were writing a Kotlin client to call our Python-based server, we would build up some kind of structure that mimics the python one we have in WorldInput, etc. But we are writing a Python client, and it sure seems that what we need is exactly what we already have … except that those objects belong to the World.

Aside
I’m thinking, vaguely, about Kotlin’s cool way of building nested structures like these, I almost remember how that works and it makes for very nice construction of things like window structures. I’d best review that, might be useful for us.

For now, let’s think about how we would like to construct a request package for sending to the World. Presumably we have a collection of bots, and we want to let each one do something. Let’s go back to the WorldInput etc tests, and see what we might like to write. How about this?

    def test_imaginary_syntax(self):
        world = World(10, 10)
        block_id = world.add_block(6, 5)
        batch_in = WorldInput() \
          .request(world.add_bot(5, 5)) \
              .action('take') \
              .action('turn','SOUTH') \
              .action('step') \
              .action('step') \
              .action('drop', block_id) \
              .finish() \
          .request(world.add_bot(7, 7)) \
              .action('step') \
              .action('step') \
              .finish()

That would be nice, if only it worked. I think in Kotlin, we’d have some curly brackets isolating the nested bits into implicit lambdas. Python doesn’t have that nifty feature.

Detour

I’m going to take a detour and see about making the code above work. The test fails now, because WorldInput does not understand request. So:

    def request(self, identifier):
        rq = EntityRequest(identifier, self)
        self.add_request(rq)
        return rq

(I think we’ll find some duplication to remove if this works.) Now we fail because EntityRequest doesn’t understand action

class EntityRequest:
    def action(self, action_word, parameter=None):
        self.add_action(EntityAction(action_word, parameter))
        return self

Now I think we are probably failing on finish. Which I don’t know how I’ll do. We need to return the WorldInput when we get finish. And we don’t have it. OK, now we have it:

class WorldInput:
    def request(self, identifier):
        rq = EntityRequest(identifier, self)
        self.add_request(rq)
        return rq

class EntityRequest:
    def __init__(self, entity_identifier, world_input=None):
        self.world_input = world_input
        self.entity_identifier = entity_identifier
        self.actions = []

    def action(self, action_word, parameter=None):
        self.add_action(EntityAction(action_word, parameter))
        return self

I think this works, and that therefore I can recast an actual test:

    def test_more_action_new_syntax(self):
        world = World(10, 10)
        block_id = world.add_block(6, 5)
        batch_in = WorldInput() \
            .request(world.add_bot(5, 5, direction=Direction.EAST)) \
                .action(EntityAction('take', None)) \
                .action(EntityAction('turn', 'SOUTH')) \
                .action(EntityAction('step', None)) \
                .action(EntityAction('step', None)) \
                .action(EntityAction('drop', block_id)) \
                .finish()
        batch_out = world.process(batch_in)
        result = batch_out.results[0]
        assert result['location'] == Location(5, 7)
        item = world.map.at_xy(5, 8)
        assert item.id == block_id

I don’t expect this to work, though I couldn’t say why I am uncertain. I think it’s just that this is such a big and tricky step. It does fail, but oddly:

    def command(self, action, bot_id, parameter=None):
        world_bot = self.map.at_id(bot_id)
        if action == 'step':
            self.step(world_bot)
        elif action == 'take':
            self.take_forward(world_bot)
        elif action == 'drop':
            block = self.map.at_id(parameter)
            self.drop_forward(world_bot, block)
        elif action == 'turn':
            self.set_direction(world_bot, parameter)
        else:
>           raise Exception(f'Unknown command {action}')
E           Exception: Unknown command EntityAction(action='take', parameter=None)

Oh. We are passing in an EntityAction to command rather than unwinding it.

Oh, I failed to finish my edit.

    def test_more_action_new_syntax(self):
        world = World(10, 10)
        block_id = world.add_block(6, 5)
        batch_in = WorldInput() \
            .request(world.add_bot(5, 5, direction=Direction.EAST)) \
                .action('take', None) \
                .action('turn', 'SOUTH') \
                .action('step', None) \
                .action('step', None) \
                .action('drop', block_id) \
                .finish()
        batch_out = world.process(batch_in)
        result = batch_out.results[0]
        assert result['location'] == Location(5, 7)
        item = world.map.at_xy(5, 8)
        assert item.id == block_id

The test is green. We need another test, one that does more than one bot at a time, to ensure that finish works as we intend. Or let’s just extend this one:

    def test_new_syntax(self):
        world = World(10, 10)
        block_id = world.add_block(6, 5)
        batch_in = WorldInput() \
            .request(world.add_bot(5, 5, direction=Direction.EAST)) \
                .action('take', None) \
                .action('turn', 'SOUTH') \
                .action('step', None) \
                .action('step', None) \
                .action('drop', block_id) \
                .finish() \
            .request(world.add_bot(7, 7)) \
                .action('step') \
                .finish()
        batch_out = world.process(batch_in)
        result = batch_out.results[0]
        assert result['location'] == Location(5, 7)
        result = batch_out.results[1]
        assert result['location'] == Location(8, 7)
        item = world.map.at_xy(5, 8)
        assert item.id == block_id

That passes. I am content. Commit: added constructor syntax to WorldInput and EntityRequest. The classes are now interlinked by knowing each others’ names.

Let’s reflect.

Reflection

I think the biggest possible objection to this is what I said in the commit message: The classes are now interlinked by knowing each others’ names. WorldInput knows how to create an EntityRequest, EntityRequest knows how to create an EntityAction. One could imagine some kind of factory objects or methods that knows all the names, leaving the classes without the need to know of each other. Of course, we intend these objects to contain each other anyway, so it’s not a huge objection.

The trick whereby we pass the WorldInput to the EntityRequest, so that it can hold on to it, so that it can respond correctly to finish is also odd: we don’t usually like child objects to know their parents.

Finally, the need for the finish operation is itself a bit of an issue. We are likely to forget it. If we do, we’ll wind up sending request to an EntityRequest. Now, as it happens, that ER could create another one, and by the nature if this whole process, it has access to the WorldInput, so it could add it. So … I think … we can remove the finish as unnecessary. Maybe next time. I’m on overtime here.

As for the Factory idea, I’ll have to think about that, and I’ll see if I can get some feedback from the FGNO gang tonight. I kind of hogged the spotlight last week, though, so I might have to lay back tonight if someone else has code to show. We’ll see about that.

Summary

Today we took a quick jog off our intended path. Thinking of Kotlin DSLs made me think that maybe we could produce a nicer syntax for building our WorldInput nests. That may not pay off much, since so far we only create them in tests and we’ll have exactly one other place where they ever get created, at the world end of the Connection. But the idea was worth developing, and we do have the need, on the client side, to build a similar thing to pass into the connection.

I’m pleased … and left with things to think about. Nothing better than that! See you next time!