I found a way to damage a Surface, given a Mask. I don’t exactly like it, but it works. Now what?
If there is a built-in way for pygame to mask a surface given a mask, I could not find it. So I did it the old-fashioned way, copying bits. My surfaces have
colorkey set to all zeros: they are transparent where they are black. So this does the job:
class ImageMasher: def apply_damage_to_surface(self, surface): new_mask = self.get_new_mask() for x in range(new_mask.get_rect().width): for y in range(new_mask.get_rect().height): bit = new_mask.get_at((x, y)) if not bit: surface.set_at((x, y), (0, 0, 0))
If there’s no bit in the mask, it’s a hole. If its a hole, we set the corresponding pixel in the image to zero and now we have what we wanted.
Attentive readers may have noticed that the shields and players are now also green. That’s because I changed the BitmapMaker to allow for color to be set in the surfaces it makes:
class BitmapMaker: ... self.shield = self.make_and_scale_surface(shield, scale, (22, 16), "green") ... def make_and_scale_surface(self, pixel_bytes, scale, size=(16, 8), color="white"): return pygame.transform.scale_by(self.make_surface(pixel_bytes, size, color), scale) def make_surface(self, pixel_bytes, size, color): s = Surface(size) s.set_colorkey((0, 0, 0)) width = size layers = len(pixel_bytes) // width for x, byte in enumerate(pixel_bytes): x_in = x // layers y_offset = 0 if layers == 1 else 8 - (x % layers)*8 self.store_byte(byte, x_in, y_offset, s, color) return s @staticmethod def store_byte(pixel_byte, x, y_offset, surface, color): for z in range(8): bit = pixel_byte & 1 y = y_offset + 7 - z if bit: surface.set_at((x, y), color) # here it is! pixel_byte = pixel_byte >> 1
I had to pass the color all the way down into
store_byte, but that was just a matter of moments. So now we have green shields and players.
Have I mentioned that the original Invaders screen was not a color display? It actually had a transparent sheet of plastic over it with red up near the top and green down near the bottom. Very clever, these Invaders.
There’s one thing left to do, and that’s to destroy the invader shot when it hits the bottom line. Easy-peasy:
class InvaderShot(InvadersFlyer): def interact_with_bottomline(self, line, fleets): self.die_on_collision(line, fleets)
That right there is what I really like about this design. Changes like that one are so easy.
Green and game is good. Commit: invader shots die if they hit the bottom line.
The BottomLine went in fairly smoothly, except that the ImageMasher turned the line to white. I let that go yesterday, as I wanted to research pygame. I couldn’t find a good solution, so I went with the direct one. What actually happened was that first I put the bit-copying code into BottomLine, and when it worked, moved it over to ImageMasher. Then I refactored to put color into the basic surfaces. That brought us to where we started this morning.
It all went quite easily.
But the code. We’ll look at BottomLine in a moment, and we’ll see that it is a bit ragged, and that it is quite similar to the code for Shield. Something may want to be done about those matters.
Let’s dive in.
class BottomLine(InvadersFlyer): def __init__(self): w = 960-128 h = 4 surface = Surface((w, h)) surface.set_colorkey((0, 0, 0)) surface.fill("green") rect = Rect(0, 0, w, h) rect.bottomleft = (64, u.SCREEN_SIZE - 56) self.position = rect.center self._map = surface self._mask = pygame.mask.from_surface(surface) self._rect = rect self._tasks = Tasks() @property def mask(self): return self._mask @property def rect(self): return self._rect def draw(self, screen): screen.blit(self._map, self.rect) def begin_interactions(self, fleets): self._tasks.clear() def end_interactions(self, fleets): self._tasks.finish() def interact_with(self, other, fleets): other.interact_with_bottomline(self, fleets) def interact_with_invadershot(self, shot, fleets): self.process_shot_collision(shot) def process_shot_collision(self, shot): collider = Collider(self, shot) if collider.colliding(): self._tasks.remind_me(lambda: self.mash_image(shot)) def mash_image(self, shot): masher = ImageMasher.from_flyers(self, shot) masher.determine_damage() self._mask = masher.get_new_mask() masher.apply_damage_to_surface(self._map)
That’s pretty much copied and pasted from Shield. Let’s see what we can do here. We only have one use of
process_shot_collision, and could inline it. Or we could inline inside that method:
def interact_with_invadershot(self, shot, fleets): self.process_shot_collision(shot) def process_shot_collision(self, shot): if Collider(self, shot).colliding(): self._tasks.remind_me(lambda: self.mash_image(shot))
I think that’s OK. I think further inlining, into
interact_with_invadeershot, is not desirable. Why? Because as it stands,
interactwith_invadershot says what will be done, and
process_shot_collision says how it is done. And especially since it files that lambda, I think it’s carrying an important bit of meaning.
def mash_image(self, shot): masher = ImageMasher.from_flyers(self, shot) masher.determine_damage() self._mask = masher.get_new_mask() masher.apply_damage_to_surface(self._map)
What we have here is three messages to the masher. That, all my foes and all my friends, is what we in the trade call feature envy. Masher really ought to be helping us out here. But what do we want it to do for us? We want it to determine the damage, apply it to our surface, and return a new mask.
As I review the code, a strange thought comes to me. It is possible that the original mask is being damaged in place. If that is true, then this code should work:
def mash_image(self, shot): masher = ImageMasher.from_flyers(self, shot) masher.determine_damage() # self._mask = masher.get_new_mask() masher.apply_damage_to_surface(self._map)
If we need to fetch the new mask, then accumulated damage would not occur if we comment that line out. But, by actual test in Shield, it does occur. Therefore … ImageMasker and Masker, its helper object, are in fact damaging the input masks in place.
Honestly, I think that’s bad. It’s true that in our case we do want the updated masks, but in general mutating things that are passed down a couple of levels is just not a good idea.
Changing Masker to copy the mask:
class Masker: def __init__(self, mask, position): self.mask = mask.copy() # don't mutate your input self.rect = mask.get_rect() self.rect.center = position
Now the damage does not accumulate if I don’t read out the new mask. I think that’s better.
Now, again, what are we to call the method in Masher. Note that it does mutate the surface. I think that may be better than creating a new one every time, although if I’m going to protect the one, you could argue that I should protect the other.
The difference is that the object itself is asking to have its surface mashed.
It would certainly be possible for ImageMasher to determine the damage right out of the box. We do have a test that calls it, but …
No, I think that’s irrelevant to our question about what the method is that we want ImageMasher to provide for us. It’s just not cohesive. It’s something like
Let’s call it
update and have it return the new mask, after updating the surface in place.
class ImageMasher: def update(self, surface): self.determine_damage() self.apply_damage_to_surface(surface) return self.get_new_mask()
def mash_image(self, shot): masher = ImageMasher.from_flyers(self, shot) self._mask = masher.update(self._map)
Works perfectly. Green. Commit: remove feature envy from Shield and BottomLine, move to ImageMasher. Slightly odd.
Why do I say it’s slightly odd? Because the update method updates one thing and returns another. That’s odd.
The Masher is only used twice and isn’t likely to be used again, so from a pragmatic point of view, we can leave it alone. And it’s really perfectly clear how to use it, from the existing usages. (There are no really great tests, as far as I know.)
Hmm. One thing that would look less strange would be this:
def mash_image(self, shot): masher = ImageMasher.from_flyers(self, shot) self._mask, self._map = masher.update(self._mask, self._map)
Now it looks as if masher updates and returns both objects. Let’s do that.
def update(self, mask, surface): self.determine_damage() self.apply_damage_to_surface(surface) return self.get_new_mask(), surface
Now it looks like we are accepting the two parameters and producing new ones. It’s not quite what’s happening, but the code makes sense in use. I think this is preferable. Commit: update accepts mask and surface, returns mask and surface.
Things are a bit strange inside ImageMasher and Masker. They are fit for the current purpose, but not entirely safe for general use, in that you could find your old surface mashed. Of course, if you call update, presumably you do intend to use the surface that comes back, and the fact that it is your old one, modified, is probably of no concern.
And while we have no pragmatic reason to improve those two classes, we may find it rewarding to refactor them at some future time, because we’re here to see how to change code.
The BottomLine is still a bit ragged. We should review it at least one more time. Lots of magic numbers.
We set out to draw the bottom line and damage it. We managed that readily, and encountered the fact that objects down there were supposed to be green. It was straightforward to change a few signatures to allow the bitmaps to define a desired color. We noticed the duplicated code between BottomLine and Shield, and that led us to observe the feature envy in that duplication. Removing that reduces the duplication as well, though they do both have a bit of similarity in how they process collisions and update their bitmaps and masks. It is possible that a common object to which to delegate that responsibility could reduce the duplication, but it is so small that I am not feeling much pressure to do it.
- What’s left?
- Well, when invaders get to the bottom of the screen, they are supposed to chew away at the shields. And when they collide with the player, the player should be destroyed. I’m not sure if that’s game over, or you just lose one player. And I suppose you should get a new rack of invaders at that point, not get rezzed with an invader right on top of you.
Speaking of a new large rack of invaders, when you shoot them all down (it could happen), you don’t get a new rack. And the later racks, I happen to know, start lower down. We should work on that.
Finally, I think the starting position for the player may be too high. So there’s some tuning to be done.
Even more finally, there is the question of making a two-player game. I’m not sure if we’ll do that or not.
We’ll find out in the future. See you there!