I’ve decided to deviate from the spec to make my work with this example more interesting (to me). I anticipate no real difficulty with this extensive set of changes.

I find doing a look on every move to be tedious, and I think the notion of only seeing in the x direction and y direction is silly. Therefore I propose the following changes to the spec.

  1. The look command will examine all cells within visibility distance of the robot;
  2. Each move will automatically return look results, like a radar that is always scanning;
  3. Enhance the look command to return a special result for “shadowed” cells, that is, those that are blocked from the radar by an impenetrable obstacle.

I expect the first two of these to be pretty easy. Number 2 should be quite straightforward, and number 1, I think, will be a localized change as well. Number three … well, honestly I’m not sure how I’ll do that. Let me explain:

I figure that the thing to do with a “circular” scan is just to generate all the cell coordinates with distance less than visibility, and return their LookPacket. Should be seriously easy. However … deciding what it means for a cell to be in the shadow of another … I don’t see how to readily do that.

In a “real radar”, each probe takes place at some particular angle, and it is a very narrow peek at whatever’s out there, and when the ping bounces back, we draw a pip at the distance the thing reflected from. That’s surely done by some simple calculation involving the speed of light. So if we scanned something not very tall and close in, we’d see its pip, and if the same scan hits something taller and further out, we’d see that pip too. And if the first thing is tall enough, then things behind it don’t get scanned and are therefore in the shadow of the tall thing.

My intended implementation is just to take a table of cells and scan them all. It makes little sense to scan square cells radially. Even if we were to sort the cells by increasing distance, how will we decide that, say, (3,2) shadows (4,2) (which I think it probably should)?

Well, that’s story 3. We’ll worry about it later. I’m sure we’ll think of something, even if it’s just a bottle of whiskey to induce the product owner to lose the card.

One more thing …

We’re going to be generating a lot of scan information with these changes. We’ll probably need to resolve this sticky note on my desk that says “tix fnoslady fov nultiok faetg venove dips”.

Let’s get to it.

Look in a Circle

I thought briefly of using manhattan distance for this story but that was before I was fully awake. I’m going to use regular vector distance. Here’s how we do look now: It’s rather complicated and I bet we wind up making it easier.

function World:look(robotName)
    local robot = self._robots[robotName]
    local packets = {}
    if robot then
        local lens = self._knowledge:newLensAt(robot._x, robot._y)
        self:lookNSEWinto(packets, lens)
    end
    return self:jsonLookResult(robot, packets)
end

function World:lookNSEWinto(packets, lens)
    local direction = {
        east= {mx= 1,my= 0},
        west= {mx=-1,my= 0},
        north={mx= 0,my= 1},
        south={mx= 0,my=-1}
    }
    self:lookInDirection(packets,lens, direction.north)
    self:lookInDirection(packets,lens, direction.south)
    self:lookInDirection(packets,lens, direction.east)
    self:lookInDirection(packets,lens, direction.west)
end

function World:lookInDirection(packets,lens, direction)
    for xy = 1,self.constants.visibility do
        self:lookFactInto(packets,lens, direction.mx*xy, direction.my*xy)
    end
end

function World:lookFactInto(packets, lens, x,y)
    table.insert(packets, self:createPacketFor(lens,x,y))
end

This code came about via several refactoring passes to remove duplication, if I correctly recall. It does some clever manipulation of x and y to allow things to fold down the way they do.

For our purposes, I think the only thing of note is probably lookFactInto, which takes the current lens and x and y coordinates, and tucks away whatever packet we come up with. The packet can be nil and this code takes advantage of the fact that appending a nil to a table is harmless.

What I think we want is to generate the sequence of x and y coordinates that are within visibility range, and feed them into lookFactInto.

I’m not sure what tests we have for look, but in the spirit of seeming to be a good person, I think I’d like to write one that generates the cells at a given distance. Honestly, though, I’m not sure how to specify the output, and I’m really quite sure how to do it.

OK, a spike. I’m on a clean commit.

function World:look(robotName)
    local robot = self._robots[robotName]
    local packets = {}
    if robot then
        local lens = self._knowledge:newLensAt(robot._x, robot._y)
        self:lookNSEWinto(packets, lens)
    end
    return self:jsonLookResult(robot, packets)
end

By intention:

