Sensible Idea?

The Repo on GitHub

Based on a few days’ experimentation, lots of thinking, and some messages with GeePaw, I have a tentative plan for moving closer to client-server style. Things go nicely!

Frequent readers will recognize most of these ideas, as they have come around on the guitar before. They’re beginning to come together into a tune I can almost recognize, and it goes like this:

Shared Knowledge

We’ll formalize the information in our Knowledge object, identifying the components that are shared between client and server. Things like the object id, its direction, location, and any detailed information that the two share. Currently, that will include the vision and the scent. All this information will be read-only from the client viewpoint, and the master version of it will reside in the World (Server).

The client-side Knowledge object will include the shared knowledge, plus any other information that the client may need. This is essentially up to the individual client authors to decide. There is no need for them to have a client-side knowledge object but they will need to communicate via the Shared Knowledge format once it is defined. And if other client authors ever arise.

Connection¹

This will be a client-side object, as I presently see it. We’ll start with a “direct” connection, which will have the only pointer to the World on the client side of things. Quite likely, it will be the sole pointer to World anywhere outside the tests. An indirect connection will be written someday, and it will have a network connection to the World, which will at that time need a connection kind of thing of its own.

Initially, in the direct form, the connection will receive all the messages that the client side code now sends to the World. I believe that all those messages specify a bot who is sending the message, and we will move over to passing the bot id to the World. Currently the World sends messages back to a Bot and these will be changed to be sent to the shared knowledge that will be the World’s representation of a Bot (or such other Entities² as we may define).

Evolution

I am hoping to evolve these ideas jointly, and in small steps, committing the code very soon and regularly thereafter. I believe that all the calls made by World to Bot are already all forwarded to the Knowledge. If that’s the case, passing the Knowledge instead of the Bot itself should “just work”. If it does …

Then the direct connection, can replace the pointer to World currently held in Bot, and it can receive all the calls that Bots send to World, and pass, not the Bot but the Knowledge over to World, which will update the Knowledge, thinking that it’s a bot. I think that should also “just work”. Then the connection can unpack just the shared info and copy it and pass that, which the World will duly update. Then the connection can copy that information back into the original Bot’s actual Knowledge. And then, over on the World side, we can build up our own Definitive Real Knowledge of What’s Up With This Entity, pay attention only to the id of the calling Bot and return the Definitive RKWUWTE to the connection, which will update the client side Bot.

Somewhere along that thread lies the point at which we can splice in a real remote connection, a socket or whatever. And if we choose to batch up information, our client-side connection will be in a position to receive all the bot messages, batch them up, send over the batch, and then redistribute the results to the various bots.

Commentary

I think this actually hangs together and will work. I believe that we’ll find plenty of flexibility in selecting small steps, what GeePaw calls “pathing”, and plenty of times when we can commit.

**Insert commentary here about confusion one day, thinking and discussion and experimentation, ideas begin to flow. **

Let’s get started. First, let’s see where we send messages from client side to world side and the reverse. We want to see if there are issues that need special consideration.

I’m already aware of one such: we change direction by just updating the direction field in Knowledge. We’d better change that to a command before we’re done. I don’t think we have to do it first.

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

Bot has the World member. How is it used?

class World:
    def add(self, entity):
        entity.world = self
        World.next_id += 1
        entity.id = World.next_id
        self.map.place(entity)

This one will be replaced when we provide the connection object. It’s the only time we set the member outside of tests.

class Bot:
    def do_state_actions(self):
        for action in self.state.action(self._knowledge):
            match action:
                case 'take':
                    self.world.take_forward(self)
                case 'drop':
                    self.world.drop_forward(self, self.inventory[0])
                case _:
                    assert 0, f'no case {action}'

    def drop(self, entity):
        self.world.drop_forward(self, entity)

I was going to show the whole list but this method is unused, as, I predict, is the similar take method. I’ll remove them now. Ah, not so fast, bunkie … take is used in tests. We’ll have to remove that one more carefully. Moving right along …

