Robot 35

Let’s slide in the remaining callbacks, and see what else we can see before brekkers.

Yesterday we averted disaster by causing calls from Robot to World to use a callback to interpret the response from World. This ensures that we could make the request and response asynchronous, with a delay, if we needed to.

Naturally, this must become the way we always do it, and, quite likely, this will lead to some interesting evolution of the code’s design. We’ll see. Today, there are at least two more occasions to install the now-standard callback: scan/look, and launch.

I keep saying “scan/look” because the real command is “look”, but when I started this game, I thought it was “scan”, so the code says scan. Let’s start by fixing that.

There are 27 occurrences of the word “scan”, if I’ve counted them correctly, and it looks as if they all need changing. I’ll spare you the code. First thing, I’ll just see what happens if I global replace “scan” with “look”. It might just work.

Codea found 26. Entirely possible. Test. Tests are green, game works. Woot! Commit: rename all “scan” to “look”. I love it when a plan comes together.

Ah, found the 27th, it was capitalized and in a comment. Fixed that too.

Let’s do the launch next, because it is probably easier. “Look” does special processing.

function Robot:init(name,aWorld)
    assert(aWorld:is_a(WorldProxy), "expected WorldProxy")
    self._world = aWorld
    self:setResponse(aWorld:launchRobot(name, self))
    self._name = name
    self.knowledge = Knowledge()    
end

Interesting in that this one doesn’t do what the others did:

function Robot:standardResponse(response)
    self:setResponse(response)
    self.knowledge = self.knowledge:newLensAt(self:x(), self:y())
end

The reason, I imagine, is that this code predates the Lens, and/or it is assuming that we’ll be started at (0,0), or it’s just plain wrong.

Kent Beck says that when faced with a hard change, first make the change easy and then make the easy change. This change isn’t very hard, but we can make it easier.

We have these bits of code now:

function Robot:back(steps)
    local callback = function(...) self:standardResponse(...) end
    self._world:back(self._name, steps, callback)
end

function Robot:forward(steps)
    local callback = function(...) self:standardResponse(...) end
    self._world:forward(self._name, steps, callback)
end

function Robot:turn(lOrR)
    local callback = function(...) self:standardResponse(...) end
    self._world:turn(self._name,lOrR, callback)
end

All three of these use the same callback, and all three define it. Can we remove this duplication somehow? Yes, like this:

function Robot:standardCallback()
    return function(...) self:standardResponse(...) end
end

function Robot:turn(lOrR)
    self._world:turn(self._name,lOrR, self:standardCallback())
end

And now we can change the others …

function Robot:back(steps)
    self._world:back(self._name, steps, self:standardCallback())
end

function Robot:forward(steps)
    self._world:forward(self._name, steps, self:standardCallback())
end

And we should be green. We are. Commit: remove duplication of callbacks using standardCallback method.

Now we can do launch like this:

function WorldProxy:launchRobot(name,robot)
    local kind = "RonRobot1"
    local shields = 99
    local shots = 99
    local request = {
        robot=name,
        command="launch",
        arguments={kind,shields,shots}
    }
    local responseDict = self.world:request(request)
    return Response(responseDict)
end

And …

function WorldProxy:launchRobot(name,robot, callback)
    local kind = "RonRobot1"
    local shields = 99
    local shots = 99
    local request = {
        robot=name,
        command="launch",
        arguments={kind,shields,shots}
    }
    local responseDict = self.world:request(request)
    callback(Response(responseDict))
end

Test. Green. Commit: launch uses standardCallback.

We’re left with the unique one, look. It looks like this:

function Robot:look()
    local packets = self._world:look(self._name)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
end

