State Machine
Alone again, naturally, as the song says. I propose to convert to a more robust form of state machine this morning. Results cheer me up a bit, sorely needed.
In my cunning scheme, based entirely on some brisk rereading of ideas on Python state machines, including but not limited to something from Bruce Eckel, I propose a new state machine, implemented as one class per state.
I am not at all sure how to put this in place in small steps. Think with me …
In the new scheme, each state of the machine will be an instance of a class named for that state. The instance will have access to whatever information from the Bot and World we may need, either passed in on creation (and passed along) or passed in as parameters to the instance methods. I think the latter is probably better.
There will be two public methods on each class.
The next method must return an instance of some class in the machine, either itself, if the state persists, or a new instance of another machine state. I think that next may not require any parameters, but we’ll see.
The other method, I think I’ll call action. The action will return a list of actions for the Bot to perform, such as “move”, “change_direction”, “take”, and so on. I suspect that along the way, the action method will send messages to the bot, since that’s how our connection to world works now, and that we’ll have to evolve into the list interface.
That’s a very big bite. We need a smaller bite.
So … what if we were to pull the entire state structure out of the Bot into a Method Object, and work from there. We might be able to break out separate state classes one at a time until done.
We’ll try that.
First Move: Method Object.
Here’s our current situation:
class Bot:
def do_something(self):
self.update()
self.state()
self.move()
def update(self):
pass
def walking(self):
if self.tired <= 0:
self.state = self.looking
def looking(self):
if self.inventory:
self.tired = 5
self.state = self.laden
return
if self.can_take():
self.take()
def laden(self):
if self.has_no_block():
self.tired = 5
self.state = self.walking
return
if self.tired <= 0:
if self.can_drop():
block = self.inventory[0]
self.world.drop_forward(self, block)
I do not know the official list of moves for Extract Method Object, and I’m not going to look them up. I’m strange that way, among others. I do think it would be reasonable to TDD a bit and create the object with some helpful tests. Let’s create a new test file.
class TestMethodObjectStateMachine:
def test_hookup(self):
assert 2 + 1 == 4
We have a failure, we are hooked up. Let’s get down to it. I only write this much of my first test and already I’m not sure what I am about to do:
def test_starts_walking(self):
pass
My concern, I think, is mostly about how I’ll evolve this to the multi-class version. Let’s ignore that concern and follow our nose. We do want to start it state walking so let’s do this:
class TestMethodObjectStateMachine:
def test_starts_walking(self):
machine = Machine()
assert machine.state == "walking"
And, trivially:
class Machine:
def __init__(self):
self.state = "walking"
What I want to do now is paste all the state machine code from the Bot into my new class and see what I need to do to make it work.
Practically every line in the Machine class is now squiggled yellow, since almost nothing is understood:
class Machine:
def __init__(self):
self.state = "walking"
def walking(self):
if self.tired <= 0:
self.state = self.looking
def looking(self):
if self.inventory:
self.tired = 5
self.state = self.laden
return
if self.can_take():
self.take()
def laden(self):
if self.has_no_block():
self.tired = 5
self.state = self.walking
return
if self.tired <= 0:
if self.can_drop():
block = self.inventory[0]
self.world.drop_forward(self, block)
But now I can see what I might do. First of all, we’ll pass in a bot on creation, and then we can talk to it from these methods. And our state member needs to be self.walking, not walking. Like this:
class TestMethodObjectStateMachine:
def test_starts_walking(self):
bot = Bot(5, 5)
machine = Machine(bot)
assert machine.state == machine.walking
And:
class Machine:
def __init__(self, bot):
self.bot = bot
self.state = self.walking
def walking(self):
if self.tired <= 0:
self.state = self.looking
def looking(self):
if self.bot.inventory:
self.tired = 5
self.state = self.laden
return
if self.can_take():
self.bot.take()
def laden(self):
if self.has_no_block():
self.tired = 5
self.state = self.walking
return
if self.tired <= 0:
if self.can_drop():
block = self.bot.inventory[0]
self.bot.world.drop_forward(self, block)
In the above, I forward some messages to my bot instance, but some are helper methods that need to be moved over from the bot to the machine …
No, I think this will go better if I begin by forwarding everything back to the Bot, like this:
class Machine:
def __init__(self, bot):
self.bot = bot
self.tired = 10
self.state = self.walking
def walking(self):
if self.tired <= 0:
self.state = self.looking
def looking(self):
if self.bot.inventory:
self.tired = 5
self.state = self.laden
return
if self.bot.can_take():
self.bot.take()
def laden(self):
if self.bot.has_no_block():
self.tired = 5
self.state = self.walking
return
if self.tired <= 0:
if self.bot.can_drop():
block = self.bot.inventory[0]
self.bot.world.drop_forward(self, block)
I am losing motivation to do more explicit tests on this. I think it is nearly ready to plug 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 = None
self.tired = 10
self.state = Machine(self)
And …
class Bot:
def do_something(self):
self.update()
self.state.state()
self.move()
And …
class Machine:
def __init__(self, bot):
self.bot = bot
self.tired = 10
self._state = self.walking
def state(self):
return self._state()
I thought this might work, and during the editing three tests were broken (surely the ones that I wrote just the other day), but one test is still broken:
def test_bot_notices_a_block(self):
world = World(10, 10)
bot = Bot(5, 5)
bot.tired = 0
bot.direction_change_chance = 0
world.add(bot)
block = Block(7, 5)
world.add(block)
bot.do_something()
assert bot.state == bot.looking
bot.do_something()
assert bot.has(block)
Expected :<bound method Bot.looking of <bot.Bot object at 0x106550050>>
Actual :<machine.Machine object at 0x106550080>
Ah that can’t work, because now, as things stand, bot.state never changes, it is always the Machine. This test can be made to pass like this:
def test_bot_notices_a_block(self):
world = World(10, 10)
bot = Bot(5, 5)
bot.state.tired = 0
bot.direction_change_chance = 0
world.add(bot)
block = Block(7, 5)
world.add(block)
bot.do_something()
assert bot.state._state == bot.state.looking
bot.do_something()
assert bot.has(block)
Note that I had to change the setting of tired as well as the assert. Test is green. I have no recourse now but to run the game and see if it works. I am very hopeful.
Well, it nearly does: the bots run around. But they never pick up any blocks. Why? Because the walking state never gets tired of walking, I think. Let’s do this:
class Machine:
def walking(self):
self.tired =- 1
if self.tired <= 0:
self._state = self.looking
Now they pick up blocks but never put them down. We need to be updating tired all the time. Let’s mash this in:
def state(self):
self.tired =- 1
return self._state()
File "/Users/ron/PycharmProjects/hbiots_reclone/world.py", line 67, in bots_next_location
location = bot.location + bot.direction
^^^^^^^^^^^^
AttributeError: 'Machine' object has no attribute 'location'
Oh. Fortunately I walked down the hall. I think we need always to return self from state(). Otherwise we’re returning a method in our object, which is not really right: the end result of all this will be to return new objects, depending on the state.
def state(self):
self.tired =- 1
self._state()
return self
But that’s not the bug above.
def laden(self):
if self.bot.has_no_block():
self.tired = 5
self._state = self.walking
return
if self.tired <= 0:
if self.bot.can_drop():
block = self.bot.inventory[0]
self.bot.world.drop_forward(self.bot, block)
This isn’t good code, it’s inconsistent, in that we can legitimately send messages to our bot but not to its members. But I think it will work and then we can make it right.
As expected, the game is now running as before. Can we remove the state methods from Bot?
The game is perfectly happy but three tests fail. They’ll probably be our three relatively new state tests, from yesterday I think it was.
Yes, it’s those. Let’s just move those to the new test file and fix them up, if we can. No, wait. Make them run where they are, then move them.
Here’s the easiest one:
def test_laden_goes_to_walking_if_no_block_in_inventory(self):
bot = Bot(5, 5)
bot.tired = 0
bot.state = bot.laden
bot.state()
assert bot.state == bot.walking
What would that test look like against our new machine? And aren’t we going to have to change it again as the machine evolves? Yes, maybe. Deal with it.
I’ll move that one over and change it.
def test_laden_goes_to_walking_if_no_block_in_inventory(self):
bot = Bot(5, 5)
machine = Machine(bot)
machine.tired = 0
machine._state = machine.laden
machine.state()
assert machine._state == machine.walking
That’s green. Move another.
def test_looking_goes_to_laden_if_block_in_inventory(self):
bot = Bot(5, 5)
machine = Machine(bot)
vision_list = [('R', 5, 5)]
bot.vision = vision_list
machine._state = machine.looking
machine.state()
assert machine._state == machine.looking
bot.receive(Block(2, 2))
machine.state()
assert machine._state == machine.laden
Passes. Woot! One more:
def test_laden_stays_laden_if_cannot_drop(self):
# we call state() twice to ensure round trip update
bot = Bot(5, 5)
machine = Machine(bot)
entity = Block(3, 3)
bot.receive(entity)
vision_list = [('R', 5, 5), ('B', 6, 5)]
bot.vision = vision_list
bot.direction = Direction.EAST
machine._state = machine.laden
machine.state()
assert bot.has_block()
assert machine._state == machine.laden
machine.state()
assert bot.has_block()
assert machine._state == machine.laden
Green. Remove the now empty old test file. Green. Game works as always. Commit: convert bot state machine logic to method object. Surely needs improvement and more tests but old and new tests are green.
Reflection
Let’s review the new machine code, see what we see.
class Machine:
def __init__(self, bot):
self.bot = bot
self.tired = 10
self._state = self.walking
def state(self):
self.tired =- 1
self._state()
return self
def walking(self):
if self.tired <= 0:
self._state = self.looking
def looking(self):
if self.bot.inventory:
self.tired = 5
self._state = self.laden
return
if self.bot.can_take():
self.bot.take()
def laden(self):
if self.bot.has_no_block():
self.tired = 5
self._state = self.walking
return
if self.tired <= 0:
if self.bot.can_drop():
block = self.bot.inventory[0]
self.bot.world.drop_forward(self.bot, block)
We have made one sneaky change: the decrementing of tired on every state transition. This may lead to issues when we replace this with a machine design with separate classes: we’ll need to remember to do it.
I see one thing that I think surely needs improvement: there is just one call to bot that accesses bot.world. That should be deferred over to the bot, I think. We have bot.take, why not have bot.drop?
class Machine:
def laden(self):
if self.bot.has_no_block():
self.tired = 5
self._state = self.walking
return
if self.tired <= 0:
if self.bot.can_drop():
block = self.bot.inventory[0]
self.bot.drop(block)
class Bot:
def drop(self, entity):
self.world.drop_forward(self, entity)
Good. Commit: refactor to make Demeter happy.
Aside from that, We’d like not to have the bot owning a machine that has a link back to the bot. GeePaw suggested that the machine might better have an object that bot also has and passes down to the machine. Probably “state-relevant-details” or something. We can explore that in due time. For now, we have done what we set out to do, to replace the explicit state transition code in bot with a single method object that does the job.
And it went fairly smoothly.
One thing that I did that is questionable was to run the game to find out what was wrong with the Machine, rather than write tests for it. I’m not proud of that, but I do think it was the shortest path to working, since the failures were either visible on screen or actual crashes, and each one led to an immediate fix.
TDD is not a moral imperative, it is just a very good way of working. A better way than what I did would have been to have written a test for each situation that I found, and make that test run. I just didn’t think of that, despite that I must have recommended it a thousand times. For my penance, I’ll look at those changes next time and see about adding those tests.
Unless I conveniently forget, but I’ll seriously try not to forget.
All in all, a pleasing morning. It lifts my spirits a bit, which were down because last night I made a serious unintended error and hurt my friend with a careless and cruel remark. I have apologized, of course, but the pain is still there on both sides.
So doing something fairly well kind of reassures me that I’m not a total write-off.
See you next time!