We’re animating our Mimic. Naturally, that leads us to thoughts of coupling, cohesion, and Beck’s Four Rules. Naturally.

Our Mimic has 60 frames of animation available to us. Our previous monsters have about five. Our Mimic has a number of attitudes. It can be hiding, standing about idling, or walking. It can be attacking, hurt, or passing out. These attitudes can be triggered by events, such as the Princess trying to move onto the Mimic’s tile, or by finding itself close enough to attack.

Some sequences will loop. When it’s walking, for example, it will cycle through ten frames of walking animation. When attacking, similarly. Other times, a sequence may want to run just once, and then drop into another. For example, when it wakes up, it’ll go through its “hide” sequence, and then drop into idle.

I’ve noticed a need for other somewhat sophisticated animatiion decisions. The “walk” animation is rather active, and looks a bit odd when the Mimic isn’t actually moving between tiles. And monsters don’t switch tiles all the time, but only when it’s their turn. So if the player dithers, the monsters don’t change locations. (Perhaps they should.)

Be that as it may, I think we want to at least try the Mimic animating its idle sequence most of the time, dropping into walk or attack just when the event takes place.

And so on. I’m looking to get some fairly sophisticated behavior. Let’s take a quick look at how animation is done now, and then we need to talk about coupling, cohesion, and Beck’s Four Rules.

Animation Now

Monsters have a few member variables for animation. The central one is named animations. Each of the variables contains a table of one or more animation frame images, and the game cycles between the frames based on a timer.

function Monster:initAnimations()
    local mtE = self.mtEntry
    self.dead = mtE.dead
    self.hit = mtE.hit
    self.moving = mtE.moving
    self.attack = mtE.attack or self.moving
    self.idle = mtE.idle or self.moving
    self.hide = mtE.hide or self.moving
    self:setAnimation(self.moving)
end

function Monster:setAnimation(animations)
    self.movingIndex = 1
    self.animations = animations
end

When the timer hits, this method is called:

function Monster:chooseAnimation()
    if self:isAlive() then
        self.movingIndex = self.movingIndex + 1
        if self.movingIndex > #self.moving then self.movingIndex = 1 end
    else
        self:setAnimation(self.dead)
    end
    self:setAnimationTimer()
end

It should be clear that this isn’t sophisticated enough to manage the transitions I described above. There’s just one more fragment of code we need to look at:

function Monster:drawMonster()
    if not self.tile.currentlyVisible then return end
    pushMatrix()
    pushStyle()
    spriteMode(CENTER)
    noTint()
    local center = self.tile:graphicCenter()
    translate(center.x,center.y)
    self:flipTowardPlayer()
    sprite(Sprites:sprite(self.animations[self.movingIndex]))
    popStyle()
    popMatrix()
end

Here, the call to sprite looks up the frame that movingIndex points to, and displays it.

This brings us to our theme:

Coupling, Cohesion, and Beck’s Four Rules

You can look up coupling and cohesion on your own, but informally, coupling refers to the amount of connection between disparate parts of a design, and cohesion refers to the extent to which common ideas in the design are kept together.

Beck’s Four Rules of Simple Design are, in my preferred formulation:

The design:

  1. Runs all our tests.
  2. Contains no duplication.
  3. Expresses all our ideas about the design.
  4. Minimizes the number of programming entities.

These rules are taken to be in priority order, and there is some disagreement about the order of numbers 2 and 3. Beck has written with them in both orders. I prefer this order, because the simplicity of the “no duplication” rule often leads us to better designs simply by removing duplication. (Of course we do it while expressing our ideas and so on. The rules all apply, all the time.)

Removing duplication increases cohesion. There used to be two things with the same idea, now there is only one. This also improves the expression of ideas, if we’ll give the new thing a reasonable name and put it in a reasonable place.

Now our present approach to animation is not very cohesive, and induces a fair amount of coupling.