function World:look(robotName)
    local robot = self._robots[robotName]
    local packets = {}
    if robot then
        local lens = self._knowledge:newLensAt(robot._x, robot._y)
        local cells = self:cellsAtDistance(self.constants.visibility)
        for i,c in ipairs(cells) do
            self:lookFactInto(packets, lens, c.x, c.y)
        end
    end
    return self:jsonLookResult(robot, packets)
end

This is supposed to generate cells that contain x and y and are at the right distance. I can code that:

Well, I thought I could, but I got weird results. Here’s what I coded, for the record:

function World:look(robotName)
    local robot = self._robots[robotName]
    local packets = {}
    if robot then
        local lens = self._knowledge:newLensAt(robot._x, robot._y)
        local cells = self:cellsWithinDistance(self.constants.visibility)
        for i,c in ipairs(cells) do
            print(c.x," ",c.y)
            self:lookFactInto(packets, lens, c.x, c.y)
        end
    end
    return self:jsonLookResult(robot, packets)
end

function World:cellsWithinDistance(d)
    local cells = {}
    for x = -d,d do
        for y = -d,d do
            local v = vec2(x,y)
            if v:len() <= d then
                table.insert(cells,v)
            end
        end
    end
    return cells
end

That seems quite OK to me, and I did examine the values (see the print there?) and they make sense to me, but the scan results on the screen do not.

weird screen results

Depending on where I go, I get those strange patterns of obstacles and pits, but the creation code for those just produces single rows. I revert. Here’s what the correct map looks like when I wander around enough:

correct world map

There are just single rows of obstacle and pit, length 7 and 5 respectively.

Ohh, I see. The rules of what comes back from look don’t support this feature!

We can only send back direction and distance, and direction is N, S, E, or W.

OK, I knew that was a dumb rule. For this feature to work we really need a new kind of agreed response, with relative x and y, not that silly direction-distance thing.

Let’s drill into the LookPacket stuff a bit.

LookPacket

We create them here:

function World:createPacketFor(lens,x,y)
    local content = lens:factAt(x,y)
    if not content then return nil end
    local dir = self:directionTo(x,y)
    local distance = math.max(math.abs(x), math.abs(y))
    return LookPacket(dir, distance, content)
end

That’s called from the function we thought would work for us:

function World:lookFactInto(packets, lens, x,y)
    table.insert(packets, self:createPacketFor(lens,x,y))
end

It seems clear that we need to revise LookPacket to include x and y and content, not direction, distance, and content, and that when we install them, we need to use them accordingly. Fortunately, the LookPacket has a method that gives us the x and y we need:

function LookPacket:asFactDetails()
    local convert = { N={0,1}, S={0,-1}, E={1,0}, W={-1,0} }
    local mul = convert[self._direction]
    if not mul then return nil,nil end
    return self._type, self._distance*mul[1], self._distance*mul[2]
end

We built that method to give us what we want, the fact type and the x and y coordinates for the fact. It happens that this code knows that only x or y can be non-zero, not both, in converting from NEWS to x,y.

Let’s try another spike. This time I’m going to change the LookPacket to include x and y, not direction and distance. We’ll have to change quite a bit of code, and we’ll have to toss the spike, but I need to learn how to do this.

Forgive the code dump, but I want you to see why I am already unhappy. Here’s all of LookPacket:

LookPacket = class()

function LookPacket:fromResponse(response)
    local objects = response:dataObjects()
    local result = {}
    for i,o in ipairs(objects) do
        table.insert(result, LookPacket:fromObject(o))
    end
    return result
end

function LookPacket:fromObject(o)
    return LookPacket(o.direction, o.distance, o.type)
end


function LookPacket:init(direction, distance, type)
    self._direction = direction
    self._distance = distance
    self._type = type
end

function LookPacket:__tostring()
    return "LookPacket("..self._direction..", "..self._distance..", "..self._type..")"
end

function LookPacket:asFactDetails()
    local convert = { N={0,1}, S={0,-1}, E={1,0}, W={-1,0} }
    local mul = convert[self._direction]
    if not mul then return nil,nil end
    return self._type, self._distance*mul[1], self._distance*mul[2]
end

function LookPacket:asObject()
    return {
        direction=self._direction,
        distance=self._distance,
        type=self._type
    }
end

