Crazy Idea

The Repo on GitHub

We try a mad idea. We discover things. We despair. We assess where we are. We get another idea that’s not so good. We roll back. We refactor just the least bit. We do a tiny JSON test. The idea becomes useful. We criticize the code. We devise a plan. Wow.

I have a mad idea:

I propose to secretly rename World to WorldServer or something like that, breaking massive numbers of tests, and then to create a replacement class for World, that is the class we’ll create for client-side tests, which we’ll then make forward to the new WorldServer object.

Before we explain to each other why that won’t quite work (it’s one of those just waking up ideas) let’s look at what it would almost do for us if it were a decent crazy idea. You know, the kind that “just might work”.

We need a clear separation between server—which manages all the objects in the whole game—and client—which manages a local collective of bots exploring the world. Our current program was intentionally written without that, instead with a direct access from bots to world, down at the level of individual messages.

Over the past few days, we’ve looked at some experiments and sketched some ideas for how we’ll do things, with proxies on each side and communications gear in between the proxies. We’re thinking—at least I am—that there will be two kinds of proxies, the production ones that run over the network and a development pair that just forward messages back and forth.

I’m thinking that if I can do this renaming thing just right, I can isolate out the methods that have to be forwarded and turn the results of an experiment like we’re talking about here into the development pair.

A problem that has come to mind is this one: there are two kinds of tests that refer to World class. At least there should be: tests that are really there to test the World class, and tests that use the World class to run scenarios intended to test bots or bot + world behavior. I’m concerned that this may become more confusing than I can deal with.

That, of course, is why rollback was invented. I’ll just do it and see what breaks, and fix up the ones that I think belong to the server side to refer to the new name. Note that we can’t do this with PyCharm rename: it would fix up all the names and that is exactly what we do not want. Let’s just try it!

My PyCharm cursor happens to be right in the middle of the class World line, how convenient. Rename it to ServerWorld, locally, by just typing in the characters. All the tests fail, not even started. I think I’ll try creating an empty class World in this same file, to see if that hooks things back up so that they fail more explicitly.

class World:
    pass

Only 26 fail, 51 now pass. Two thirds fixed with one change! That was a great bug fix right there! Let’s see what’s breaking.

OK, this is fascinating! There appears not to be a single test that is actually a direct test of the World class. I could have missed one, but it appears that Every Single Test referring to World is a story test stuffing bots into the World and using them to see if the World has done what it should. This fact will not impress GeePaw. It will not impress almost anyone I know. It does not impress me.

Why?

What we in my community all believe is that there should be sufficient small tests of the world to ensure that it works, and sufficient small tests of the bot to ensure that it works, and then perhaps a few tests of the combination. That is not what we have. Why do we have that? Well, the way we worked—OK the way that I worked—on this code led me to focus on the bots and use them to drive code into the world. Was that wrong, right, or something else? It had the result that we see: no direct tests for World.

Given my focus and therefore our focus, on ridiculously simple bots, the result is not surprising. And this discovery also explains to me some of GeePaw’s comments about how this program makes him feel. Fascinating.

Moving right along …

But we are here now and we must figure out what to do. We need a new plan. We could roll back, but I don’t think we need to. Instead, what I propose to do is to begin to elaborate the newly created empty World class, giving it a ServerWorld to talk to. We’ll see what we learn. I have a feeling that we’ll learn some things that we like, and some that we do not like. Let’s just see. This morning, I suspect, is going to include a rollback after we learn things. But we’ll see.

class World:
    def __init__(self, max_x, max_y):
        self._server = ServerWorld(max_x, max_y)

class ServerWorld:
    next_id = 100

    def __init__(self, max_x, max_y):
        self.width = max_x
        self.height = max_y
        self.map = Map(max_x, max_y)

Curiously, now 24 tests are failing rather than 21. You’d think this would have made it better. Here’s the first one:

    def test_bot_in_world(self):
        World.next_id = 100
        bot = Bot(10, 10)
        world = World(10, 10)
        world.add(bot)
        point = Location(10, 10)
        assert bot.id == 101
        assert bot.location == point

This suggests a few things to me. First of all, clearly World needs an add method. But mostly, this test is mostly testing ServerWorld behavior to see whether it has correctly given the bot an id and the correct location. There really should be a ServerWorld test for some of that behavior. But let’s do a simple add and see what’s next: I expect it’ll be quite interesting: a message from the server back to the client.