class Bot:
    def scan(self):
        return self.world.scan(self)

    def step(self):
        self.world.step(self)
        self.tired -= 1

Scan is used in tests, as is take. These should be tests on World. I think this morning’s work should be to clean that up, if we can. It’ll be a bit tricky, though, because the take and scan, as implemented, probably slam things into the Bot (or its Knowledge).

Anyway, that’s all there is, not too daunting. Let’s check further, though, and see what the World is saying to the Bot.

class World:
    def set_bot_vision(self, bot):
        bot.vision = self.map.vision_at(bot.location)

    def set_bot_scent(self, bot):
        bot.scent = self.map.scent_at(bot.location)

    def take_forward(self, bot: Bot):
        take_location = self.bots_next_location(bot)
        if take_location == bot.location:
            return
        entity = self.map.entity_at(take_location.x, take_location.y)
        if self.can_take_entity(entity):
            self.map.remove(entity.id)
            bot.receive(entity)

    def drop_forward(self, bot, entity):
        drop_location = self.bots_next_location(bot)
        if drop_location != bot.location and self.is_empty(drop_location):
            entity.location = drop_location
            self.add(entity)
            bot.remove(entity)

And there is a hidden access here:

class World:
    def step(self, bot):
        location = self.bots_next_location(bot)
        self.map.attempt_move(bot.id, location)
        self.set_bot_vision(bot)
        self.set_bot_scent(bot)

class Map:
    def attempt_move(self, id, location: Location):
        entity = self.contents[id]
        if self.location_is_valid(location):
            entity.location = location

It turns out that what we store in the Map is currently the actual Bot:

class World:
    def add(self, entity):
        entity.world = self
        World.next_id += 1
        entity.id = World.next_id
        self.map.place(entity)

class Map:
    def place(self, entity):
        self.contents[entity.id] = entity

I think this should all resolve almost readily. At the point where we do the add, World will create its own Entity object, that will go into the Map, which, as a subordinate of World, has legitimate write access to the object. I do think we might move the validation up into World, it seems a bit odd to have the map deciding not to move the thing, but it’s harmless albeit odd.

Reflection

I’m trying to think of a nice next move that will really feel like a step in the right direction. I guess that before I do that, though, I should look at those tests that are doing take and scan. I don’t think we even use scan anywhere. Here’s the first one:

    def test_scan(self):
        world = World(10, 10)
        bot = Bot(10, 10)
        world.add(bot)
        result = bot.scan()
        expected_scan = [('R', 10, 10)]
        assert result == expected_scan

This is pretty weak, but I think we’ll preserve it as a foundation for scanning. No, let’s not. It’s not used, we have a better form of scanning in the vision concept. Let’s remove these if this is all they do.

Two of the checks are simple. This next one could actually check two interesting things: does the Bot get the Block, and is the Block gone from the World Map. (This is an odd thing that I suspect should be changed. The Block should be off the map but not unknown to the world. It should be at an unknown location or something.)

    def test_take_a_block(self):
        world = World(10, 10)
        bot = Bot(5, 5)
        world.add(bot)
        block = Block(6, 5)
        world.add(block)
        bot.take()
        result = bot.scan()
        expected_scan = [('R', 5, 5)]
        assert result == expected_scan

I’ll change that to check more directly.

    def test_take_a_block(self):
        world = World(10, 10)
        bot = Bot(5, 5)
        world.add(bot)
        block = Block(6, 5)
        world.add(block)
        assert not world.is_empty(Location(6, 5))
        bot.take()
        assert bot.has(block)
        assert world.is_empty(Location(6, 5))

That’s a bit better and doesn’t use scan. There are no more. I think I can remove World and Bot scan. I do. Green. Commit: remove scan

Removing that turned up a method is_close_enough. That occurs on both Bot and Block. Remove it from both. We can code it again if and when we need it. Let’s get as much cruft out of here as we can. Commit: remove is_close_enough from Bot and Block.