function LookPacket:direction()
    return self._direction
end

function LookPacket:distance()
    return self._distance
end

function LookPacket:type()
    return self._type
end

Basically every line of this thing is built around distance and direction. We need it to be built around x and y, or at least to allow for x and y.

Let’s start with the creation and forward the x and y to a new creation method:

function World:createPacketFor(lens,x,y)
    local content = lens:factAt(x,y)
    if not content then return nil end
    local dir = self:directionTo(x,y)
    local distance = math.max(math.abs(x), math.abs(y))
    return LookPacket(dir, distance, content)

We’ll posit that LookPacket has a new method fromXY and implement it. We can begin with an extract:

function World:createPacketFor(lens,x,y)
    local content = lens:factAt(x,y)
    if not content then return nil end
    return self:packetFromXY(content,x,y)
end

function World:packetFromXY(content,x,y)
    local dir = self:directionTo(x,y)
    local distance = math.max(math.abs(x), math.abs(y))
    return LookPacket(dir, distance, content)
end

function World:directionTo(x,y)
    if x > 0 then return "E" end
    if x < 0 then return "W" end
    if y > 0 then return "N" end
    return "S"
end

This works. No surprise there.

I’m changing strategy. I’m going to do this for real. The code above works. Commit: extract packetFromXY in World.

Move directionTo to LookPacket class:

function LookPacket:directionTo(x,y)
    if x > 0 then return "E" end
    if x < 0 then return "W" end
    if y > 0 then return "N" end
    return "S"
end

And call it from our new method:

function World:packetFromXY(content,x,y)
    local dir = LookPacket:directionTo(x,y)
    local distance = math.max(math.abs(x), math.abs(y))
    return LookPacket(dir, distance, content)
end

This should be green. It is. Commit: refer to LookPacket for direction in packetFrom.

Now move packetFrom to LookPacket and call it from our create:

function LookPacket:packetFromXY(content,x,y)
    local dir = LookPacket:directionTo(x,y)
    local distance = math.max(math.abs(x), math.abs(y))
    return LookPacket(dir, distance, content)
end

function World:createPacketFor(lens,x,y)
    local content = lens:factAt(x,y)
    if not content then return nil end
    return LookPacket:packetFromXY(content,x,y)
end

Should be green. Yes. Rename method to fromXY since it’s now obvious what it is. Still green. Commit: World:createPacket provides x and y to LookPacket creation.

Let’s reflect. This is important.

Reflection

This is actually quite wonderful!

In three simple steps …

  1. extract method;
  2. move method to another class;
  3. move method to another class;

… we have isolated the notion of direction distance completely inside LookPacket. The creator now thinks in terms of relative x and y.

Now you and I know that we have a very nefarious purpose for this change: we’re going to change the whole idea of how the radar works. But all the weird bits are now isolated inside LookPacket.

This is a very nifty result. Sit with it a moment. We’ve completely surrounded the x,y to dir-distance issue.

Now we can save x and y safely. Let’s examine the LookPacket creation methods:

function LookPacket:fromResponse(response)
    local objects = response:dataObjects()
    local result = {}
    for i,o in ipairs(objects) do
        table.insert(result, LookPacket:fromObject(o))
    end
    return result
end

function LookPacket:fromObject(o)
    return LookPacket(o.direction, o.distance, o.type)
end

function LookPacket:fromXY(content,x,y)
    local dir = LookPacket:directionTo(x,y)
    local distance = math.max(math.abs(x), math.abs(y))
    return LookPacket(dir, distance, content)
end

function LookPacket:init(direction, distance, type)
    self._direction = direction
    self._distance = distance
    self._type = type
end

We see that the first two methods there are decoding what comes back from the response. (We have an advantage here, which is that we’re using the same LookPacket object in World and in Robot. If we were not, we’d have to change two objects in sync.) We know that our objects are created with x and y in World, so let’s add in those parameters when we create:

function LookPacket:fromXY(content,x,y)
    local dir = LookPacket:directionTo(x,y)
    local distance = math.max(math.abs(x), math.abs(y))
    return LookPacket(dir, distance, content, x, y)
end

function LookPacket:init(direction, distance, type, x, y)
    self._direction = direction
    self._distance = distance
    self._type = type
    self._x = x
    self._y = y
end