By the way

Should we rename World to ClientWorld right here and now? I’m starting to think it would help. We’ll see.

class World:
    def add(self, entity):
        self._server.add(entity)

Already I do not like this and it’s because we are passing a client entity (a Bot) over to the server. And in fact we get bad news, which is that the test above passes, because, since we actually passed it a Bot, the ServerWorld was able to update it, setting the id and location directly. We can’t allow that. Let’s make a note. I’ll call it a Discovery, not to say that it is a major surprise, but just so that we can go back and find them all.

Discovery

The client World passes actual entities to the world. These are updated directly.

What should really be done here?

The ServerWorld maintains a “world” containing entities such as Bots and Blocks, at locations. The world provides entities with certain affordances, including pick up the thing in the cell in front of you, drop what you are carrying on the empty cell in front of you, take a step, plus a cumulative “scent” of what is near you, and a “vision” of the 8 cells surrounding out.

Currently, the ServerWorld has access to the client’s entity. This is bad.

Discovery

I am used to thinking of what is currently named ServerWorld as World. Whatever renaming we do should cause as little confusion in my mind as possible, while still making sense. The current scheme may not be ideal.

I am feeling anxiety, tension, something. This is probably a sign that this is not going well. But I’m just here to learn: there is no reason to be feeling concern. But I am. (There are other things in life that might make me feel this way but I’m here now and focused here, so I suspect it’s what’s here that’s doing it.)

I have taken a break. Let’s see what I think now.

The pictures from yesterday and the days before include the notion of a BotProxy on the server side, standing in for the Bot, which cannot really be sent over there.

Despair

This is the point on a legacy effort when we despair. The code is so entangled, so different from what it needs to be, that we despair of ever getting it in the shape we need. We start preparing arguments for rewriting the entire application and doing it right this time. Unfortunately for me, my boss (me) has had experience with rewrites and none of them ever went well enough to justify the effort. Some never shipped, some shipped late, none were the resounding success we had hoped for. I’m sticking with the run. I’d abandon this effort and let the Bots starve before I’d rewrite it from scratch.

Not that that would be all that hard, there are only twelve or fifteen classes in here, I’m sure we could just whip them right out and really a lot of them would be just fine the way they are and there’d be lots of reuse and reuse is really a good thing and I’m sure that in a week, at most two, I mean possibly three if all kinds of things went wrong but they won’t likely go wrong, we’d be up and running again on a much better code base.

Sorry, no. Not gonna do it. There’s always a way to refactor to a better design, and our job is to find it.

What Do We Know, What Do We Do?

We cannot send a bot across the wire. What can we send? Well, Bots do have an id, which we created with the intended purpose of using it as the shared way to identify a bot between client and server. So we could pass the bot over. We also have the relatively recent Knowledge object, which contains just about everything the Bot needs to know on the client side:

class Knowledge:
    drop_threshold = 4
    take_threshold = 7
    energy_threshold = 5

    def __init__(self, location, direction):
        self._old_location = None
        self._location = location
        self._direction = direction
        self._vision = Vision([], self.location, self.direction)
        self._entity = None
        self._energy = self.energy_threshold
        self.scent = 0

Looks to me as if we ought to rename scent to _scent. Can’t do that right now, too many balls in the air but we’ll remember next time.

There are some elements there that are pretty clearly shared with the world, or should we say dependent on what the world thinks, and some that are not. I think that _old_location is of no interest to the world, and at least now, the _energy is a property that is unique to our particular kind of Bot. I suspect that the world may also impose some kind of energy notion on bots, as we evolve it into a more rich environment worthy of exploring, but this _energy is kind of the mental energy of our particular ant-like bots.

The _entity member, by the way, is the entity we are carrying it. It used to be a list, _inventory. We simplified that a while back because we only allow one thing carried at a time.

As things stand, all the shared information is only output from the world, except for _direction. We do not have a command in world to change direction: we just change it directly and, as things stand now, the world honors its setting.

All the rest is, in essence, the information the world returns to us.

OK, some useful ideas are emerging from my brain fog. The Knowledge object is (almost) a shared data structure that server and client can use to communicate and agree upon, regarding the status of an entity.

Whoa! Idea!!

