Let’s not allow movement into obstacles, and maybe penalize moving into pits.

The spec says you can’t move onto an Obstacle, and that if you fall into a Pit you die. I have in mind making falling into a Pit less awful than that, but we’ll see. My plan for this, my third foray of the day, is to deal with not moving through obstacles.

When you try to move onto an Obstacle, your response.data.message should be “Obstructed” and your coordinates shouldn’t change. Presently I only support single step motion, but the code would just teleport you if you entered a larger distance. To do it right, if the Robot requests a distance greater than 1, we should check all the intervening squares. We’ll look at that.

If we were wise, and now that I’ve mentioned it we’d better be, we’d do this with some tests.

Let’s see what we have now. We have a couple of moving-related tests, including this one:

        _:test("forward adjusts position", function()
            local world = World(25,25)
            local rq = {
                robot="xyzzy",
                command="launch",
                arguments={"plugh", 99,99}
            }
            world:request(rq)
            local robotState = world:getRobot("xyzzy")
            _:expect(robotState._x).is(0)
            _:expect(robotState._y).is(0)
            world:forward("xyzzy", 1)
            _:expect(robotState._x).is(0)
            _:expect(robotState._y).is(1)
        end)

Let’s have a new, simple test:

        _:test("can't move onto obstacle", function()
            local world = World(25,25)
            world:addObstacle(0,1,0,1)
            local rq = {
                robot="xyzzy",
                command="launch",
                arguments={"plugh", 99,99}
            }
            world:request(rq)
            rq = {
                robot="xyzzy",
                command="forward",
                arguments = {1}
            }
            local response = world:request(rq)
            _:expect(response.result).is("OK")
            _:expect(response.data.message).is("Obstacle")
            _:expect(response.state.position[2]).is(0)
        end)

We have to move using request so that we get a response dictionary back. Test fails just as I’d like:

24: can't move onto obstacle  -- 
Actual: nil, 
Expected: Obstacle
24: can't move onto obstacle  -- 
Actual: 1, 
Expected: 0

No message. I wonder if it’s supposed to default to blank. I bet it is. Anyway, as expected.

Now we really ought to think about how to do this but let’s at least see what happens now.

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction)
    robot._x = robot._x + xStep*whichWay*distance
    robot._y = robot._y + yStep*whichWay*distance
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot)
    }
    return response
end

Well, we just do it. What should really happen? we should check each position from 1 to distance. That means we should loop, only returning a response when we’re done or hit something.

We have some multi-step tests I think. Let’s see. Ah, there’s one:

        _:test("turns take you to the right places", function()
            local world = World(25,25)
            local rq = {
                robot="xyzzy",
                command="launch",
                arguments={"plugh", 99,99}
            }
            world:request(rq)
            local robot = world:getRobot("xyzzy")
            world:forward("xyzzy", 2)
            _:expect(robot._x).is(0)
            _:expect(robot._y).is(2)
            world:turn("xyzzy","right") -- east
            world:forward("xyzzy", 3)
            _:expect(robot._x).is(3)
            _:expect(robot._y).is(2)
            world:turn("xyzzy", "right") -- south
            world:forward("xyzzy",5)
            _:expect(robot._x).is(3)
            _:expect(robot._y).is(-3)
            world:turn("xyzzy","right") -- west
            world:forward("xyzzy",3)
            _:expect(robot._x).is(0)
            _:expect(robot._y).is(-3)
        end)

This one moves varying distances after turning. Should be enough to test changing to a loop for motion. Oh, and forward just calls moveFB which does return the response so I can simplify my test a bit if I care to. Let’s do the looping first.

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction)
    for i = 1,distance do
        robot._x = robot._x + xStep*whichWay
        robot._y = robot._y + yStep*whichWay
    end
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot)
    }
    return response
end

I expect everything to work except my new test, which should fail as before. That’s what happens. We’re good. Let’s change the loop:

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction)
    local count = 1
    local message = ""
    while message == "" and count <= distance do
        count = count + 1
        robot._x = robot._x + xStep*whichWay
        robot._y = robot._y + yStep*whichWay
    end
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot)
    }
    return response
end

Here I’m just using the message as a flag. Now to put the message in the result:

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction)
    local count = 1
    local message = ""
    while message == "" and count <= distance do
        count = count + 1
        robot._x = robot._x + xStep*whichWay
        robot._y = robot._y + yStep*whichWay
    end
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

