P-249 - Masker Aid

In which, a small object helps improve our code.

One might think that the ImageMasher is about as tight as it can get, as clear as it can get. But one might recall that I’ve been saying that the code seems to be asking for another object to help out. Here’s one explanation for why that might be the case.

We’ve noted already that the ImageMasher factory method accepts two Flyers, a target and a shot. It creates the ImageMasher instance, which determines damage to the target mask, by erasing bits from the shot mask and its associated explosion mask.

class ImageMasher:
    def determine_damage(self):
        self.erase_shot()
        self.erase_explosion()

    def erase_shot(self):
        self.erase_mask(self.shot_mask)

    def erase_explosion(self):
        self.erase_mask(self.explosion_mask)

Internally, the Masher concerns itself with manipulating various masks, rectangles and offsets as it does its work:

    def erase_mask(self, shot_mask):
        shot_offset = self.mask_offset_from_target(shot_mask)
        self.new_mask.erase(shot_mask, shot_offset)

    def mask_offset_from_target(self, mask):
        explosion_rectangle = self.mask_rectangle_in_shot_position(mask)
        return self.damage_offset_from_target(explosion_rectangle)

    def mask_rectangle_in_shot_position(self, mask):
        rectangle_moved_to_shot_position = mask.get_rect()
        rectangle_moved_to_shot_position.center = self.shot_center_position
        return rectangle_moved_to_shot_position

    def damage_offset_from_target(self, damage_rectangle):
        return self.offset(damage_rectangle.topleft, self.target_topleft_position)

    @staticmethod
    def offset(point1, point2):
        return Vector2(point1) - Vector2(point2)

After all that messing about is done, ImageMasher is ready to return the updated mask and to blit that mask to the target surface.

Aside: Someone has to update the surface. Having the ImageMasher do so seems a bit off, but for now I don’t see anything that I’d prefer. But I digress.

This morning I plan to create a new helper object, a Masker. I think it will be created with a mask and a position, and that one Masker will be able to mask, i.e. erase, another. Whether it will do that in place or create a new masker, I don’t know. There are probably good arguments either way.

I am not certain about any of the above: the code has not yet had a chance to express its opinion of this idea, but I am pretty confident that we’ll wind up with something helpful.

I trust my tests (and you can be sure I’ll run the game from time to time anyway). Because I trust the existing tests, and because they won’t run if the Masker doesn’t work, we’ll create it and add it in without new tests. We might decide that tests are needed after all, but I feel confident that we can just build this thing in and then make the tests run again.

No, by golly.: If I proceed that way, several tests will be broken for a moderately long time as I build up the small Masker and get it correctly integrated. It’s better to go in two steps. We’ll TDD it. Not because it’s our rule, or because we always do, but because in this case, fewer tests will break. In fact, with a bit of luck, no Masher tests will break at all.

OK, TDD:

    def test_masker_exists(self, make_plus):
        plus = make_plus
        masker = Masker(plus.mask, (100, 200))

Because the Masker is just a helper for ImageMasher, I’ll put this little class in the same file.

class Masker:
    def __init__(self, mask, position):
        pass

As soon as I import the new class, the test runs green.

The only operation of the masker is to erase pixels, so let’s create a simple erasure test and then, later, make it a bit harder.

I think this will do for starters:

    def test_masker_centered(self):
        five_data = """
        11111
        11111
        11111
        11111
        """
        target_mask = self.mask_from_string(five_data)
        three_data = """
        111
        111
        111
        """
        bullet_mask = self.mask_from_string(three_data)
        target = Masker(five_data, (100, 200))
        bullet = Masker(three_data, (100, 200))
        target.erase(bullet)
        erased = target.get_mask()
        hits = 
        self.check_bits(erased, hits)

Note: I meant that top five_data to be 5x5. It isn’t. I discover that later. No harm done.

I’ll probably move those masks up as fixtures, although I only plan to use them one more time.

The new methods erase and get_mask need implementing. Let’s see how far I can get. It seems rather far but I don’t have a smaller idea. Also, I forgot to commit my first running test and empty class. NNo biscuit for me.

