Python Asteroids+Invaders on GitHub

Part two, following P-247. Another idea. And we still have more opportunities to improve! Wait, SQUIRREL! Two of them!

I hope you’ve come back. If you have not come back, don’t read this. This article continues #247.

Oh No, Another Idea

There’s no doubt that ImageMasher mostly thinks about masks, rectangles, and points. It would probably be better to make its member variables more about those things and less about Flyer instances. We could do that with a factory method that takes Target and Shot and produces an ImageMaker that has the masks and such.

Let’s do it step by step. First let’s set up the masks and rectangles that we need. We’ll proceed in steps, not least because we’re going to break some tests. We start here:

class ImageMasher:
    def __init__(self, target, shot):
        self.target = target
        self.shot = shot
        self.new_mask = self.target.mask.copy()

Wait, SQUIRREL!. Look at a bit more of this:

class ImageMasher:
    def __init__(self, target, shot):
        self.target = target
        self.shot = shot
        self.new_mask = self.target.mask.copy()

    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.shot.explosion_mask

Hey! We do exactly the same thing twice, once with the shot mask and once with the target mask.

Well … not quite exactly. The two erases are one on top of the other, first the shot, then the explosion, both accumulating into one mask. Still, it’s all the same.

Why is this one object doing one thing twice? Why isn’t it two objects each doing one thing once?

We’ll ignore the squirrel for now, but I think we’ll want to revisit that question. Let’s get this object closer to being a mask processor. We’ll do that by setting up more values in __init__ rather than referencing them down in the methods.

See those two references self.shot.mask and self.shot.explosion_mask? Let’s fetch those in __init__. That will break some tests that don’t provide explosion masks. Here goes:

Let’s start by not saving target, instead the mask.

class ImageMasher:
    def __init__(self, target, shot):
        self.target_mask = target.mask
        self.shot = shot
        self.new_mask = self.target_mask.copy()

This requires a change to accommodate this:

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

Let’s just cache the topleft for target:

class ImageMasher:
    def __init__(self, target, shot):
        self.target_mask = target.mask
        self.target_topleft = target.rect.topleft
        self.shot = shot
        self.new_mask = self.target_mask.copy()

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

We are green. Commit: caching target mask and topleft, not target.

Similarly with shot, which will cause a little more trouble.

class ImageMasher:
    def __init__(self, target, shot):
        self.target_mask = target.mask
        self.target_topleft = target.rect.topleft
        self.shot_mask = shot.mask
        self.new_mask = self.target_mask.copy()

Tests fail and there are markers in the code. The first was obvious, the next two not so obvious:

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

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

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

I think we’ll cache both the position, which we absolutely need, and the explosion mask.

class ImageMasher:
    def __init__(self, target, shot):
        self.target_mask = target.mask
        self.target_topleft = target.rect.topleft
        self.shot_mask = shot.mask
        self.shot_position = shot.position
        self.explosion_mask = shot.explosion_mask
        self.new_mask = self.target_mask.copy()

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

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

I have two tests failing. They have the same problem:

E       AttributeError: 'Thing' object has no attribute 'explosion_mask'

That’s because early tests did not use that mask (and the erase_shot still doesn’t. Now those tests must provide something.) I just add in an explosion, which is unused in the test:

    def test_masher_exists(self, make_missile, make_target, make_small_explosion):
        shield = make_target
        shot = make_missile
        expl = make_small_explosion
        shot.explosion_mask = expl.mask
        ImageMasher(shield, shot)

    def test_masher_vs_shot(self, make_missile, make_target, make_small_explosion):
        shield = make_target
        shield.position = (100, 200)
        shot = make_missile
        shot.position = (100, 200)
        expl = make_small_explosion
        shot.explosion_mask = expl.mask
        assert shot.rect.center == (100, 200)
        masher = ImageMasher(shield, shot)
        masher.erase_shot()
        mask = masher.get_new_mask()
        hits = [(3, 3), (4, 3), (5, 3), (4, 4), (4, 5)]
        self.check_bits(mask, hits)