Some but not all of the member variables of Monster refer to animations: dead, hit, moving, attack, idle, hide, animations, moving, and movingIndex. We should include the timer, I suppose, which includes a table animationTimes, with the high and low delay between frames for that monster. There are a number of methods that know something about those variables, including the ones shown above.

The “animation idea” is not represented cohesively in the code, and the code in monster is more highly coupled than it might be. Multiple methods knowing about details like movingIndex and such are interspersed among the many other methods of Monster.

In terms of coupling and cohesion, our animation code has too much coupling and not enough cohesion. In terms of the Four Rules, there’s a bit of duplication, but the main objection I’d have is that, because animation is just spread around in Monster, the idea of “animation” is not as well expressed in the code as it might be.

I think we should fix that.

Improving Animations

With that introduction, how do we improve the coupling and cohesion of something in a program like ours? I’d say that we have two primary tools at our disposal, the class or object, and the method or function. I imagine we’ll use them both.

Let’s imagine an object, to be called Animation, that’s going to do the job for us. For now, we’ll assume that the Monster will have an instance of this object.

The main purpose of the Animation object is to provide a frame to be displayed in the Monster’s drawing, here:

function Monster:drawMonster()
    if not self.tile.currentlyVisible then return end
    pushMatrix()
    pushStyle()
    spriteMode(CENTER)
    noTint()
    local center = self.tile:graphicCenter()
    translate(center.x,center.y)
    self:flipTowardPlayer()
    sprite(Sprites:sprite(self.animations[self.movingIndex]))
    popStyle()
    popMatrix()
end

That sprite call might change either to this:

    sprite(self.animation:frame())

Or, perhaps, to this:

    self.animation:draw()

Of course, as soon as we consider those two options, we see that the second one is better. Now the Monster doesn’t even know whether a sprite will be drawn or something else. Very nice separation of concerns, another randomly selected guideline for good design.

From a coupling-cohesion viewpoint, in the first example above, the Monster is coupled to the type of graphic that the animation draws: it knows it will be a sprite. In the second example, Monster neither knows nor cares how it is drawn. It just knows that it will happen.

Enough musing. Let’s TDD up an Animation class, and see what we can come up with.

Animation Class via TDD

Begin with a test. And, you know what? It’s a pain to create a new test. So what I think I’ll do is create an empty test in a tab of its own, and then copy it when I need a new test. That will make it just a bit easier to start TDDing when otherwise I might not.

-- Test Frame
-- 20210424
-- copy / paste for a new test

function testRenameMe()
    CodeaUnit.detailed = false
    
    _:describe("RENAME ME", function()
        
        _:before(function()
        end)
        
        _:after(function()
        end)
        
        _:test("First Test", function()
            _:expect(4).is(2)
        end)
        
    end)
end

That fails, as expected. Make it run:

        _:test("First Test", function()
            _:expect(2).is(2)
        end)

Then clone it to make my new Animation test, and write the first test. I think I’ll do a cyclic animation as my first test, because when that works we can actually use it.

        _:test("Cyclic Animation", function()
            local frames = { "a", "b", "c" }
            local a = Animation(frames)
            _:expect(a:frame()).is("a")
            a:step()
            _:expect(a:frame()).is("b")
            a:step()
            _:expect(a:frame()).is("c")
            a:step()
            _:expect(a:frame()).is("a")
        end)

That expresses my idea, which is pretty much a chunk in my head. We can imagine building the test up incrementally, but I had it in my brain so I typed it in.

Now to make it work. I coded this in one go:

Animation = class()

function Animation:init(frames)
    self.frames = frames
    self.index = 1
end

function Animation:frame()
    return self.frames[self.index]
end

function Animation:step()
    self.index = (self.index+1) % #self.frames
end

I rather expected that to run. But no:

1: Cyclic Animation  -- Actual: nil, Expected: c

Did I skip a step? No! It’s a real bug, and it’s one I may have perpetrated elsewhere. We want the index to run from 1 through #frames and then cycle. But if we use mod (%) that way, we’ll go 1, 2, 0, 1, 2. That’s seriously not OK.