Suppose that our current little World object, instead of passing our Bot over to the World, were to produce a copy of our Bot, including a copy of the Knowledge and send that over. That would break the connection between the two sides, but would leave the server side still correctly updating the bot (copy). But the client side World object would still have ahold of the copied Bot, and could therefore copy its information back to the “real” Bot. Almost good, but it leaves the world running too much client-side code.

I’m going to roll back. This crazy idea might work but for now it has provided enough thinking and if I did want to push it further it would be easy. We need to get to green so that we can make some small changes.

Since PyCharm is open on Knowledge, I’m going to gently rearrange it.

class Knowledge:
    def __init__(self, location, direction):
        # shared info
        self._direction = direction
        # world write / client read only
        self._entity = None
        self._location = location
        self._scent = 0
        self._vision = Vision([], self.location, self.direction)
        # local info client
        self._energy = self.energy_threshold
        self._old_location = None

Green. Commit: tidying Knowledge, local and shared notion.

I don’t usually condone comments like that, but they are there to delineate things for changes that we’ll probably make, because I’m moving in the direction of having the code pass a thing like Knowledge back and forth between the sides, which I think will make us a lot happier.

To that end, the object’s id should be part of its knowledge, it’ll be the key used to connect server and client. So we’ll give out Bot class a property for id instead of its current settable member.

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()

    @property
    def id(self):
        return self._knowledge.id

    @id.setter
    def id(self, id):
        self._knowledge.id = id


class Knowledge:
    drop_threshold = 4
    take_threshold = 7
    energy_threshold = 5

    def __init__(self, location, direction):
        # shared info
        self._direction = direction
        # world write / client read only
        self._entity = None
        self._id = None
        self._location = location
        self._scent = 0
        self._vision = Vision([], self.location, self.direction)
        # local info client
        self._energy = self.energy_threshold
        self._old_location = None

    @property
    def id(self):
        return self._id

    @id.setter
    def id(self, id):
        self._id = id

Green, commit: move id into Knowledge.

Have you noticed that suddenly I’ve just swung into action, as if I know what I’m doing? Well, I kind of almost do know. At least, I have a tentative direction in which I’m moving. It comes from the Whoa! idea about copying the bot, but instead we’ll copy Knowledge!

We’re going to isolate all the information that is shared between server and client, about a bot, into the Knowledge class, and we are going to declare that information as the official way that we communicate back and forth.

I plan, as early as I can, but probably not this morning, to pack up the knowledge object into something more generic, perhaps a dictionary, and somehow copy that back and forth between the two sides. That dictionary, or whatever it is,, will represent the data part of the commands that pass back and forth. Something like turning step into a command “step” with an associated Knowledge dictionary, which results in an updated dictionary coming back.

I can imagine readers out there going “Dammit, Ron, just create a message and a response and get it over with!” If those readers were here, I’d say “show me” and get them to point to the place in the code where we can do that. I don’t think there is such a place, because of the design decision that I made that allows a bot to talk directly to the world and the world to talk directly back.

But if there is such a place, or you see an easy place to make one, by all means let me know. I think I’ll find my way, but if there is a better path, I would love to have it shown to me.

Looking to the future

The theory of this effort, at one time, was that players could write their own bot programs, in any programming language, and connect to our world, written in whatever language it was in, Python as it happens, and their bots could play in our world. So the information that passes across the wire needs to be generic enough so that any language can reasonably create it. That will probably come down to something somewhat simple, like a JSON representation of a dictionary. That’s my current guess, and that’s why I’m moving that way.

I just did this tiny test:

    def test_json_dict(self):
        dict = {'a': 1, 'b': 2, 'c': 3}
        json_str = json.dumps(dict)
        assert json_str == ''

It fails like this:

Expected :''
Actual   :'{"a": 1, "b": 2, "c": 3}'

So, right, that can probably work pretty well. What if we tried to pass something real though? We’d get a message like this:

TypeError: Object of type Location is not JSON serializable

No big surprise there. Anyway, we can sort it out readily I’m sure, by judiciously deciding what kind of information we share back and forth. I bet it can do a tuple. Yes:

Expected :''
Actual   :'{"a": 1, "location": [5, 3], "c": 3}'

Converted (5, 3) to JSON array, just as you’d hope. So, one way or another, JSON or something like it will be OK. Probably JSON, because it has become pretty standard across languages.

Time to break so let me sum up.

Summary