Green. Commit: caching shot position and mask, not shot.

Let’s review the init and consider a class factory method:

class ImageMasher:
    def __init__(self, target, shot):
        self.target_mask = target.mask
        self.target_topleft = target.rect.topleft
        self.shot_mask = shot.mask
        self.shot_position = shot.position
        self.explosion_mask = shot.explosion_mask
        self.new_mask = self.target_mask.copy()

Let me see if I can explain why I want to do this next thing. The input to init is two Flyer instances. The object wants to think about three masks, one target and two erasure masks, and about two positions, target top left and shot center. I want to begin to separate those concerns. (And, I begin to think, since we can’t make use of both top lefts, perhaps we should make use of two center positions rather than one center position and one top left. Balance. Symmetry. Stuff like that. We’ll see.)

So to separate the concerns, let’s create a class method that takes the target and shot and constructs the ImageMasher given what it actually wants.

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

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

This seems righteous. Tests are failing, probably just because they’re not calling from_flyers. Yes! We just insert from_flyers into four tests and we are green. OOps, I also have to fix Shield to use the new class method. There is no test that detects that error. Must think about that.

Commit: new from_flyers factory method.

This is getting long. I’ll split this article into two, but I don’t feel done yet. I’ve been going about three hours and fifteen minutes and not really whipped yet.

What troubles me now is the difference between getting the position of the shot, which we absolutely need because we have to set the centers of the shot mask rectangle, and getting the target top_left. Maybe it’s OK, the target is special anyway.

I notice that we don’t actually use the target_mask other than to copy it. Let’s change that:

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

Nice. Removed a whole member variable. Commit: remove unused member.

Let’s reorder the init a bit, to show a symmetry:

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

I wanted to get the setting of two masks together. I feel that that’s important somehow. Let’s change the signature of the init to match the order of use of the parms.

Weirdly, PyCharm does not find and fix the class method. We do it by hand.

    @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 = target_topleft
        self.shot_position = shot_position
        self.shot_mask = shot_mask
        self.explosion_mask = explosion_mask

Commit: reorder init and its parameters.

Now I find that I am tired, so let’s sum up this second half of our two-article push.

Summary

This most recent change, with the new class method, just breaks out the translation from target and shot flyer objects into the objects that the ImageMasher really cares about, namely masks and positions. Let me rename a bit to make that more clear:

    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

This would be more symmetric if we could deal either with two toplefts or two centers. We’ll probably explore that later: I think it’s possible, and might be an improvement.

Overall, what we’ve done today is to reduce the two rather different-appearing cases, erasing the shot and the explosion, first to looking similar, then to looking identical, then to calling the same method, providing only a single parameter, the single difference between the cases.

I confess that I like that a lot, and I’m pretty sure it isn’t even a darling that needs to be, um, eliminated.

chrome squirrel

Chrome Squirrel! Most distracting object known to man or dog.

I just had the following little thought. It might be a useful idea toward improving things. This object’s top-level behavior is a bit like this:

new_mask = old_mask.copy()
new_mask += changes_due_to(shot)
new_mask += changes_due_to(explosion)

We just “accumulate” changes into the new mask. If we could write that, it would be pretty clear, wouldn’t it?

It would probably be going too far, especially if we actually implemented += on some object. But the idea of a MaskAccumulator has some appeal.

I’ll include all the code below just in case you want to look it over. But for today, I think we’ve improved things a bit, and that we can, if we choose, improve it further. One thing to observe in the ImageMasher as it stands is that it has these pretty clear stages:

  1. Creation;
  2. Produce new mask
  3. Fetch new mask
  4. Update Surface using new mask

Those last two steps suggest to me that there should perhaps be an object called MaskedSurface, or something of that nature. Why should we have to ask all these questions to get what we need?

13 commits this morning. My lucky number!

See you next time!

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

    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)