That should be harmless: no one is looking at those. Green. Commit: LookPacket:fromXY saves _x and _y.

Let’s see how we create the object that is sent across the wire:

function LookPacket:asObject()
    return {
        direction=self._direction,
        distance=self._distance,
        type=self._type
    }
end

Let’s include x and y in the packet. No one will notice … yet.

function LookPacket:asObject()
    return {
        direction=self._direction,
        distance=self._distance,
        type=self._type,
        x=self._x,
        y=self._y
    }
end

Test. Green. Commit: x and y passed as json from world in LookPacket.

Look at all these tiny safe steps! This is so fine!

Now in the response we will be seeing an object with x and y as well as direction and distance. And we have a test for that:

        _:test("Packets from Response",  function()
            local response= Response({
                data={
                    objects={
                        {direction="N", type="O", distance=5},
                        {direction="S", type="P", distance=3}
                    }
                }
            })
            local packets = LookPacket:fromResponse(response)
            _:expect(packets[1]:direction()).is("N")
            _:expect(packets[2]:direction()).is("S")
            _:expect(packets[1]:type()).is("O")
            _:expect(packets[2]:type()).is("P")
            _:expect(packets[1]:distance()).is(5)
            _:expect(packets[2]:distance()).is(3)
        end)

We can extend that test to have expectations for x and y:

            local packets = LookPacket:fromResponse(response)
            _:expect(packets[1]:direction()).is("N")
            _:expect(packets[2]:direction()).is("S")
            _:expect(packets[1]:type()).is("O")
            _:expect(packets[2]:type()).is("P")
            _:expect(packets[1]:distance()).is(5)
            _:expect(packets[2]:distance()).is(3)
            _:expect(packets[1]:x()).is(0)
            _:expect(packets[1]:y()).is(5)
            _:expect(packets[2]:x()).is(0)
            _:expect(packets[2]:y()).is(3)
        end)

Test should fail with nils.

2: Packets from Response -- TestLookPacket:38: attempt to call a nil value (method 'x')

function LookPacket:x()
    return self._x
end

function LookPacket:y()
    return self._y
end

Still fail with nils, lots of them this time.

2: Packets from Response  -- 
Actual: nil, 
Expected: 0
2: Packets from Response  -- 
Actual: nil, 
Expected: 5
2: Packets from Response  -- 
Actual: nil, 
Expected: 0
2: Packets from Response  -- 
Actual: nil, 
Expected: 3

And now in the constructor:

function LookPacket:fromObject(o)
    return LookPacket(o.direction, o.distance, o.type, o.x, o.y)
end

I expect green. I get it except for the 3 in the test, which should be -3. Careless of me. Green. Slow down. Commit: LookPackets from world include x and y, which are seen in Robot.

But now that we have x and y, we can call our fromXY method. This:

function LookPacket:fromObject(o)
    return LookPacket(o.direction, o.distance, o.type, o.x, o.y)
end

Becomes this:

function LookPacket:fromObject(o)
    return LookPacket:fromXY(o.type, o.x, o.y)
end

Should be green. Green. LookPacket uses fromXY for creation from json objects.

Now it seems to me that we have a method that converts distance and direction to x and y for the Robot:

function LookPacket:asFactDetails()
    local convert = { N={0,1}, S={0,-1}, E={1,0}, W={-1,0} }
    local mul = convert[self._direction]
    if not mul then return nil,nil end
    return self._type, self._distance*mul[1], self._distance*mul[2]
end

Can’t we just return x and y now, since we have them?

function LookPacket:asFactDetails()
    return self._type, self._x, self._y
end

We can, but a test fails.

6: Robot understands new 'look'  -- 
Actual: nil, 
Expected: OBSTACLE

Let’s have a look at that, as it were:

        _:test("Robot understands new 'look'", function()
            local world = WorldProxy()
            local robot = Robot("Louie", world)
            local look = LookPacket("N", 3, "OBSTACLE")
            robot:addLook(look)
            _:expect(robot:factAt(0,3)).is("OBSTACLE")
        end)

Right. No one should be using the vanilla init on LookPacket unless they’re going to provide x and y. What’s a good fix for that? I’m planning to get rid of the NSEW stuff entirely. I’ll ignore these tests for now.

Test. Green (well, yellow). Commit: LookPacket now returns x and y directly, ignoring direction and distance.