Moving forward:

class Masker:
    def __init__(self, mask, position):
        self.mask = mask
        self.rect = mask.get_rect()
        self.rect.center = position

Why did I do that? Well, because I have in mind what this object will do, I’m not discovering it as I sometimes do. It’s setting the center of the rect so that it can use topleft to get an offset. We’ll see how we use that in a moment.

We now need the erase and get_mask methods. get_mask seems easy:

    def get_mask(self):
        return self.mask

Now if I implement erase as pass the test should fail nicely.

    def erase(self, masker):
        pass

Sure enough, the test fails, printing … something surprising me …

>       self.rect = mask.get_rect()
E       AttributeError: 'str' object has no attribute 'get_rect'

That’s because I interfered with the canine on the test, which should be:

        bullet_mask = self.mask_from_string(three_data)
        target = Masker(target_mask, (100, 200))
        bullet = Masker(bullet_mask, (100, 200))
        target.erase(bullet)
        erased = target.get_mask()
        hits = []
        self.check_bits(erased, hits)

The test does not fail, however, because I’m not asking for any bits to be cleared. Let’s make it fail.

        hits = [(0, 0)]

Now it prints the block I expected:

Well, almost. there are only four rows and I intended five. I am striking out more often than I get to the plate. Fix that.

Perfect failure at last. Now let’s do the erase.

Note that my two objects have already set their member variable rect centers to their position. That means that the difference between their toplefts is the offset I need for the erase.

How should erase work? Let’s write it out longhand to begin with. We’ll try this:

    def erase(self, masker):
        his_mask = masker.mask
        his_topleft = masker.topleft
        offset = his_topleft - self.topleft
        self.mask.erase(his_mask, offset)

Now I need the topleft property.

    @property
    def topleft(self):
        return Vector2(self.rect.topleft)

Now if this all works as I expect, there should be a hole in the middle of the test output.

11111
10001
10001
10001
11111
[(1, 1), (2, 1), (3, 1), (1, 2), (2, 2), (3, 2), (1, 3), (2, 3), (3, 3)]

Just what I wanted! Now I can paste that list into hits (Approval Test) and I’ll be green.

We are green. Commit: initial tests and Masker object.

One more test and I’ll be satisfied. I’m going to move the bullet down and two the right.

    def test_masker_down_right(self):
        five_data = """
        11111
        11111
        11111
        11111
        11111
        """
        target_mask = self.mask_from_string(five_data)
        three_data = """
        111
        111
        111
        """
        bullet_mask = self.mask_from_string(three_data)
        target = Masker(target_mask, (100, 200))
        bullet = Masker(bullet_mask, (101, 201))
        target.erase(bullet)
        erased = target.get_mask()
        hits = [(2, 2), (3, 2), (4, 2), (2, 3), (3, 3), (4, 3), (2, 4), (3, 4), (4, 4)]
        self.check_bits(erased, hits)

I swear to you that I manually edited the hits to adjust for the movement of the bullet. And we’re green. I want to see the output so I add an assert False to the test for a moment.

As desired! I love it when a plan comes together. Green. Commit: Masker passing its two tests.

I think I’m ready to use it. Notice that I chose to update the erase target in place. We might wish for immutability. We’ll try to think about that later. For now, I am psyched to install it.

We have this:

class ImageMasher:
    @classmethod
    def from_flyers(cls, target, shot):
        return cls(target.mask, target.rect.topleft, shot.position, shot.mask, shot.explosion_mask)

    def __init__(self, target_mask, target_topleft, shot_position, shot_mask, explosion_mask):
        self.new_mask = target_mask.copy()
        self.target_topleft_position = target_topleft
        self.shot_center_position = shot_position
        self.shot_mask = shot_mask
        self.explosion_mask = explosion_mask