Let’s check the take uses, we might be able to remove take from Bot. We can replace any and all calls to bot.take with world.take_forward. That we will want to preserve. So we change all the tests. Done. No usages. Remove bot.take. Green. Commit: remove bot.take.

Reflection

This has been good, it has simplified the problem a bit and also refreshed our understanding of what’s going on. I think the the Connection idea and the DirectConnection object are important enough to deserve some tests of their own. And yet, at least the direct one should be very simple, though it will grow.

I’ll make a test file called TestConnection. My canonical first test fails as expected:

class TestConnection:
    def test_hookup(self):
        assert 2 + 1 == 4

I move on:

class Connection:
    def __init__(self, world):
        self.world = world


class TestConnection:
    def test_exists(self):
        world = World(20, 20)
        connection = Connection(world)

This is surely not the smallest first step I could have taken, but it is small enough. Green. Commit: initial Connection.

What would be a good first real test? Rather than deal with adding a Bot, let’s slip one in sideways and then send world a message about it. I’m not sure about this, it just seems like it might be good.

    def test_step(self):
        world = World(20, 20)
        bot = Bot(10, 10)
        world.add(bot)
        connection = Connection(world)
        connection.step(bot)
        assert connection.knowledge.location == Location(11, 10)

This is tentative, but I think it drives out some behavior all in one go. Might be a bit large, but I don’t think so, since this object is not entirely live. We will extend the test to check further into what happened here.

class Connection:
    def __init__(self, world):
        self.world = world

    def step(self, bot):
        self.knowledge = bot._knowledge
        self.world.step(self.knowledge)

The test passes! I am not surprised, exactly. I thought it might. But it’s still a delight at how easy it was. Let’s do examine the world to be sure it has changed as it should:

    def test_step(self):
        world = World(20, 20)
        bot = Bot(10, 10)
        world.add(bot)
        connection = Connection(world)
        connection.step(bot)
        assert connection.knowledge.location == Location(11, 10)
        assert world.map.entity_at(11, 10) == bot

I added that last line to the test, checking the map to see if the Bot is in the right location in the world’s map, and it is.

Oh, but I meant to call this object DirectConnection. Rename it. Commit: DirectConnection can properly forward step using direct access to bot knowledge. Let’s reflect.

Reflection

A lot happened here. We passed World, not the bot it expected, but an instance of Knowledge, and the World happily updated it. We sent the DirectConnection a message that the Bot would have sent step(bot), and it round-tripped OK, so that, as it happens right now, but bot in question will actually have its knowledge updated, since we passed the same object over. Let’s enhance the test to show that … because we’re going to want to change it soon.

In fact, I think I have the spoons to do it now, but how can I legitimately test this?

What needs to happen, and I think it’ll be OK to do it now, is for the DirectConnection to copy the input knowledge and send the copy to the world, which will update it. Then the DC should copy the information back to the bot. That will better reflect the network character of things, although not perfectly, since we want, ultimately, to just pass the id over and have a knowledge come back. We can’t quite do that yet without breaking too many things, I think.

OK, I’ll put this check in, break it, and then make it work.

    def test_step(self):
        world = World(20, 20)
        bot = Bot(10, 10)
        world.add(bot)
        connection = DirectConnection(world)
        connection.step(bot)
        assert bot.location == Location(11, 10)
        assert connection.knowledge.location == Location(11, 10)
        assert world.map.entity_at(11, 10) == bot

The bot location is, of course, updated, because we are using the identical Knowledge. Change that:

    def step(self, bot):
        self.knowledge = self.copy_knowledge(bot)
        self.world.step(self.knowledge)

    def copy_knowledge(self, bot):
        return copy.copy(bot._knowledge)

That breaks the test because we give the World a shallow copy of the bot’s Knowledge. Now, however, we need to put the new knowledge back into the bot. I think we’ll copy it back just to be pure.