Now I want to do a change to the visibility coloring in the robot’s screen.

Show the Scan

I want to show the area scanned in that lighter color, and now I want to show the whole radius. Recall we did that like this:

function Robot:look()
    self:markCellsLookedAt(self.worldinfo.visibility)
    local callback = function(response) self:lookCallback(response) end
    self._world:look(self._name,callback)
end

We mark the cells before even issuing the command. With this code:

function Robot:markCellsLookedAt(v)
    self:markHCellsLookedAt(v)
    self:markVCellsLookedAt(v)
end

function Robot:markHCellsLookedAt(v)
    for x = -v,v do
        self:addFactAt("LOOK",x,0)
    end
end

function Robot:markVCellsLookedAt(v)
    for y = -v,-1 do
        self:addFactAt("LOOK",0,y)
    end
    for y = 1,v do
        self:addFactAt("LOOK",0,y)
    end
end

We’ll just change the markCellsLookedAt:

function Robot:markCellsLookedAt(v)
    for x = -v,v do
        for y = -v,v do
            if vec2(x,y):len() <= v then
                self:addFactAt("LOOK",x,y)
            end
        end
    end
end

And when we run the program and do a look, we get this:

screen showing roundish radar profile

Just as intended. I have no test for that and I don’t want one. Don’t sue me, I never signed that contract. Commit: screen now shows full radar scan area in light color.

One more thing. Now I should be able to do the full scan in World as well.

function World:look(robotName)
    local robot = self._robots[robotName]
    local packets = {}
    if robot then
        local lens = self._knowledge:newLensAt(robot._x, robot._y)
        self:lookCircularIntoPackets(packets, lens)
    end
    return self:jsonLookResult(robot, packets)
end

And …

function World:lookCircularIntoPackets(packets,lens)
    local v = self.constants.visibility
    for x = -v,v do
        for y = -v,v do
            if vec2(x,y):len() <= v then
                self:lookFactInto(packets, lens, x,y)
            end
        end
    end
end

And when we now do a look:

radar shows all obstacles and pits

Perfect! I love this!

And I have a couple of broken tests.

1: Robot updates knowledge on move exists not yet visible -- 
Actual: not visible on first look, 
Expected: nil
1: Robot updates knowledge on move  -- 
Actual: not visible on first look, 
Expected: nil

Those are both wrong now, because we have the upgraded radar.

I could defer making this last change, but it clearly works. I’m going to call over a couple of other team members and the product owner and see what we think.

The product owner loves the feature. The other programmers agree that the code works, and none of us sees an easy test for it. We decide to mark the tests ignore and treat removing the ignores as our next story, but to leave the code. It’s a risk, and a bit of a violation, but we’re sure it’s righteous.

Commit: radar now scans a radius of visibiity around the robot.

Let’s sum up.

Summary

There’s good news and not so good.

The good news is that we changed the program from doing an x scan and y scan to doing a full radial scan in very small steps, each one just a line or two, with tests all green, from beginning to end.

We did start with an experiment that showed why we couldn’t start at the left end and go to the right end … the people in the middle were expecting distance and direction.

So in the real implementation we slowly changed the expectation of the LookPacket, then it creators, then its users, to be focused on x and y. We were under excellent control, guided by the tests, right till near the end.

Then, however, we resorted to looking at the screen to see about painting the looked-at cells. I’d have preferred a test, but while the code is simple (length <= visibility), the test would be hard to explicitly verify and just rechecking <= visibility seems silly. But I feel badly for not having the test.

Then I compounded the feeling by not checking the scan in World. And it could be wrong. Suppose we had forgotten the < visibility. Then we’d scan the entire square, not the roundish bit.

So these bits of code really do need tests.

Finally, because my various personas were easily convinced to commit, I’ve committed this code, untested, with a few tests marked “ignore” and with some old unused methods left in the code, because I didn’t tidy up.

A better, wiser person wouldn’t have left the cruft, and wouldn’t commit without improving the tests.

I am just the this-good-this-wise person that I am, and sometimes I don’t measure up to my own standards, much less yours. Still, while the situation isn’t perfect. I feel that it went very nicely and that I can clean things up quickly.

In fact, to prove it … I’ll do it now. See how much better a person I am than that guy in the previous paragraph?