What we have here is me experiencing the result of a few mistakes. Since my profession here at ronjeffries.com is making mistakes and writing about them, this is a good thing, because there’s no danger of running out of things to write about. What are some of the key decisions that we might call mistakes?

Client-Server

We knew going on that this was a client-server application. I made the explicit decision to ignore that fact as long as possible, because I wanted to see and show what happened if we did that, and how we’d move forward to get the feature in. We are presently engaged in that experience, and have been making tiny steps and small discoveries. Who knows, any moment now we may break loose!

Two-Way Messaging

I made an odd decision, and I think I made it lightly, that the Bot would send a command to the world, like world.step(bot) and the world would send a message back, like entity.location=location from somewhere deep in the World. I was thinking of both sides of the system being independent actors, neither really subordinate to the other, just sending messages back and forth. Had I chosen for the World to return information to the Bot, this all might have been a lot easier. But I do like the independent actors style, though I may be the only person on the team who does. I’m also nearly the only person on the team, so … where was I? Oh right:

Story Tests

Most of the tests for interesting behavior involve both one or more Bots and the World. This means that these tests cannot run without the other side. It has also led to a situation where, to often, we determine whether the right thing happened by checking some other matter entirely. We determine whether the pattern worked by checking whether we actually dropped the block, things like that. I know that GeePaw, in particular, dislikes those tests, and he has been more kind than I may deserve in how little and how gently he has mentioned them. Which is just as well, because if he pushed me, I’d just say “show me” and then one of us would learn something, and with luck, we both would.

No Direct World Tests

This is a fascinating fact. There are about 80 lines of code in World and not a single test that can justly be considered a test of that class. It is well exercised during testing, but all via those story tests.

The Objects Aren’t Helping

This phrase is a common one in my lexicon, even if I don’t use it very often. When a design is really nice, the work is done by small objects that have simple behavior and it all adds up to something that is clear at every level and easy to understand. There are places in this design that are like that, and I think we’re getting more such places, but there are others where the objects are more in the way than they should be. Story tests are often a sign of this issue, and, possibly, the use of Fake objects in tests may also suggest that the real objects aren’t helpful.

In this case, the objects aren’t helping us envision the server-client relationship. It’s possible that if this were not trying to be a client-server program, the Bot-to-World direct connection would be a good thing. However:

Many Pass-Thru Properties

Bot has no fewer than six two-sided properties, providing getter and setter that access the Knowledge member, through the bot. In a bad mood I would describe that as a semi-clever way to disguise a Law of Demeter violation, because it allows writing code like this: bot.location=new_location where what’s really happening is bot.location._knowledge.location=new_location. The object of interest is the Knowledge, and we have the bot.

This observation may lead to a good thing, because we’re coming to see that the Knowledge is the basis for the object that will be serialized back and forth across the wires, so that unwinding all this tampering with Demeter may be just what we need to do to get client-server in place.

I said we might call the above items mistakes. They are certainly properties that the code has, and some of them, at least, like story tests, and things we know to be undesirable. But you write one and then it’s not so bad writing the next one and after a while you’re used to the pain. We’re human, we do things that are not perfect. It’s one of the reasons why we do better programming in pairs and small groups: we’re more likely to pick up on these issues and someone in the room is likely to have the energy and insight to make things better.

This is good: I get to blame people who were never here for the things that we don’t like in this code. Excellent! It is Chet’s fault! Outstanding!

Is this a path I see before me?

What I want to happen is one of two things:

1. Through a series of small steps like the ones taken in the past few days, more and more of them going into the running code, we one day find ourselves with client-server all nicely captured;
2. Failing that, after a series of small changes, there is just one day where we tear up a lot of stuff and put it together, with client-server at the end of the day, even if we couldn’t commit for a few hours.

I haven’t given up on #1, and I would always have settled for #2 if I saw something that could be done in a day so far I have not seen that. But I have the glimmers of a plan, based on the experiments of the past few days. It goes something like this:

1. Continue to isolate the parts of Knowledge that are “shared”;
2. Change World to be given a Knowledge, not a Bot;
3. Change World to receive a copy of our knowledge and update it;
4. Change World to return its copy to us, where we copy it back into our copy.

I think that could work. I suspect we can do part of it in a form that we can commit. If not, I think a spike may set us up with enough understanding so that we can do much of the prep work, and then finish up in a separate session.

We might be onto something. I’m glad we had this chat! See you next time!