Ah! It doesn’t work. In a sense, that’s good news. Here’s the new code:

class DirectConnection:
    def step(self, bot):
        self.knowledge = self.copy_knowledge(bot)
        self.world.step(self.knowledge)
        bot._knowledge = copy.copy(self.knowledge)

    def copy_knowledge(self, bot):
        return copy.copy(bot._knowledge)

So the idea was, copy the knowledge we send over, and copy it back after the call. That’s what we’ll really have to do. The good news / bad new is, that doesn’t work, because right now, the Map contains the actual entity.

I think we can fix that by simply changing what the Map stores, but I need a break and a breakfast. And a chai latte.

So we have a clean version in the repo, and a broken test in my IDE. A perfect place to stop, because the test is where we’ll pick up next time.

Summary

I think this is going well. We are a hair away from the DC working properly, passing knowledge back and forth. It’s just one operation but they are all basically the same, so I think in the next session or two we can have the whole shebang running with the two sides isolated, connected only by the thin wire of the [Direct]Connection.

After all my running around barking at it and jumping up at it and generally fussing with it, what needed to be done as turned out to be rather simple. But while it does seem now that it will be easy, I can assure you that getting to this point of understanding was not easy for me. Perhaps, if I were much more intelligent and experienced than I am, etc., etc., it would have popped right out on the first day what needed to be done. But what has happened is goog enough, and there are some key matters that actually contribute to this seeming easy and simplicity.

First of all, there have been quite a few small changes and simplifications on the way, including the isolation of the Knowledge object, which represents the information that is shared between Server and Client (plus some additional client-side info). That was done with client-server in mind, to begin to isolate the information package that will have to be wired back and forth.

In addition, there were methods removed, tests revised, conversations with colleagues, and a lot of rumination much of which was written down here, until finally, I saw enough light to do the thing today.

This time, some of you readers might have done better. None of you advised me how to do it, however, so I must assume that you didn’t give it much thought, preferring just to watch me slowly turn over the fire until I was done. And that’s fine. But my point is, you may one day encounter a situation that seems impossible to refactor, inducing you to want to rewrite a bunch of code. And you get to decide what to do.

My experience is that rewriting, beyond just a few lines, is rarely the best idea. The rewrite seems pure and good but in fact it looks good mostly because the details are not clearly dealt with in the picture. Without that clarity, the rewrite goes awry in similar and different ways from the original. Once we become clear on the details, my experience is that the refactoring is quite often easier to do and is almost invariably safer. It can almost always be done in small steps, and the rewrite seem always to require a big bang solution at the end, where we hammer it into the system. And it really won’t quite fit the hole.

That’s not to say that I’d never rewrite something. I might do so, for two possible reasons. One is with intent to put it into the existing system, because I just can’t or just don’t want to refactor, and damn the difficulties, we’re gonna do it. OK, that’s a decision we can make and we get to live with it.

The other reason is just to find out more about what that new solution looks like. In that case, we would write it in a separate sandbox and learn from it. Possibly it would look good enough to finish and put in. Very commonly, I find that it informs me and then I use the ideas in a refactoring approach. In this series or articles, I tried a few little objects, like the Population and the two Proxy objects ClientProxy and WorldProxy. Those gave me a better sense than my imagination or my sketches could give me of what the call and response might look like.

I think this refactoring shows promise and that it will go in smoothly over about two days. And I think the preceding few days of fumbling and confusion are the investment that make what’s about to happen possible.

It’s kind of Big Up Front Design, except that it wasn’t big and it wasn’t exactly design, and we certainly did not get a design document or an architecture department out of the deal.

What should you do? I’m not here to say. I’m here to say that even in messy code, there is quite often a series of small improvements that will remove the mess. Is there always that way out of a mess? I don’t know that, but I would always put in a good chunk of effort to find out, because when there is, it’s almost certain to be a better way to go.

We’re nearing the thrilling conclusion of our setting up for client-server. See you next time!