function WorldProxy:look(...)
    local jsonString = self.world:look(...)
    local outcome = json.decode(jsonString)
    local packets = outcome.data.objects
    --print(#packets, " packets")
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        --print(lp)
        table.insert(result, lp)
    end
    return result
end

The WorldProxy is helping us out here, by pulling apart the response that came back and creating a table of LookPackets, that the Robot then tucks away using its addLook method.

We could argue that the WorldProxy is being too helpful here, but it’s a dirty job and someone has to do it. Our mission right now is to do all this via callback, and at first glance it’s not clear that moving this functionality will make the change easier … and yet …

We clearly do not want to use the standard callback for look, but it does seem that what we want in the callback is all this looking stuff.

Make the change easy—or at least easier—by refactoring out the packet making. When I look more closely, I see that we have another issue in the way. We’re calling the world directly, on its look method. We first need to package all this up in a request and go through the request interface.

There’s a fair amount of code involved in requests, and in World:look. Let’s have a new heading.

Robot-World Look

function World:request(rq)
    ok, result = pcall(function() return self:requestProtected(rq) end)
    if ok then
        return result
    else
        return self:error("SERVER ERROR "..tostring(result))
    end
end

function World:requestProtected(rq)
    local robotName = rq.robot
    local c = rq.command
    local args = rq.arguments
    local response
    if c == "noResponse" then
        -- nothing
    elseif c == "launch" then
        response = self:launchRobot(robotName, args[1], args[2], args[3])
    elseif c == "forward" then
        response = self:forward(robotName, args[1])
    elseif c == "back" then
        response = self:back(robotName, args[1])
    elseif c == "turn" then
        response = self:turn(robotName, args[1])
    else
        response = self:error("No such command "..tostring(c))
    end
    if self:isValidResponse(response) then
        return response
    else
        return self:error("SERVER RETURNED NO RESPONSE TO "..tostring(c))
    end
end

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

We are not using a legal request, nor are we as yet creating a legal response.

Let’s see what our response looks like now, no pun intended.

function World:jsonLookResult(packetArray)
    local outcome = {}
    outcome.result="OK"
    local objects = {}
    for i,p in ipairs(packetArray) do
        table.insert(objects,p:asObject())
    end
    outcome.data = { objects=objects }
    local jsonString = json.encode(outcome, {indent=true})
    return jsonString
end

Yeah, I don’t even know if this is a legal response packet, but I’m betting it isn’t. It’s surely lacking all kinds of things. Let’s see how they are built elsewhere:

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

OK, we can start there.

function World:jsonLookResult(packetArray)
    local outcome = {
        result="OK",
        data = {},
        state=self:robotState(robot)
    }

This much tells me that I need to pass in the robot …

function World:jsonLookResult(robot, packetArray)
    local outcome = {
        result="OK",
        data = {},
        state=self:robotState(robot)
    }
    local objects = {}
    for i,p in ipairs(packetArray) do
        table.insert(objects,p:asObject())
    end
    outcome.data = { objects=objects }
    local jsonString = json.encode(outcome, {indent=true})
    return jsonString
end

This should be harmless. Test and see. Good so far. Should I commit? In favor is that this is progress. Opposed is that I may screw up. I go with in favor, commit: Adding in legit outcome values in World:jsonLookResult.

I’d really like outcome to be named response to match the other code. Do that. Test and commit: rename local variable.

Now I think I have to review the documentation, which is a downer. I’m hoping that this response we’re building is in valid format. The document says that objects should be an array of direction, type, and distance. Our code in World is creating LookPacket instances, and they have direction, distance, and type:

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

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

Perfect. I believe that we are creating a legitimate Response object here, now that we’ve added in the extra stuff at the top.

So over in WorldProxy, we can be confident that we have a real response.

We have a couple of things we need to do. One is that we need to change the implementation of look to use the request/response form of code. The other is to change it to use a callback.

Let’s start with the request part and see what happens.

Use Request in Look

We change look thus:

function WorldProxy:look(name)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    local jsonString = self.world:request(request)
    local outcome = json.decode(jsonString)
    local packets = outcome.data.objects
    --print(#packets, " packets")
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        --print(lp)
        table.insert(result, lp)
    end
    return result
end

And expect that world will barf on look because it’s not in the dispatch. Yes, we get lots of serious errors:

2: Look on all four sides -- ...
EAF/Codea.app/Frameworks/RuntimeKit.framework/dkjson.lua:693: 
bad argument #2 to 'pegmatch' 
(string expected, got table)

We build look into the dispatch:

function World:requestProtected(rq)
    local robotName = rq.robot
    local c = rq.command
    local args = rq.arguments
    local response
    if c == "noResponse" then
        -- nothing
    elseif c == "launch" then
        response = self:launchRobot(robotName, args[1], args[2], args[3])
    elseif c == "look" then
        response = self:look(robotName)
    elseif c == "forward" then
        response = self:forward(robotName, args[1])
    elseif c == "back" then
        response = self:back(robotName, args[1])
    elseif c == "turn" then
        response = self:turn(robotName, args[1])
    else
        response = self:error("No such command "..tostring(c))
    end
    if self:isValidResponse(response) then
        return response
    else
        return self:error("SERVER RETURNED NO RESPONSE TO "..tostring(c))
    end
end

And test again. Still those same errors:

4: Robot does look/look on l key -- ...
EAF/Codea.app/Frameworks/RuntimeKit.framework/dkjson.lua:693: 
bad argument #2 to 'pegmatch' 
(string expected, got table)

Best look and see what that even is … a framework error. Oh my. Let’s revert and try again, I don’t even want to try to figure that out.

I can’t resist looking: Here’s a picture of all I changed:

working copy showing changes

I don’t know, revert, go in much smaller steps.

Revert, Smaller Steps

First thing that happens is that Codea comes up blank after the revert. I reboot the iPad. Test. Green. Change look to be more explicit about parameters. This is just lazy, done when I plugged in the proxy:

function WorldProxy:look(...)
    local jsonString = self.world:look(...)
    local outcome = json.decode(jsonString)
    local packets = outcome.data.objects
    --print(#packets, " packets")
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        --print(lp)
        table.insert(result, lp)
    end
    return result
end

Calling, we have:

function Robot:look()
    local packets = self._world:look(self._name)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
end

And so we can do this:

function WorldProxy:look(name)
    local jsonString = self.world:look(name)
    local outcome = json.decode(jsonString)
    local packets = outcome.data.objects
    --print(#packets, " packets")
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        --print(lp)
        table.insert(result, lp)
    end
    return result
end

Test. Green. Commit: explicit name parameter between Robot and WorldProxy look.

Now let me try the request route again.

function WorldProxy:look(name)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    local jsonString = self.world:request(request)
    local outcome = json.decode(jsonString)
    local packets = outcome.data.objects
    --print(#packets, " packets")
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        --print(lp)
        table.insert(result, lp)
    end
    return result
end

And this time, because superstition, I don’t run it until I get the command dealt with in World:

This time I notice that I’m returning a jsonString out of the look function. That could readily be the issue, since json is well inside Codea:

function World:requestProtected(rq)
    local robotName = rq.robot
    local c = rq.command
    local args = rq.arguments
    local response
    if c == "noResponse" then
        -- nothing
    elseif c == "launch" then
        response = self:launchRobot(robotName, args[1], args[2], args[3])
    elseif c == "look" then
        response = self:look(robotName)
    elseif c == "forward" then
        response = self:forward(robotName, args[1])
    elseif c == "back" then
        response = self:back(robotName, args[1])
    elseif c == "turn" then
        response = self:turn(robotName, args[1])
    else
        response = self:error("No such command "..tostring(c))
    end
    if self:isValidResponse(response) then
        return response
    else
        return self:error("SERVER RETURNED NO RESPONSE TO "..tostring(c))
    end
end

And in look we are still returning json:

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

And …

function World:jsonLookResult(robot, packetArray)
    local response = {
        result="OK",
        data = {},
        state=self:robotState(robot)
    }
    local objects = {}
    for i,p in ipairs(packetArray) do
        table.insert(objects,p:asObject())
    end
    response.data = { objects=objects }
    local jsonString = json.encode(response, {indent=true})
    return jsonString
end

We need to return the response, not the json thereof, so, for now:

function World:jsonLookResult(robot, packetArray)
    local response = {
        result="OK",
        data = {},
        state=self:robotState(robot)
    }
    local objects = {}
    for i,p in ipairs(packetArray) do
        table.insert(objects,p:asObject())
    end
    response.data = { objects=objects }
    return response
end

I think we can test. And again I get those nasty pegmatch messages. Revert again? No. I did a bit of printing. Then, with that telling me nothing, I typed l in the game and got an error I could understand, because the traceback pointed here:

function WorldProxy:look(name)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    local jsonString = self.world:request(request)
    local outcome = json.decode(jsonString)
    local packets = outcome.data.objects
    --print(#packets, " packets")
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        --print(lp)
        table.insert(result, lp)
    end
    return result
end

And there’s the bug! Since we’re getting a response back, not a string, we try to json decode a dictionary and apparently that’s not as good an idea as it might be. So the fix is:

function WorldProxy:look(name)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    local outcome = self.world:request(request)
    local packets = outcome.data.objects
    --print(#packets, " packets")
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        --print(lp)
        table.insert(result, lp)
    end
    return result
end

Test. Green. Commit: look now passes a request and gets a response back.

Back on track but I need a break and the cat is hungry,

Post Break

Well, we’re back on track. I wonder what I could have done that would have avoided that problem. My colleagues with type-checking languages would assure me that their language would have given me a compile error. That’s nice, but we are where we are, aren’t we? A better message from the json decode would have been nice.

Even after a bit of reflection, I don’t see a credible “should have”. I think the bear just bit me this time.

What about making that method smaller and what about tests for it?

function WorldProxy:look(name)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    local jsonString = self.world:request(request)
    local outcome = json.decode(jsonString)
    local packets = outcome.data.objects
    --print(#packets, " packets")
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        --print(lp)
        table.insert(result, lp)
    end
    return result
end

Let’s do refactor that: I think we’ll be glad we did anyway:

function WorldProxy:look(name)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    local outcome = self.world:request(request)
    return self:makePackets(outcome)
end

function WorldProxy:makePackets(outcome)
    local packets = outcome.data.objects
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        table.insert(result, lp)
    end
    return result
end

Somewhat better. Test. Green. Commit: refactor pulling out makePackets from WorldProxy:look.

Let’s remember what we’re up to. We’re trying to make the Robot-World “look” capability work like the others, including a callback. We will certainly want a special callback, because we expect WorldProxy to build our LookPackets for us and return them.

What we should do, I think, is have WorldProxy return us the response, as always, and deal with the details in Robot.

function Robot:look()
    local packets = self._world:look(self._name)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
end

Let’s change the rules here. We’ll have WorldProxy return the result, and we’ll call it back to get the packets made, at least for now, because that will be a smaller step:

function Robot:look()
    local response = self._world:look(self._name)
    local packets = self._world:makePackets(response)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
end

function WorldProxy:look(name)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    return self.world:request(request)
end

I expect this to work. It does. Green: commit: Robot uses envious function in WorldProxy to get LookPackets during look command.

Now we can just move that method from WorldProxy to Robot:

function Robot:look()
    local response = self._world:look(self._name)
    local packets = self:makePackets(response)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
end

function Robot:makePackets(outcome)
    local packets = outcome.data.objects
    local result = {}
    for i,p in ipairs(packets) do
        local lp = LookPacket:fromObject(p)
        table.insert(result, lp)
    end
    return result
end

Should be green. Is. Commit: Robot uses its own envious function to get look packets from response.

Feature envy: an object working on things it doesn’t even own.

Why envious? Because it has nothing to do with either Robot or WorldProxy. It looks like a class method on LookPacket to me. Let’s move it and let’s see if we need a test.

Yeah, we have like no tests for LookPacket (it is, after all, an object with no behavior):

        _: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)

Let’s add a new test:

        _:test("Packets from Response",  function()
            local response= {
                data={
                    objects={
                        {direction="N", type="O", steps=5},
                        {direction="S", type="P", steps=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]:steps()).is(5)
            _:expect(packets[2]:steps()).is(3)
        end)

That will drive out the fromResponse method:

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

And:

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

I just went wild there. I expect green. I don’t get it because there is no method steps. Its name is distance. Fix the test.

        _:test("Packets from Response",  function()
            local 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)

Green. Commit: LookPacket has fromResponse method to make array of LookPackets.

Now use it:

function Robot:look()
    local response = self._world:look(self._name)
    local packets = LookPacket:fromResponse(response)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
end

Test expecting green. Green after trying three times to say “fromResponse”. Commit: Robot uses LookPacket class method fromResponse to make its packets.

Pause to reflect and to consider whether to continue.

Reflection

This is all good, though I am wondering about whether LookPacket is carrying its weight. We just seem to convert simple dictionaries back and forth to it. I think I’ll default to keep it, because using primitive data types is always kind of messy, so it may pay off later even if right now it’s pretty light.

We’re just left with the need to do the callback. And I think we should do this first:

function Robot:look()
    local response = self._world:look(self._name)
    local packets = LookPacket:fromResponse(response)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
    self:standardResponse(response)
end

I’ve added a call to standardResponse because it’s the right thing to do. We might even have been hit by an arrow or something, and it’s in standardResponse where that would be dealt with. Test. Error, and it’s a useful one:

4: Robot does look/look on l key -- TestRobot:202: attempt to call a nil value (method 'is_a')

We need our response to be a Response, not just a dictionary:

function Robot:look()
    local response = Response(self._world:look(self._name))
    local packets = LookPacket:fromResponse(response)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
    self:standardResponse(response)
end

This may break our fromResponse code so let’s enhance its test as well:

        _: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)

This does fail because we have a response now, so we must do this:

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

And this:

function Response:data()
    return self._r.data or {}
end

function Response:dataObjects()
    return self:data().objects or {}
end

And expect green. Green it is: Commit: Robot: look expects a response back, not a dictionary.

Finally, the callback:

Now where were we? Oh, yes, we want to change these to use a callback:

function Robot:look()
    local response = Response(self._world:look(self._name))
    local packets = LookPacket:fromResponse(response)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
    self:standardResponse(response)
end

function WorldProxy:look(name)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    return self.world:request(request)
end

Change Robot to refactor out the callback:

function Robot:look()
    local response = Response(self._world:look(self._name))
    self:lookCallback(response)
end

function Robot:lookCallback(response)
    local packets = LookPacket:fromResponse(response)
    for i,packet in ipairs(packets) do
        self:addLook(packet)
    end
    self:standardResponse(response)
end

We could commit but let’s not. No, let’s do the right thing. Commit: refactor out lookCallback from Robot:look.

Now change to have WorldProxy call it:

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

function WorldProxy:look(name, callback)
    local request = {
        robot=name,
        command="look",
        arguments={}
    }
    local responseDict = self.world:request(request)
    callback(Response(responseDict))
end

Test. Green. Commit: Robot look uses callback mechanism. All commands converted.

Let’s sum up.

Summary

We started with the easy one, launch, and made the change easy and then made the easy change.

Look was not so simple and your engaging host messed it up a few times. The most confusing part was due to a simple type error, where I tried to json.decode a dictionary rather than a string.

It might be wise to cover the Codea JSON with our own little class and methods, where we could check the inputs for being strings and such. Might be worth it, at least for decode.

We had to do a number of steps, three or four at least, to convert “look” to the new callback scheme. In my defense, “look” was one of the first commands implemented, so it was a bit ragged. But then we have to ask ourselves, selves, why did we leave it ragged? Why didn’t we bring it up to standard?

I have no answer for that. We just didn’t. It was working and so we left it alone. Ain’t broke, don’t fix it. May have been the right thing to do since it wasn’t that hard to do today. What might have helped, though, would have been to bring all the commands up to snuff every time we redefine what snuff is, since then, each change would have been fresh in our minds, and smaller as well.

But we did what we did and the results are just fine. We reflect, not to beat ourselves up, but to try to beat into our heads some notions that will help us in future.

The main lesson for me, again and again, is small steps, and right on its heels is “test everything”.

What are your first couple of things you need to learn over and over?

See you next time!

P.S. Just noticed in the pic above: the last green line is the line that needed to be changed to stop that weird error. There is was, right in front of me. What could I have done to notice that a string wasn’t going to come back any more? I’m not sure. But there it was, ready to be spotted. Could have saved a few minutes if I were more clever, but if I were more clever, I’d have more money and an even better car. Same cat, though, probably.