Coda

Remove Robot methods markHCellsLooked at and the V version.

Commit: remove unused methods.

In World, remove lookNSEWinto and lookInDirection.

Commit: remove unused methods.

OK, some ignored tests. There are these two:

        _:ignore("Robot understands new 'look'", function()
            local world = WorldProxy()
            local robot = Robot("Louie", world)
            local look = LookPacket("N", 3, "OBSTACLE")
            robot:addLook(look)
            _:expect(robot:factAt(0,3)).is("OBSTACLE")
        end)
        
        _:ignore("Robot ignores bad packet direction", function()
            local world = WorldProxy()
            local robot = Robot("Louie", world)
            robot:forward(1)
            local look = LookPacket("Q", 3, "OBSTACLE")
            robot:addLook(look)
            _:expect(robot.knowledge:factCount()).is(0)
        end)

These both test aspects of the LookPacket that no longer need testing, especially the second one. We can delete that one, and let’s extend the other.

        _:test("Robot understands new 'look'", function()
            local world = WorldProxy()
            local robot = Robot("Louie", world)
            local look = LookPacket("N", 3, "OBSTACLE", 0,3)
            robot:addLook(look)
            _:expect(robot:factAt(0,3)).is("OBSTACLE")
        end)

Green. Commit: remove unneeded test. improve new look test.

There’s one more:

        _:ignore("Robot updates knowledge on move", function()
            local world = WorldProxy:test1()
            local robot = Robot("Louie",world)
            robot:look()
            --robot.knowledge:dump()
            _:expect(robot:factAt(3,0)).is("fact")
            _:expect(robot:factAt(3,1),"exists not yet visible").is(nil)
            robot:forward(1)
            _:expect(robot:factAt(3,-1)).is("fact")
            _:expect(robot:factAt(3,0)).is(nil)
            robot:look()
            _:expect(robot:factAt(3,0), "after second look").is("not visible on first look")
        end)

This test is wrong because the second fact is in fact in range of a round scan. We can fix it with better coordinates. What’s outside of range at x = 3 … (3,8). Let’s try that.

        _:test("Robot updates knowledge on move", function()
            local world = WorldProxy:test1()
            local robot = Robot("Louie",world)
            robot:look()
            --robot.knowledge:dump()
            _:expect(robot:factAt(3,0)).is("fact")
            _:expect(robot:factAt(3,8),"exists not yet visible").is(nil)
            robot:forward(1)
            _:expect(robot:factAt(3,-1)).is("fact")
            _:expect(robot:factAt(3,7)).is(nil)
            robot:look()
            _:expect(robot:factAt(3,7), "after second look").is("not visible on first look")
        end)

I had to change the test1 method in world:

function World:test1()
    local world = World()
    world:addFactAt("fact",3,0)
    world:addFactAt("not visible on first look", 3,8)
    return world
end

We are green. Commit: updated tests for look distance.

One more thing … leftover stuff in LookPacket. All the direction and distance code needs to go … and there are probably tests to fail if we do that. Let’s find out.

Some tests break, including:

14: World can create LookPacket from look -- TestWorld:420: attempt to call a nil value (method 'direction')
6: Robot understands new 'look' -- TestKnowledge:187: attempt to add a 'number' with a 'string'
1: LookPacket -- TestLookPacket:17: attempt to call a nil value (method 'direction')
2: Packets from Response -- TestLookPacket:32: attempt to call a nil value (method 'direction')

Fix them:

        _:test("World can create LookPacket from look", function()
            local world = World(25,25)
            world:addPit(0,3,0,3) -- N 3
            world:addPit(0,-4,0,-4) -- S 4
            world:addObstacle(2,0,2,0) -- E 2
            world:addObstacle(-5,0,-5,0) -- W 5
            local lens = world._knowledge:newLensAt(0,0)
            local packet = world:createPacketFor(lens, 0,3)
            _:expect(packet:direction()).is("N")
            _:expect(packet:distance()).is(3)
            _:expect(packet:type()).is("PIT")
            local packet = world:createPacketFor(lens, 0,-4)
            _:expect(packet:direction()).is("S")
            _:expect(packet:distance()).is(4)
            _:expect(packet:type()).is("PIT")
            local packet = world:createPacketFor(lens, 2,0)
            _:expect(packet:direction()).is("E")
            _:expect(packet:distance()).is(2)
            _:expect(packet:type()).is("OBSTACLE")
            local packet = world:createPacketFor(lens, -5,0)
            _:expect(packet:direction()).is("W")
            _:expect(packet:distance()).is(5)
            _:expect(packet:type()).is("OBSTACLE")
        end)