We can code this clearly, or cleverly. Cleverly would be to let index go 0,1,2 and fetch items 1,2,3. Clearly might be like this:

function Animation:step()
    self.index = self.index+1
    if self.index > #self.frames then
        self.index = 1
    end
end

I think that passes the test. And it does. Let’s skip clever.

I notice that my tests are taking a long time to run. I’ll chase that soon. For now I’m going to skip the long ones.

I removed testDungeonUtilities and testDungeon2 by renaming their top functions. Unfortunately or maybe not, that won’t turn my tests yellow, they’ll just run faster.

Now back to Animation. I wonder how hard it would be to plug it in right now. Do we want to create a new Animation every time we change the frame set, or do we want to provide another frame set to the existing one? I think the former. It doesn’t happen all that often, and it makes the Animation object a bit less complex and less mutable.

We have a function now, setAnimation:

function Monster:setAnimation(animations)
    self.movingIndex = 1
    self.animations = animations
end

Let’s have that create an animation. But first, let’s commit what we have. Our tests run and the Animation class isn’t plugged in, but we have it. Commit: initial Animation class.

OK, plug in the new animation:

function Monster:setAnimation(animations)
    self.animations = Animation(animations)
end

We’ll need to change the stepper, and the code that does the display.

function Monster:chooseAnimation()
    if self:isAlive() then
        self.animations:step()
    else
        self:setAnimation(self.dead)
    end
    self:setAnimationTimer()
end

function Monster:drawMonster()
    if not self.tile.currentlyVisible then return end
    pushMatrix()
    pushStyle()
    spriteMode(CENTER)
    noTint()
    local center = self.tile:graphicCenter()
    translate(center.x,center.y)
    self:flipTowardPlayer()
    sprite(Sprites:sprite(self.animations:frame()))
    popStyle()
    popMatrix()
end

I rather think that will do it. For a test, I’ll set the simple animation monsters to level 10 so that only Mimic is level 1.

That just works. I love when a plan comes together.

mimics

I think they’ll die properly too.

dead

The Mimic transitions nicely into attack mode and when I vanquish it, it winds up in the position shown above. But now maybe we can start to enhance what can be done with Animation. Let’s begin by creating an instance of Animation that runs its cycle once and then stops. Once we have a test for that, we’ll figure out how to do it.

        _:test("One Shot Animation", function()
            local frames = { "a", "b", "c" }
            local a = Animation(frames)
            _:expect(a:frame()).is("a")
            a:step()
            _:expect(a:frame()).is("b")
            a:step()
            _:expect(a:frame()).is("c")
            a:step()
            _:expect(a:frame()).is("c")
        end)

This is good except that as written, the Animation has no clue that we have the one-shot behavior in mind.

Let’s see. We could send it a message after creation, or we could create it knowing what to do. What I prefer is to build a creation method that does the whole job, even though it will be sending messages to the Animation before returning it.

So …

        _:test("One Shot Animation", function()
            local frames = { "a", "b", "c" }
            local a = Animation:oneShot(frames)
            _:expect(a:frame()).is("a")
            a:step()
            _:expect(a:frame()).is("b")
            a:step()
            _:expect(a:frame()).is("c")
            a:step()
            _:expect(a:frame()).is("c")
        end)

We now need a oneShot method, and a plan for how it will work.

One possibility would be a flag in the object to tell it how to iterate. Another, and a better one, I think, would be to have the object plug in a suitable incrementation method.

First this:

Animation = class()

function Animation:init(frames, iterationMethod)
    self.frames = frames
    self.index = 1
    self.iterate = iterationMethod or self.cycle
end

function Animation:cycle()
    self.index = self.index+1
    if self.index > #self.frames then
        self.index = 1
    end
end

function Animation:frame()
    return self.frames[self.index]
end

function Animation:step()
    self.iterate(self)
end

This should give me the existing behavior, and it does. Now for oneShot and its iterationMethod

function Animation:oneShot(frames)
    return self(frames,self.oneShotStep)
end