Hm, this is getting nasty, but I don’t seem to be in trouble. I’ll press on.

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction)
    local count = 1
    local message = ""
    while message == "" and count <= distance do
        local new_x = robot._x + xStep*whichWay
        local new_y = robot._y + yStep*whichWay
        count = count + 1
        robot._x = new_x
        robot._y = new_y
    end
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

I’ve extracted the calculations for the new x and y … because we won’t necessarily move there.

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction)
    local count = 1
    local message = ""
    while message == "" and count <= distance do
        local new_x = robot._x + xStep*whichWay
        local new_y = robot._y + yStep*whichWay
        count = count + 1
        robot._x = new_x
        robot._y = new_y
    end
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

I expect the tests to pass, although I hate this code. Well, not so much hate as dislike. Well, quite dissatisfied. Anyway, we are green. Commit: Robot cannot move onto or over obstacle.

So that’s nice. Let’s see about making the code better.

I wonder why we didn’t fold whichWay right in? Let’s do that.

It may not be clear what’s going on there, let me explain. The whichWay variable is +1 for forward, -1 for backward. The getSteps function returns the x and y steps to take, depending on direction. If we’re going north, we move (0.1) forward. If going south, we move (0,-1) forward, and so on.

We can fold whichWay into the original setup of the steps and then not do it all the time in the loop.

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction, whichWay)
    local count = 1
    local message = ""
    while message == "" and count <= distance do
        local new_x = robot._x + xStep
        local new_y = robot._y + yStep
        if self:factAt(new_x, new_y) == nil then
            count = count + 1
            robot._x = new_x
            robot._y = new_y
        else
            message="Obstacle"
        end
    end
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

We must change getSteps to include whichWay:

function World:getSteps(direction)
    local steps = {
        N={0,1}, E={1,0}, S={0,-1},W={-1,0}
    }
    return table.unpack(steps[direction])
end

That can become …

function World:getSteps(direction, whichWay)
    local steps = {
        N={0,1}, E={1,0}, S={0,-1},W={-1,0}
    }
    local dx,dy = table.unpack(steps[direction])
    return dx*whichWay, dy*whichWay
end

Let’s make that better.

function World:getSteps(direction, whichWay)
    local xStep = { N=0, E=1,S=0,W=-1 }
    local yStep = { N=1, E=0, S=-1, W=0 }
    return xStep[direction]*whichWay, yStep[direction]*whichWay
end

Nice. I should be committing these changes. Green. Commit: refactoring in getStep and moveFB.

Now for the big method:

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction, whichWay)
    local count = 1
    local message = ""
    while message == "" and count <= distance do
        local new_x = robot._x + xStep
        local new_y = robot._y + yStep
        if self:factAt(new_x, new_y) == nil then
            count = count + 1
            robot._x = new_x
            robot._y = new_y
        else
            message="Obstacle"
        end
    end
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

Let’s have a new method that returns your final location x and y, and the message to return. it will need your current position (or the robot?) and the dx dy to apply.

I am tempted to TDD this method but I think the test is harder to write than the code. The method call would look like this:

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction, whichWay)
    local count = 1
    local new_x,new_y,message = self:moveSteps(robot._x, robot._y, xStep, yStep)
    robot._x = new_x
    robot._y = new_y
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

That’s not great. Let’s pass in the robot and move it internally.

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction, whichWay)
    local count = 1
    self:moveSteps(robot, xStep, yStep)
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

Meh. How about this:

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    message = self:moveSteps(robot, whichWay)
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

OK, let’s work toward that … starting from the original nasty method:

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction, whichWay)
    local count = 1
    local message = ""
    while message == "" and count <= distance do
        local new_x = robot._x + xStep
        local new_y = robot._y + yStep
        if self:factAt(new_x, new_y) == nil then
            count = count + 1
            robot._x = new_x
            robot._y = new_y
        else
            message="Obstacle"
        end
    end
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

Let’s extract the loop part, this bit:

    local count = 1
    local message = ""
    while message == "" and count <= distance do
        local new_x = robot._x + xStep
        local new_y = robot._y + yStep
        if self:factAt(new_x, new_y) == nil then
            count = count + 1
            robot._x = new_x
            robot._y = new_y
        else
            message="Obstacle"
        end
    end

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction, whichWay)
    local message = self:extracted(robot, xStep, yStep, distance)
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

