Today, girls just want to have fun, and so do I.

I hate to seem like a complainer here, but this hassle with the Combat has been irritating. I’ve felt that I couldn’t move on to other things while the rudimentary combat cycle wasn’t working. Now I have confidence in it and I feel we can move forward.

Even cleaning up some code would feel good today. And I do have something in mind there.

It is my practice, lately, to keep methods in alphabetical order within a class, perhaps also separated out by class vs instance methods, occasionally otherwise grouped. But since Codea has no real hierarchical view in the IDE (coming soon, I’m told), alphabetical order is quite helpful when you’re looking for something.

I’ve noticed in particular that Player needs help, so I’ll do that. I’ll spare you the display of 200 lines out of order and then 200 in order. Quickly done. A nice little warmup, and it leaves things a bit easier to work with.

I was thinking that now that the combat round works, it should be beefed up a bit. And I think it needs a bit of internal improvement as well:

CombatRound = class()

function CombatRound:init(attacker,defender, rng)
    self.attacker = attacker
    self.defender = defender
    self.random = rng or math.random
end

function CombatRound:__tostring()
    return "CombatRound"
end

function CombatRound:attack()
    if self.attacker:willBeDead() or self.defender:willBeDead() then return {} end
    local result = {}
    table.insert(result, self:display("  "))
    local msg = string.format("%s attacks %s!", self.attacker:name(), self.defender:name())
    table.insert(result, self:display(msg) )
    local cmds = self:attemptHit()
    table.move(cmds,1,#cmds,#result+1, result)
    return result
end

function CombatRound:attemptHit()
    local result = {}
    local msg = string.format("%s whacks %s!", self.attacker:name(), self.defender:name())
    table.insert(result, self:display(msg) )
    local cmds = self:rollDamage()
    table.move(cmds,1,#cmds,#result+1, result)
    return result
end

function CombatRound:display(aString)
    return OP:display(aString)
end

function CombatRound:rollDamage()
    local result = {}
    local damage = self.random(1,6)
    self.defender:accumulateDamage(damage)
    local op = { op="extern", receiver=self.defender, method="damageFrom", arg1=nil, arg2=damage }
    table.insert(result,op)
    table.insert(result, self:display(self.defender:name().." takes "..damage.." damage!"))
    return result
end

The structure here is left over from the time when the various methods were called by the crawl, one by one. So each one needed to return its own collection of results. We wrap the whole thing up as a unit now, so we can go to a single accumulator. I think that should make the code more clear. Let’s try it.

function CombatRound:init(attacker,defender, rng)
    self.attacker = attacker
    self.defender = defender
    self.random = rng or math.random
    self.commandList = {}
end

I’ve added a new member variable, commandList, an empty table. The plan will be to accumulate everything into that list and return it from here:

function CombatRound:attack()
    if self.attacker:willBeDead() or self.defender:willBeDead() then return {} end
    local result = {}
    table.insert(result, self:display("  "))
    local msg = string.format("%s attacks %s!", self.attacker:name(), self.defender:name())
    table.insert(result, self:display(msg) )
    local cmds = self:attemptHit()
    table.move(cmds,1,#cmds,#result+1, result)
    return result
end

I think all the other methods here can be considered private, though they are called in the tests. We’ll see about that as we move along.

I’ll be creating one or more helper methods as we go here.

function CombatRound:append(cmd)
    table.insert(self.commandList, cmd)
end

function CombatRound:appendAll(cmds)
    table.move(cmds,1,#cmds,#self.commandList+1, self.commandList)
end

function CombatRound:attack()
    if self.attacker:willBeDead() or self.defender:willBeDead() then return self.commandList end
    self:append(self:display("  "))
    local msg = string.format("%s attacks %s!", self.attacker:name(), self.defender:name())
    self:append(self:display(msg))
    local cmds = self:attemptHit()
    self:appendAll(cmds)
    return self.commandList
end

I think this is correct. Tests will tell. Everything seems to work, though I did think I saw a late “Ghost is dead” message scrolling by after another monster attack. I’m going to ignore that, but I have to confess, it’s making me a bit sad to think that there may still be something off about combat and the crawl.

My overall plan here is to have the methods that are called in attack do their own appending to the command list, rather than accumulate a temp result and then move it. So …

function CombatRound:attack()
    if self.attacker:willBeDead() or self.defender:willBeDead() then return self.commandList end
    self:append(self:display("  "))
    local msg = string.format("%s attacks %s!", self.attacker:name(), self.defender:name())
    self:append(self:display(msg))
    self:attemptHit()
    return self.commandList
end

function CombatRound:attemptHit()
    local msg = string.format("%s whacks %s!", self.attacker:name(), self.defender:name())
    self:append(self:display(msg) )
    local cmds = self:rollDamage()
    self:appendAll(cmds)
end

Again this seems likely to work. But it breaks a test, because now attemptHit isn’t returning a result and the test expects that it will. Let’s see what we can do about that:

        _:test("attempt hit", function()
            local result
            local i,r
            local player = FakeEntity("Princess")
            local monster = FakeEntity("Spider")
            local co = CombatRound(player, monster)
            result = co:attemptHit()
            i,r = next(result,i)
            _:expect(r.op).is("display")
            _:expect(r.text).is("Princess whacks Spider!")
        end)

If we fetch the commandList from co, we should find this test running.

        _:test("attempt hit", function()
            local result
            local i,r
            local player = FakeEntity("Princess")
            local monster = FakeEntity("Spider")
            local co = CombatRound(player, monster)
            co:attemptHit()
            result = co.commandList -- invasive
            i,r = next(result,i)
            _:expect(r.op).is("display")
            _:expect(r.text).is("Princess whacks Spider!")
        end)

Test is green. I note in passing that an improvement I have in mind is going to break these tests further. We’ll talk about that soon. Let’s finish this refactoring improvement.

Next step is to make rollDamage do its own accumulation:

function CombatRound:attemptHit()
    local msg = string.format("%s whacks %s!", self.attacker:name(), self.defender:name())
    self:append(self:display(msg) )
    self:rollDamage()
end

function CombatRound:rollDamage()
    local damage = self.random(1,6)
    self.defender:accumulateDamage(damage)
    local op = { op="extern", receiver=self.defender, method="damageFrom", arg1=nil, arg2=damage }
    self:append(op)
    self:append(self:display(self.defender:name().." takes "..damage.." damage!"))
end

This probably breaks more tests.

3: rollDamage -- TestCombatRound:55: bad argument #1 to 'next' (table expected, got nil)

Yes. Same fix:

        _:test("rollDamage", function()
            local result
            local i,r
            local player = FakeEntity("Princess")
            local monster = FakeEntity("Spider")
            randomNumbers = {4}
            local co = CombatRound(player, monster, fakeRandom)
            co:rollDamage()
            result = co.commandList
            i,r = next(result,i)
            _:expect(r.op).is("extern")
            _:expect(r.receiver).is(monster)
            _:expect(r.method).is("damageFrom")
            _:expect(r.arg2).is(4)
            i,r = next(result,i)
            _:expect(r.op).is("display")
            _:expect(r.text).is("Spider takes 4 damage!")
        end)

All good. However, this code seems odd to me. attack calls attemptHit, which calls rollDamage. But there are reasons why it wants to be that way. The sequence is conditional, and in fact my plan is to enhance it to make it more so. But first, one more minor improvement:

function CombatRound:attack()
    if self.attacker:willBeDead() or self.defender:willBeDead() then return self.commandList end
    self:append(self:display("  "))
    local msg = string.format("%s attacks %s!", self.attacker:name(), self.defender:name())
    self:append(self:display(msg))
    self:attemptHit()
    return self.commandList
end

I think this will be more clear this way:

function CombatRound:attack()
    if self.attacker:willBeAlive() and self.defender:willBeAlive() then
        self:append(self:display("  "))
        local msg = string.format("%s attacks %s!", self.attacker:name(), self.defender:name())
        self:append(self:display(msg))
        self:attemptHit()
    end
    return self.commandList
end

We don’t have willBeAlive but that is easily remedied:

function Entity:willBeAlive()
    return not self:willBeDead()
end

A quick test … Oddly, I’m getting this error:

6: monster attack -- TestCombatRound:123: attempt to call a nil value (method 'willBeAlive')

Oh. The error is from my Fakes.

function FakeEntity:willBeAlive()
    return not self:willBeDead()
end

This is one reason why I don’t like using test doubles: they often require changes after confusing me. If I were smarter, perhaps I’d like them better.

Testing, all seems well. Let’s commit: refactoring CombatRound.

Now, I’d like to improve the cycle just a bit. I have in mind that an attempted hit could fail. This would be done by a roll of attacker speed against defender speed. If defender is faster, the attack misses.

function CombatRound:attemptHit()
    local msg
    if self.attacker:speedRoll() >= self.defender:speedRoll() then
    msg = string.format("%s whacks %s!", self.attacker:name(), self.defender:name())
    self:append(self:display(msg) )
    self:rollDamage()
    else
        msg = string.format("%s evades attack!", self.defender:name())
        self:append(self:display(msg))
    end
end

We need to implement speedRoll of course:

function Entity:speedRoll()
    return math.random(1,self:speed())
end

I remembered to put something in for FakeEntity this time.

evades

The new evasion feature works nicely. Let’s commit: faster entity can evade attack.

I think we’ll want to improve things further here, with different weapons and perhaps armor and the like. The mechanism seems to be in place and works well. I do think I’ve seen some sequencing issues in the crawl, but I’m not sure. Nothing really odd has happened, to the point where I’m not sure anything’s wrong, and a user probably will not notice a problem.

That’s not very comforting to me, and it shouldn’t be comforting to you either. If the code works, we should know it. If it doesn’t, we should know it, find out why, and fix it. “Seems OK” is not a good state to be in.

But sometimes … that’s the best we seem to get.

I was also thinking of improving the text that comes out. Every attacker always “whacks” the defender. That was fine when I was just testing, but now I’d like to have something more varied and interesting.

What if entities knew an “attack verb”? Perhaps a serpent poisons the princess, a vampire bat bites her, a ghost terrifies her, and so on. We could add that to the monster table, and give the princess a list of her own.

Let’s try:

function Monster:attackVerb()
    return self.mtEntry.attackVerb or "whacks"
end

...
    self.attackVerbs = {"whacks", "bashes", "stabs", "slaps", "punches", "slams"}
...

function Player:attackVerb()
    return self.attackVerbs[math.random(1,#self.attackVerbs)]
end

And finally:

function CombatRound:attemptHit()
    local msg
    if self.attacker:speedRoll() >= self.defender:speedRoll() then
    msg = string.format("%s %s %s!", self.attacker:name(), self.attacker:attackVerb(), self.defender:name())
    self:append(self:display(msg) )
    self:rollDamage()
    else
        msg = string.format("%s evades attack!", self.defender:name())
        self:append(self:display(msg))
    end
end

verbs

So that’s fun. It’s coming up on lunch time, let’s call it a day. We’ll do the monster ones tomorrow.

Commit: attack verbs for Player.

Summary

It’s so freeing to feel that things are under control. We managed to alphabetize a class today, then improved combat’s code, then added an additional combat feature, the possibility of evading a strike, and then added some interesting verbs to the Player’s vocabulary.

Strikes. I’ll add that. Commit: added “strikes” to player attack verbs.

So today has gone smoothly. I do believe I’ve seen some crawl anomalies, such as a death message coming out a bit late, but I’m not certain. I’ve certainly not seen anything really strange like we used to see. We’ll stay on the alert, and I’ll try to think of some way to test things more extensively, but the inherently asynchronous behavior makes that difficult, at least for the likes of me.

Overall, a pleasant morning, and that’s just what I needed. I wish the same to you!

D2.zip