function Animation:oneShotStep()
    self.index = self.index + 1
    if self.index > #self.frames then
        self.index = #self.frames
    end
end

I expect the new test to run. and it does.

That says that I should be able to change the dead animations for the mimic to use the new oneShot.

    m = {name="Mimic", level=1, health={10,10}, speed={10,10}, strength=10, facing=-1, strategy=MimicMonsterStrategy,
        attackVerbs={"bites", "chomps", "gnaws"},
        dead={"mdead01","mdead02","mdead03","mdead04","mdead05","mdead06","mdead07","mdead08","mdead09","mdead10"},
        hit={"mhurt01", "mhurt02", "mhurt03", "mhurt04", "mhurt05", "mhurt06", "mhurt07", "mhurt08", "mhurt09", "mhurt10"},
        moving={"mwalk01", "mwalk02", "mwalk03", "mwalk04", "mwalk05", "mwalk06", "mwalk07", "mwalk08", "mwalk09", "mwalk10"},
        attack={"mattack01", "mattack02", "mattack03", "mattack04", "mattack05", "mattack06", "mattack07", "mattack08", "mattack09", "mattack10"},
        idle={"midle01", "midle02", "midle03", "midle04", "midle05", "midle06", "midle07", "midle08", "midle09", "midle10"},
        hide={"mhide01", "mhide02", "mhide03", "mhide04", "mhide05", "mhide06", "mhide07", "mhide08", "mhide09", "mhide10"},
    }

There I’ve put all the dead frames into the dead table, and now to set them:

function Monster:chooseAnimation()
    if self:isAlive() then
        self.animations:step()
    else
        self:setAnimation(self.dead)
    end
    self:setAnimationTimer()
end

Hm. We’re not quite set up to do this, are we? Let’s code it directly:

function Monster:chooseAnimation()
    if self:isAlive() then
        self.animations:step()
    else
        self.animations = Animation:oneShot(self.dead)
    end
    self:setAnimationTimer()
end

I expect this should give the Mimic the chance to do something dramatic when he gets killed. Let’s try it.

nope

It did not in fact work. The Mimic just stopped moving.

Oh. We don’t step if you’re not alive. That needs to be improved. And it’s a bit tricky, isn’t it? We want to continue to step the animation even if you are dead, but we want only to set the animation once, when first we notice that you are dead.

How can we best do this? For now, I think I’ll hack in a flag for the one shot, but we’ll need to fix that. Let’s try this:

function Monster:chooseAnimation()
    if self.deadOneShot == nil and self:isDead() then
        self.animations = Animation:oneShot(self.dead)
        self.deadOneShot = true
    else
        self.animations:step()
    end
    self:setAnimationTimer()
end

death throes

dead

OK, that worked. He sort of collapses into a mess, as advertised.

I’m going to commit: Mimic runs one-shot death upon being vanquished.

Now about that flag. Where do we set death?

function Entity:isAlive()
    return self.characterSheet:isAlive()
end

function Entity:isDead()
    return self.characterSheet:isDead()
end

We tell the character sheet to die:

function Entity:die()
    self.characterSheet:die()
end

This could be a good place to do the death throes, except that then the princess might become confused.

Who sends die?

function Entity:damageFrom(amount, kind)
    if not self:isAlive() then return end
    self.characterSheet:applyAccumulatedDamage(amount,kind)
    if self.characterSheet:isDead() then
        sound(self.deathSound, 1, self.pitch)
        self:die()
    else
        sound(self.hurtSound, 1, self.pitch)
    end
end

Here’s probably the main place. I think we’re good. In the Entity:die, let’s set the death animation.

function Entity:die()
    self:setDeathAnimation()
    self.characterSheet:die()
end

In Player, we’ll do nothing:

function Player:setDeathAnimation()
end

And in Monster:

function Monster:setDeathAnimation()
    self.animations = Animation:oneShot(self.dead)
end

And back in chooseAnimation:

function Monster:chooseAnimation()
    self.animations:step()
    self:setAnimationTimer()
end

