Wrong, Wrong, Wrong
Our little Bot just shouldn’t be this complicated. It’s not getting better. Time to regroup and change course.
It is 0545 on a Wednesday morning, and I woke up about 0445 for reasons and have been thinking about our bot and this state machine stuff. It just isn’t right. I’ve been chasing my tail and I haven’t even caught it. My plan this morning is to gently but firmly rip it all out and do it again.
I am tempted to do just that: it wouldn’t take long. But I’m not going to be quite that random. I’m going to clearly state what needs to be done and test it into existence. Join me.
Bot Behavior
The Bot’s desired behavior is very simple. I’ll include here things that it currently does and some that it does not, because the whole thing just shouldn’t be that big a deal.
- The Bot can only do four things: change direction, try to take a step, try to pick up a block, try to drop a block.
- The Bot only knows five things: has it moved since last time, is it tired, does it have a block just now, is there a block in front of it, is there a space in front of it.
The desired behavior is to produce a list of things that the Bot wants to do in this turn, consisting of
- Possibly trying to pick up a block;
- Possibly trying to drop a block;
- Possibly changing direction;
- Trying to take a step;
There are many more things that we might want our Bot to do in the future, but this is now and we want a simple solution to what we need now, not a complex solution to a problem we do not have.
Reflection / Planning
I am settling down now, less inclined to rip and tear at the code. But I still think there’s way too much, and even though I believe we “need” a state machine, we don’t really need one now, and there is, for now, surely a simpler way to express the desired behavior.
That doesn’t mean that I’m going to write out a long procedure full of random calculations and if statements. I’m going to try to express what we need in tests, and make it work with sensible simple objects. I might even first make our current code behave a bit more like what it says above, or I might not. I don’t know yet.
I’m thinking just now about one possible object, maybe called BotKnowledge, which is an object that can answer the five knowledge questions above. To do that, it needs to know the bot’s previous location, current location, inventory, and vision.
As our Bot becomes more intelligent, if in fact it ever does, we’ll probably need to add in more knowledge.
Another object, and I’m even less certain about this one, might be called the BotBrain or BotDecider, which takes the BotKnowledge (BotSituation?) and produces a list of commands to perform. Right now, I figure those could be strings or an enum, ‘take’, ‘drop’, ‘turn’, ‘step’.
Hm, those sound interesting. Let’s do some TDD.
class TestDecisions:
def test_hookup(self):
assert 2 + 1 == 4
I think the Bot will start with an empty knowledge and update it on each cycle. Let’s work without an actual bot for a while.
How about this for a first test:
class TestDecisions:
def test_initial_knowledge(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
assert knowledge.has_moved
With lots of help from PyCharm:
class Knowledge:
def __init__(self):
self._old_location = None
self._location = None
@property
def location(self):
return self._location
@location.setter
def location(self, location):
self._old_location = self.location
self._location = location
@property
def has_moved(self):
return self.location != self._old_location
Curiously enough, this test does not pass, because the != fails because None is not a location.
Fix Location.
class Location:
def __eq__(self, other):
return self.x == other.x and self.y == other.y
That needs to be:
def __eq__(self, other):
return isinstance(other, Location) and self.x == other.x and self.y == other.y
Green. Commit: improve Location, beginning new tests for Bot decisions.
def test_move(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
knowledge.location = Location(10, 9)
assert knowledge.has_moved
That passes. Commit: additional testing.
def test_no_move(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
knowledge.location = Location(10, 10)
assert not knowledge.has_moved
Also passes. Commit again.
Reflection
I feel pretty good about this so far. What else do we need? We need a vision for vision testing.
def test_vision(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
knowledge.vision = vision_list
assert knowledge.can_take
This needs a vision_list, I think, but I want to look at Bot to see what we know now and when we know it.
Right now, the World sets the Bot vision directly, giving it a list of locations and names. So we want one of those here. Why? Because I’m thinking that we’ll install this Knowledge thing into Bot and keep all its info in there.
def test_vision(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
vision_list = [('B', 10, 9)]
knowledge.vision = vision_list
assert knowledge.can_take
I realize while doing this that the BK needs to have the Bot’s direction as well. So be it, we’ll add it now:
def test_vision(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
knowledge.direction = Direction.NORTH
vision_list = [('B', 10, 9)]
knowledge.vision = vision_list
assert knowledge.can_take
I’m green with this class, which we need to reflect upon and think about as well as consider and review.
class Knowledge:
def __init__(self):
self._old_location = None
self._location = None
self._direction = None
self._vision = Vision([], None, None)
@property
def vision(self) -> Vision:
return self._vision
@vision.setter
def vision(self, vision_list):
self._vision = Vision(vision_list, self.location, self.direction)
@property
def direction(self):
return self._direction
@direction.setter
def direction(self, direction):
self._direction = direction
@property
def location(self):
return self._location
@location.setter
def location(self, location):
self._old_location = self.location
self._location = location
@property
def has_moved(self):
return self.location != self._old_location
@property
def can_take(self):
return self.vision.match_forward_and_one_side('B', '_')
What is odd about this class, compared to most of what we’ve done in this project, is that it is all properties. In aid of that, it keeps all its actual knowledge private and wraps it all in properties. I grant that this is different, but I think I like it.
What else did we say that a BotKnowledge knows? Oh, right, whether we have a block, and whether we can drop one. Let’s give it receive and remove:
def test_has_block(self):
knowledge = Knowledge()
assert not knowledge.has_block
Oh, I forgot to commit. Remove that for a moment and commit: added can_take
Put that test back, it fails. Fake it:
@property
def has_block(self):
return False
Pass. Commit: initial has_block
def test_has_block(self):
knowledge = Knowledge()
assert not knowledge.has_block
block = Block(3, 3)
knowledge.receive(block)
assert knowledge.has_block
There is a decision to be made here. Currently the Bot has a list for inventory and it only sort of half maintains it. We could echo that, on the grounds that we’re going to need it, or we could just save the block if we get one and clear it if we don’t.
Let’s do the simple thing.
def receive(self, entity):
self._entity = entity
@property
def has_block(self):
return self._entity
Green. Commit: Can receive and detect block. Enhance the test:
def remove(self, entity):
if self._entity == entity:
self._entity = None
Test passes. Commit: can remove and detect missing block.
What else? We want a method for ability to drop our block. Or whatever we have. Let’s add can_drop:
def test_can_drop(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
knowledge.direction = Direction.NORTH
vision_list = [('B', 10, 9)]
knowledge.vision = vision_list
assert not knowledge.can_drop
Fake it:
@property
def can_drop(self):
return False
Could commit, but let’s do the other part:
def test_can_drop(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
knowledge.direction = Direction.NORTH
vision_list = [('B', 10, 9)]
knowledge.vision = vision_list
assert not knowledge.can_drop
knowledge.vision = [('B', 9, 9)]
assert knowledge.can_drop
Failing of course … so:
@property
def can_drop(self):
return self.vision.match_forward_and_one_side('_', 'B')
And while we are at it:
def test_can_drop(self):
knowledge = Knowledge()
knowledge.location = Location(10, 10)
knowledge.direction = Direction.NORTH
vision_list = [('B', 10, 9)]
knowledge.vision = vision_list
assert not knowledge.can_drop
knowledge.vision = [('B', 9, 9)]
assert knowledge.can_drop
knowledge.vision = [('B', 11, 9)]
assert knowledge.can_drop
Green. Commit: can_drop complete
Reflection
We have a nice little object here, and it seems to be what is needed. I emphasize seems, because it’s not in use and its spec was the one that I dreamed up just at the beginning of this article. We need to know whether it’s actually useful at all. Let’s see whether we can plug it into Bot. Here’s Bot now:
class Bot:
def __init__(self, x, y, direction=Direction.EAST):
self.world = None
self.id = None
self.name = 'R'
self.location = Location(x, y)
self.direction = direction
self.direction_change_chance = 0.2
self.inventory = []
self.vision = []
self.tired = 10
self.state = Machine(self)
First thing, I think we need a creation method on BotKnowledge. But for now, we’ll do it the hard way, just adding code into the Bot __init__:
Oh! I forgot to add the tired capability. We’ll do that as we go. And … you know what … I think we’re going to want a general update method as well. Let’s see if we can discover those needs and refactor them in.
class Bot:
def __init__(self, x, y, direction=Direction.EAST):
self.world = None
self.id = None
self.name = 'R'
self.location = Location(x, y)
self.direction = direction
self.direction_change_chance = 0.2
self.inventory = []
self.vision = []
self.tired = 10
self._knowledge = Knowledge()
self._knowledge.location = self.location
self._knowledge.direction = self.direction
self._knowledge.vision = []
self.state = Machine(self)
I plan to reduce that list Real Soon Now. Let’s go thru Bot, changing things to use the _knowledge member.
After a very small amount of scurrying, the tests are green, on this code:
class Bot:
def __init__(self, x, y, direction=Direction.EAST):
self.world = None
self.id = None
self.name = 'R'
self.direction_change_chance = 0.2
self.inventory = []
self.tired = 10
self._knowledge = Knowledge()
self._knowledge.location = Location(x, y)
self._knowledge.direction = direction
self._knowledge.vision = []
self.state = Machine(self)
@property
def direction(self):
return self._knowledge.direction
@direction.setter
def direction(self, direction):
self._knowledge.direction = direction
@property
def location(self):
return self._knowledge.location
@location.setter
def location(self, location):
self._knowledge.location = location
@property
def vision(self):
return self._knowledge.vision
@vision.setter
def vision(self, vision):
self._knowledge.vision = vision
I think there is a issue and I see no test that is dealing with it. I think we’re still keeping track of the inventory in Bot, not in Knowledge. I need a test to show that bug.
def test_bot_uses_knowledge_inventory(self):
bot = Bot(5, 5)
block = Block(3, 3)
bot.receive(block)
assert bot._knowledge.has_block
That fails. Fix receive:
def receive(self, entity):
self._knowledge.receive(entity)
Ten tests fail. Fixing remove had better bring us back to life.
def receive(self, entity):
self._knowledge.receive(entity)
def remove(self, entity):
self._knowledge.remove(entity)
It does not. Uh Oh, I didn’t need this.
Ah. The has method.
class Bot:
def has(self, entity):
return self._knowledge.has(entity)
class Knowledge:
def has(self, entity):
return entity == self._entity
Still three failing … this is getting critical, if it doesn’t work soon I have to roll back.
Ah, the three failing ones are all in the state machine. They check bot inventory:
def walking_update(self):
if self.tired <= 0:
if self.bot.inventory:
self.set_states(self.laden_states())
else:
self.set_states(self.looking_states())
Let’s provide that property:
class Bot:
@property
def inventory(self):
if self._knowledge._entity:
return [self._knowledge._entity,]
else:
return []
This is a bit awkward because a) we’re ripping out the inventory of the Bot and looking at it in the Machine, and b) the inventory format has changed for the simpler and we need to compensate. And c) we need to make inventory be a legitimate list despite the fact that it’s only being checked for being truthy.
A bit messy, that.
But the tests are all green. I’m going to commit this and then see what needs improvement. Commit: Bot now uses new Knowledge object, soon to be passed to Machine or otherwise used in decision logic. Lots of forwarding, needs improvement. I hope this is progress.
Reflection
Installing that object took about 25 minutes, and it seemed longer to me. But it never seemed out of reach, except when I was briefly confused by my first implementation of inventory, which was returning the list [None], which, contrary to what one might expect, is truthy.
The good news is that, aside from tired, what the Bot knows about the current situation is all inside Knowledge. Less good is the fact that there is a lot of forwarding going on from bot to Knowledge. Some of that should sort out once we get a reasonable information package back from the World, which is somewhere on the list of things to do. I think Knowledge needs a real __init__. We change this:
class Bot:
def __init__(self, x, y, direction=Direction.EAST):
self.world = None
self.id = None
self.name = 'R'
self.direction_change_chance = 0.2
self.tired = 10
self._knowledge = Knowledge()
self._knowledge.location = Location(x, y)
self._knowledge.direction = direction
self._knowledge.vision = []
self.state = Machine(self)
To this:
class Bot:
def __init__(self, x, y, direction=Direction.EAST):
self.world = None
self.id = None
self.name = 'R'
self.direction_change_chance = 0.2
self.tired = 10
self._knowledge = Knowledge(Location(x, y), direction)
self.state = Machine(self)
And …
class Knowledge:
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
This breaks all my new tests, of course. They all need construction parameters. I provide them and we are green.
Commit: Knowledge now has complete constructor.
It’s past time for a break. Let’s sum up.
Summary
I have mixed feelings about this. On the dark side, I list:
-
The need for this object was slim. It’s true that Bot’s member list was too wide, but we built this more because I thought we needed it than to improve Bot.
-
Knowledge was built abstractly, from tests and general assumptions about the need for it It might have been better to build it up incrementally, in place in Bot. However, the tests we have are probably far better than we’d have had the other way.
-
At this point, Bot complexity seems higher. There are a few more methods than we had before. Some methods are a bit strange as they translate from Bot style to Knowledge style.
-
The style of Knowledge, mostly presenting properties that can be read and set, is a bit different from what we have done before. We have, for example,
has_blockinstead ofhas_block(). Readers may stumble over that a bit. -
There is a lot of forwarding going on, which may suggest that some objects have a hold on the wrong thine, or that the Bot interface is just too wide. (I suspect both are true.)
On the good side:
-
Operational information (except for tired, which we’ll do shortly) is all conveniently stored in Knowledge.
-
The system’s tests have been shown to be robust, since any of them broke during the installation of Knowledge and were resolved by mating up Bot and Knowledge, rather than by changing tests. The changes needed were real.
-
Once
tiredis in place, I think we can pass Knowledge to Machine in lieu of Bot, and that will be a good thing. We can look at that time at whether some of Bot’s forwarding can be removed.
Overall … I really thought that I was just going to sit down, type in a few if statements, and clean up the whole machine issue. Those are the sorts of thoughts one gets at 5 AM. Then reality gets in the way.
I am feeling more and more sure that I chose a path that went around the long way, but that it will get us there. I suspect that had I had someone to pair with, things would have gone better and in a different direction. But it is what it is, and we’re here now. I think we’re improving things … though perhaps not as rapidly as we might like.
What do you think? Toot me up if you wish!