This one needs lots of trivial changes to check the various x and y … except that now fewer tests will suffice: there’s no behavior in the constructor any more.

        _:test("World can create LookPacket from look", function()
            local world = World(25,25)
            world:addPit(0,3,0,3)
            world:addObstacle(2,0,2,0)
            local lens = world._knowledge:newLensAt(0,0)
            local packet = world:createPacketFor(lens, 0,3)
            _:expect(packet:x()).is(0)
            _:expect(packet:y()).is(3)
            _:expect(packet:type()).is("PIT")
            local packet = world:createPacketFor(lens, 2,0)
            _:expect(packet:x()).is(2)
            _:expect(packet:y()).is(0)
            _:expect(packet:type()).is("OBSTACLE")
        end)

That’s green. What’s next?

6: Robot understands new 'look' -- TestKnowledge:187: attempt to add a 'number' with a 'string'

That’s interesting … Probably a bad constructor:

        _:test("Robot understands new 'look'", function()
            local world = WorldProxy()
            local robot = Robot("Louie", world)
            local look = LookPacket("N", 3, "OBSTACLE", 0,3)
            robot:addLook(look)
            _:expect(robot:factAt(0,3)).is("OBSTACLE")
        end)

Right:

        _:test("Robot understands new 'look'", function()
            local world = WorldProxy()
            local robot = Robot("Louie", world)
            local look = LookPacket("OBSTACLE", 0,3)
            robot:addLook(look)
            _:expect(robot:factAt(0,3)).is("OBSTACLE")
        end)

Good. Now:

1: LookPacket -- TestLookPacket:17: attempt to call a nil value (method 'direction')

        _:test("LookPacket", function()
            local packet = LookPacket("N", 5, "O")
            _:expect(packet:direction()).is("N")
            _:expect(packet:distance()).is(5)
            _:expect(packet:type()).is("O")
        end)

No longer needed, we create lots of them. That leaves this one:

1: Packets from Response -- TestLookPacket:25: attempt to call a nil value (method 'direction')

This is a big test of little consequence, which is why I allowed us to ignore it.

        _:test("Packets from Response",  function()
            local response= Response({
                data={
                    objects={
                        {direction="N", type="O", distance=5, x=0,y=5},
                        {direction="S", type="P", distance=3, x=0,y=-3}
                    }
                }
            })
            local packets = LookPacket:fromResponse(response)
            _:expect(packets[1]:direction()).is("N")
            _:expect(packets[2]:direction()).is("S")
            _:expect(packets[1]:type()).is("O")
            _:expect(packets[2]:type()).is("P")
            _:expect(packets[1]:distance()).is(5)
            _:expect(packets[2]:distance()).is(3)
            _:expect(packets[1]:x()).is(0)
            _:expect(packets[1]:y()).is(5)
            _:expect(packets[2]:x()).is(0)
            _:expect(packets[2]:y()).is(-3)
        end)

Remove the references to direction and distance, no longer used.

        _:test("Packets from Response",  function()
            local response= Response({
                data={
                    objects={
                        {direction="N", type="O", distance=5, x=0,y=5},
                        {direction="S", type="P", distance=3, x=0,y=-3}
                    }
                }
            })
            local packets = LookPacket:fromResponse(response)
            _:expect(packets[1]:type()).is("O")
            _:expect(packets[2]:type()).is("P")
            _:expect(packets[1]:x()).is(0)
            _:expect(packets[1]:y()).is(5)
            _:expect(packets[2]:x()).is(0)
            _:expect(packets[2]:y()).is(-3)
        end)

Green. Commit: Cleaned up tests to match new LookPacket structure and behavior.

OK, happy now? No new ignored tests, and the code has been tidied. That took me about another hour, which is a lot when you’re trying to only work three hours a day. But it was all easy enough.

We have the first of our new stories done. I still expect auto-look to be easy, and I still think shadowing will be difficult. We’ll find out … next time. Join me then!