function World:extracted(robot, xStep, yStep, distance)
    local count = 1
    local message = ""
    while message == "" and count <= distance do
        local new_x = robot._x + xStep
        local new_y = robot._y + yStep
        if self:factAt(new_x, new_y) == nil then
            count = count + 1
            robot._x = new_x
            robot._y = new_y
        else
            message="Obstacle"
        end
    end
    return message
end

I just named the method extracted because I don’t know what its name should be. Tests are green. Commit: refactoring moveFB.

I find the extracted code a bit hard to understand, because it is doing that odd thing of proposing a robot position and then applying it all through the loop. Let’s instead do this:

function World:extracted(robot, xStep, yStep, distance)
    local count = 1
    local message = ""
    local new_x, new_y = robot._x, robot._y
    while message == "" and count <= distance do
        new_x = new_x + xStep
        new_y = new_y + yStep
        if self:factAt(new_x, new_y) == nil then
            count = count + 1
        else
            message="Obstacle"
        end
        if message ~= "Obstacle" then
            robot._x = new_x
            robot._y = new_y
        end
    end
    return message
end

Still green, but not satisfied. Let’s work some more. Best commit again: refactoring.

Perhaps some thinking would be good. If the obstacle is an Obstacle, we cannot step on it. We won’t be able to step on a Tank. We can step into a Pit but then we must stop.

I’m sure I can code this, but I don’t see a quick way to test it. I guess I’ll just rely on my existing tests, which seem solid.

The reason I’m somewhat stuck is that I’m thinking of too many things at once, the loop counter, the dx and dy, the x and y, the message, the facts, and a little bit of thinking “I have to stop before an Obstacle but in a Pit”.