I think we should create the maskers in the class method and create ImageMasher with nothing but maskers. Each Masker, we recall, gets created with its mask and position. Like this:

    @classmethod
    def from_flyers(cls, target, shot):
        target_masker = Masker(target.mask, target.position)
        shot_masker = Masker(shot.mask, shot.position)
        explosion_masker = Masker(shot.explosion_mask, shot.position)
        return cls(target_masker, shot_masker, explosion_masker)

Of course init does not expect that, so we’ll proceed accordingly:

    def __init__(self, target_masker, shot_masker, explosion_masker):
        self.target_masker = target_masker
        self.shot_masker = shot_masker
        self.explosion_masker = explosion_masker

Now some of the methods below are not happy. Let’s look at the rest of the class so that we can appreciate how it changes.

    def determine_damage(self):
        self.erase_shot()
        self.erase_explosion()

    def erase_shot(self):
        self.erase_mask(self.shot_mask)

    def erase_explosion(self):
        self.erase_mask(self.explosion_mask)

    def erase_mask(self, shot_mask):
        shot_offset = self.mask_offset_from_target(shot_mask)
        self.new_mask.erase(shot_mask, shot_offset)

    def mask_offset_from_target(self, mask):
        explosion_rectangle = self.mask_rectangle_in_shot_position(mask)
        return self.damage_offset_from_target(explosion_rectangle)

    def mask_rectangle_in_shot_position(self, mask):
        rectangle_moved_to_shot_position = mask.get_rect()
        rectangle_moved_to_shot_position.center = self.shot_center_position
        return rectangle_moved_to_shot_position

    def damage_offset_from_target(self, damage_rectangle):
        return self.offset(damage_rectangle.topleft, self.target_topleft_position)

    @staticmethod
    def offset(point1, point2):
        return Vector2(point1) - Vector2(point2)

    def get_new_mask(self):
        return self.new_mask

    def apply_damage_to_surface(self, surface):
        area_to_blit = self.new_mask.get_rect()
        damaged_surface = self.new_mask.to_surface()
        surface.blit(damaged_surface, area_to_blit)

The first methods to be unhappy are the erase_shot and erase_eplosion. We have a new way to do those:

    def erase_shot(self):
        self.target_masker.erase(self.shot_masker)

    def erase_explosion(self):
        self.target_masker.erase(self.explosion_masker)

Some tests are failing, of course, since we do have ImageMasher tests that we know and love. But we’re not done here.

We are, however, almost done, because after erase_explosion is called, the target_masker has the updated mask. So:

    def get_new_mask(self):
        return self.target_masker.mask

    def apply_damage_to_surface(self, surface):
        new_mask = self.get_new_mask()
        area_to_blit = new_mask.get_rect()
        damaged_surface = new_mask.to_surface()
        surface.blit(damaged_surface, area_to_blit)

The tests are green! I will check the game now. I’m glad I did because:

AttributeError: 'Shield' object has no attribute 'position'

Huh. I thought it had. Fix:

class Shield(InvadersFlyer):
    @property
    def position(self):
        return self._rect.center

With that in place the game works perfectly. I sort of wish a test had found that missing position method, but it was not to be. Let’s now remove all the code from ImageMasher that it does not use.

And here is the new ImageMasher, in its entirety:

class ImageMasher:
    @classmethod
    def from_flyers(cls, target, shot):
        target_masker = Masker(target.mask, target.position)
        shot_masker = Masker(shot.mask, shot.position)
        explosion_masker = Masker(shot.explosion_mask, shot.position)
        return cls(target_masker, shot_masker, explosion_masker)

    def __init__(self, target_masker, shot_masker, explosion_masker):
        self.target_masker = target_masker
        self.shot_masker = shot_masker
        self.explosion_masker = explosion_masker

    def determine_damage(self):
        self.erase_shot()
        self.erase_explosion()

    def erase_shot(self):
        self.target_masker.erase(self.shot_masker)

    def erase_explosion(self):
        self.target_masker.erase(self.explosion_masker)

    def get_new_mask(self):
        return self.target_masker.mask

    def apply_damage_to_surface(self, surface):
        new_mask = self.get_new_mask()
        area_to_blit = new_mask.get_rect()
        damaged_surface = new_mask.to_surface()
        surface.blit(damaged_surface, area_to_blit)