That’s rather neat. Does it work? Let’s find out.

throes again

Yes, just fine. I can imagine that we might want to run that animation a bit faster, and it’s clear that the crawl could obscure the action. Not our concern just now.

Set the other monsters back into play and then commit: Monsters use Animation object. Mimic dies nicely.

Good spot to break on a Saturday. Let’s sum up.

Summary

We have a new object, Animation, that encapsulates the frames of an animation and the index of the frame currently to be shown. It returns the frame to be displayed.

The Animation does not, as yet, embody the timer-based stepping, nor does it switch from one sheet to another on its own. I think we should probably move the timer logic inside, and give it a draw method that can be called to display the frame / sprite. I’m not sure about generally switching between sheets, but I do think we might want one capability: play this sheet once and then cycle this second one. We could use that when the Mimic comes to life, converting from his chest appearance to his monster appearance. He might go through “hide” and then into “idle” after one hide cycle.

For some reason, the “hide” sheet is more of an “unhide”, but we could run it backwards if we want him to drop back into hidden mode.

But already we have a fair bit of uncoupling in place. The stepper doesn’t know how the Animation instance steps, with that logic hidden inside via pluggable behavior, two different cycling methods. Which reminds me of a change we need:

I named the step method for “one shot” as oneShotStep. The other method, the default one, is named cycle. Let’s rename that to cycleStep for consistency and, I hope, a bit of clarity.

That leaves us with this:

Animation = class()

function Animation:oneShot(frames)
    return self(frames,self.oneShotStep)
end

function Animation:init(frames, iterationMethod)
    self.frames = frames
    self.index = 1
    self.iterate = iterationMethod or self.cycleStep
end

function Animation:cycleStep()
    self.index = self.index+1
    if self.index > #self.frames then
        self.index = 1
    end
end

function Animation:frame()
    return self.frames[self.index]
end

function Animation:oneShotStep()
    self.index = self.index + 1
    if self.index > #self.frames then
        self.index = #self.frames
    end
end

function Animation:step()
    self.iterate(self)
end

OK. As long as I’m changing things, instead of being done for the day, let’s commit that and then do another quick change.

Commit: rename Animation:cycle to Animation:cycleStep.

Now that we’re not quite summarizing, let’s also give Animation a draw:

function Animation:draw()
    sprite(Sprites:sprite(self:frame()))
end

And let’s call that from Monster:

Old:

function Monster:drawMonster()
    if not self.tile.currentlyVisible then return end
    pushMatrix()
    pushStyle()
    spriteMode(CENTER)
    noTint()
    local center = self.tile:graphicCenter()
    translate(center.x,center.y)
    self:flipTowardPlayer()
    sprite(Sprites:sprite(self.animations:frame()))
    popStyle()
    popMatrix()
end
~~

New:

~~~lua
function Monster:drawMonster()
    if not self.tile.currentlyVisible then return end
    pushMatrix()
    pushStyle()
    spriteMode(CENTER)
    noTint()
    local center = self.tile:graphicCenter()
    translate(center.x,center.y)
    self:flipTowardPlayer()
    self.animations:draw()
    popStyle()
    popMatrix()
end

I’ll test that with a bit of game play. The monsters are all animating as before. Commit: Animation can draw itself. Monster uses it.

Now we have even more of the responsibilities nicely bundled into Animation. It knows the frames, it cycles them, and it draws the right one. Once we give it the timing, we’ll have even more of the stuff that belongs with Animation in there. I’m not sure what we’ll want to do about the more strategic changes of animation. I don’t think those should be automatic so much as volitional in the Monster’s strategy.

There’s also the question of how the Monster should store all those different animations that are now in a half-dozen member variables.

But all that’s for another day.

What we see here is a very smooth process of removing some behavior from Monster, putting it over in Animation. We have reduced the coupling of the system. Monster knows less about animation than it did, and much of the inner workings of animation is over in the Animation class.

We’re not done, but we’ve made the world a better place, and given the monster some neat new behavior as well.

See you next time!

D2.zip