Wait. All we really have to do now is return a message (with the Robot moved to the point it should be moved to. Let’s try that:

function World:extracted(robot, xStep, yStep, distance)
    local count = 1
    local message = ""
    local new_x, new_y = robot._x, robot._y
    while message == "" and count <= distance do
        new_x = new_x + xStep
        new_y = new_y + yStep
        if self:factAt(new_x, new_y) == "OBSTACLE" then
            return "Obstacle"
        end
        robot._x = new_x
        robot._y = new_y
        count = count + 1
    end
    return message
end

Now we are early outing. And the loop can be simplified:

function World:extracted(robot, xStep, yStep, distance)
    local new_x, new_y = robot._x, robot._y
    for i = 1,distance do
        new_x = new_x + xStep
        new_y = new_y + yStep
        if self:factAt(new_x, new_y) == "OBSTACLE" then
            return "Obstacle"
        end
        robot._x = new_x
        robot._y = new_y
    end
    return ""
end

Tests are green. Commit: refactoring.

I think that’ll do for now. Let’s add a test.

        _:test("can't move past pit", function()
            local world = World(25,25)
            world:addPit(0,2,0,2)
            local rq = {
                robot="xyzzy",
                command="launch",
                arguments={"plugh", 99,99}
            }
            world:request(rq)
            rq = {
                robot="xyzzy",
                command="forward",
                arguments = {3}
            }
            local response = world:request(rq)
            _:expect(response.result).is("OK")
            _:expect(response.data.message).is("Pit")
            _:expect(response.state.position[2]).is(2)
        end)

We can move into a pit, but we can’t get past it. This test will fail both its expectations.

25: can't move past pit  -- 
Actual: , 
Expected: Pit
25: can't move past pit  -- 
Actual: 3, 
Expected: 2

And to fix:

function World:extracted(robot, xStep, yStep, distance)
    local new_x, new_y = robot._x, robot._y
    for i = 1,distance do
        if self:factAt(new_x, new_y) == "PIT" then
            return "Pit"
        end
        new_x = new_x + xStep
        new_y = new_y + yStep
        if self:factAt(new_x, new_y) == "OBSTACLE" then
            return "Obstacle"
        end
        robot._x = new_x
        robot._y = new_y
    end
    return ""
end

Very fine. If you’re already in a pit, you can’t move out of it. We should extend the test:

        _:test("can't move past pit", function()
            local world = World(25,25)
            world:addPit(0,2,0,2)
            local rq = {
                robot="xyzzy",
                command="launch",
                arguments={"plugh", 99,99}
            }
            world:request(rq)
            rq = {
                robot="xyzzy",
                command="forward",
                arguments = {3}
            }
            local response = world:request(rq)
            _:expect(response.result).is("OK")
            _:expect(response.data.message).is("Pit")
            _:expect(response.state.position[2]).is(2)
            rq = {
                robot="xyzzy",
                command="forward",
                arguments = {3}
            }
            local response = world:request(rq)
            _:expect(response.result).is("OK")
            _:expect(response.data.message).is("Pit")
            _:expect(response.state.position[2]).is(2)
        end)

We just try the move again and nothing happens but we get the “Pit” message again. Commit: World supports not moving past a pit.

Works in the game, too, but the display when you are in a pit is slightly disappointing:

in pit does not show black around tank

The display doesn’t show the black square around the tank. I wish it would. Let’s see if that’s too difficult. At a quick glance, yes it’s too difficult. The display of a given cell is this:

function drawCell(x,y, size)
    local fact = CurrentRobot:factAt(x,y)
    if x == 0 and y == 0 then
        dir = CurrentRobot:direction()
        fact = "R"..dir
    end
    local gd = GridDisplay:display(fact)
    if gd.color then
        fill(gd.color)
    else
        noFill()
    end
    rect(x*size,y*size,size,size)
    if gd.text then
        fill(0,256,0)
        text(gd.text, x*size,y*size)
    end
    if gd.asset then
        pushMatrix()
        scale(0.8, 0.8)
        sprite(gd.asset,x,y)
        popMatrix()
    end
end

If the display is at 0,0 (relative to the robot), we set up the Robot as the only fact, so we have lost the option of the other displays. But wait, what if we did this:

function drawCell(x,y, size)
    local fact = CurrentRobot:factAt(x,y)
    local gd = GridDisplay:display(fact)
    if gd.color then
        fill(gd.color)
    else
        noFill()
    end
    rect(x*size,y*size,size,size)
    if gd.text then
        fill(0,256,0)
        text(gd.text, x*size,y*size)
    end
    if x == 0 and y == 0 then
        dir = CurrentRobot:direction()
        fact = "R"..dir
        gd = GridDisplay:display(fact)
    end
    if gd.asset then
        pushMatrix()
        scale(0.8, 0.8)
        sprite(gd.asset,x,y)
        popMatrix()
    end
end

We wait until it’s time to display the asset. Our only asset (right now) is the robot, so we are forcing him to display. That’s not going to work with other assets. Let’s change it again:

function drawCell(x,y, size)
    local fact = CurrentRobot:factAt(x,y)
    local gd = GridDisplay:display(fact)
    if gd.color then
        fill(gd.color)
    else
        noFill()
    end
    rect(x*size,y*size,size,size)
    if gd.text then
        fill(0,256,0)
        text(gd.text, x*size,y*size)
    end
    if gd.asset then
        pushMatrix()
        scale(0.8, 0.8)
        sprite(gd.asset,x,y)
        popMatrix()
    end
    if x == 0 and y == 0 then
        dir = CurrentRobot:direction()
        fact = "R"..dir
        gd = GridDisplay:display(fact)
        pushMatrix()
        scale(0.8, 0.8)
        sprite(gd.asset,x,y)
        popMatrix()
    end
end

A bit nasty, but good for now. Commit: robot now shows in a pit when in a pit. It looks like this:

robot in black square for pit

And that’ll do for my third article of the day. Let’s sum up.

Summary

The code is a bit better and we now support not moving onto or past an OBSTACLE and moving into but not out of a PIT.

I just kind of programmed my way through that method named extracted, which really needs a better name. Let’s do that right now before we forget. I called it moveToLimit:

function World:moveFB(robotName,whichWay, distance)
    local robot = self._robots[robotName]
    local xStep,yStep = self:getSteps(robot._direction, whichWay)
    local message = self:moveToLimit(robot, xStep, yStep, distance)
    local response = {
        result = "OK",
        data = {},
        state = self:robotState(robot),
        data = { message=message }
    }
    return response
end

function World:moveToLimit(robot, xStep, yStep, distance)
    local new_x, new_y = robot._x, robot._y
    for i = 1,distance do
        if self:factAt(new_x, new_y) == "PIT" then
            return "Pit"
        end
        new_x = new_x + xStep
        new_y = new_y + yStep
        if self:factAt(new_x, new_y) == "OBSTACLE" then
            return "Obstacle"
        end
        robot._x = new_x
        robot._y = new_y
    end
    return ""
end

Test. Green. Commit: rename extracted to moveToLimit.

I think naming it extracted was a decent idea. I didn’t want to get bogged down in meaning, because I was just extracting the method by rote, not yet thinking about what it meant. Of course, a wiser person might have been able to do both.

All that said, it took me a few tries to see how to improve that method but it went well and never broke anything except for some occasional typos which I spared you.

And the display looks good too. A fine finish to a fine day.

See you next time!