We have removed 19 lines from ImageMasher, just shy of half: it’s 21 lines now. Of course, we added a new class, Masker:

class Masker:
    def __init__(self, mask, position):
        self.mask = mask
        self.rect = mask.get_rect()
        self.rect.center = position

    @property
    def topleft(self):
        return Vector2(self.rect.topleft)

    def erase(self, masker):
        his_mask = masker.mask
        his_topleft = masker.topleft
        offset = his_topleft - self.topleft
        self.mask.erase(his_mask, offset)

    def get_mask(self):
        return self.mask

That’s 18 lines, so we haven’t reduced total code volume, but look at what we’ve done for clarity. It seems to me that what’s going on now is far more clear.

In ImageMasher the work is obvious: from the target we erase a shot and an explosion:

    def determine_damage(self):
        self.erase_shot()
        self.erase_explosion()

We do that using three Maskers:

    def erase_shot(self):
        self.target_masker.erase(self.shot_masker)

    def erase_explosion(self):
        self.target_masker.erase(self.explosion_masker)

We return the damaged mask on request:

    def get_new_mask(self):
        return self.target_masker.mask

And, still a bit odd, we offer a service to apply the new mask to your surface:

    def apply_damage_to_surface(self, surface):
        new_mask = self.get_new_mask()
        area_to_blit = new_mask.get_rect()
        damaged_surface = new_mask.to_surface()
        surface.blit(damaged_surface, area_to_blit)

Admit it, that’s pretty easy to figure out. But how do we do those erasures? Well, with this:

    def erase(self, masker):
        his_mask = masker.mask
        his_topleft = masker.topleft
        offset = his_topleft - self.topleft
        self.mask.erase(his_mask, offset)

We get the other masker’s mask, use his topleft and ours to compute his offset relative to us, and we erase his bits in our mask.

I think I’d inline that one temp:

    def erase(self, masker):
        his_mask = masker.mask
        offset = masker.topleft - self.topleft
        self.mask.erase(his_mask, offset)

Maybe inline the mask too:

    def erase(self, masker):
        offset = masker.topleft - self.topleft
        self.mask.erase(masker.mask, offset)

Yes. Commit: tidying.

Let’s sum up.

Summary

About eight or nine articles ago, I was uncomfortable with the code in Shield that updated its mask and surface. It seemed to have nothing to do with being a Shield and a lot to do with masks, rectangles, and points. Over a few articles, I wrote some tests to learn about masks and then we started work on ImageMasher.

I felt that that ImageMasher improved Shield, since Shield no longer had to think about all those details, but ImageMasher itself was not a paragon of clarity. We improved ImageMasher’s code over a few sessions. One key discovery or realization was that we didn’t need the overlap_mask facility, and could just erase the shot from the shield. That made the two branches of ImageMasher, erasing shot and erasing explosion, the same, resulting in a bit of code compaction.

But there was still something odd about ImageMasher and even after a number of improvements it seemed to be digging down too deeply into its member variables—and the member variables themselves were oddly different.

Somehow, and if I could explain it better I would, it was seeming to me more and more that a small helper object would make things better. Yesterday I thought about it and this morning we implemented Masker, which neatly juggles the mask, its rectangle, its center, and its resulting top left. Those are pretty close to all being at the same level of abstraction, and the result was that Masker’s erase operation is pretty simple and makes sense.

I like the result. Shield is much simpler. ImageMasher is much simpler, half the size that it originally was. And even Masker is pretty simple, really only embodying one operation, erasure.

Small objects and “Many more much smaller steps” have improved our code base.

Worthwhile?

Well, we’ve talked about this. In this code base, as small as it is, and as unlikely to change as it is, maybe this won’t pay off in increased maintainability, speed of change, and the like.

But in my brain, my capabilities, my facility with the code, it has paid off a lot.

And if it gives you a bit of an idea … that’s absolutely worth it!

See you next time!