Python Asteroids+Invaders on GitHub

Let’s clean up the shields code and improve the damage. I’ve found a missing piece, the explosion part. A rather nice refactoring sequence ensues.

We’ve done an outstanding job of slicing the Shields Story into thin slices. Here’s where we stand now, sorted just a bit from last time to keep undone bits together:

  1. Get one shield on the screen;
  2. Get all four on the screen;
  3. Shots ignore shields;
  4. InvaderShots die upon hitting shields;
  5. PlayerShots die upon hitting shields;
  6. InvaderShots do simple damage;
  7. InvaderShots do fancy damage;
  8. PlayerShots do simple damage;
  9. PlayerShots do fancy damage;
  10. Display invader shot explosion at shield?
  11. If explosion doesn’t look great, flash it?
  12. Should there be explosions when shot hits shot?

I checked the ancient scrolls (original game videos) and it does appear that when a shot hits the shield, the game displays the shot explosion and then erases it from the shield. I think I’ll save that story, because the player shots should follow along with how we did the invader ones, so doing them next takes advantage of any ancestral memory that I might have.

I think we’ll just follow the pattern and see where it leads. We have:

    def interact_with_invadershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            self._mask_copy.erase(self._explosion_mask, collider.offset())
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

I’ll replicate that without the explosion mask, which we don’t have yet:

    def interact_with_playershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            # self._mask_copy.erase(self._explosion_mask, collider.offset())
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

I think that’s gonna leave a mark. Sure enough, it does:

shooting the shield leaves a mark

It remains to get the explosion and erase it.

Aside
It also remains to ask, where the heck are the tests for this behavior? I could remove a lot of this code and the tests would still run, the game would still run, and the display would be wrong. I tend to cut myself slack on tests for visible things but … can we not do better?
class Shield(InvadersFlyer):
    def __init__(self, position):
        map = BitmapMaker.instance().shield
        explo = BitmapMaker.instance().invader_shot_explosion
        self._explosion_mask = pygame.mask.from_surface(explo)
        player_explo = BitmapMaker.instance().player_shot_explosion
        self._player_explosion_mask = pygame.mask.from_surface(player_explo)
        self._map = map.copy()
        self._map.set_colorkey("black")
        self._mask = pygame.mask.from_surface(map)
        self._mask_copy = self.mask.copy()
        self._rect = self._map.get_rect()
        self._rect.center = position

    def interact_with_playershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            self._mask_copy.erase(self._player_explosion_mask, collider.offset())
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

This doesn’t look good. The player shot is just a line and the explosion is wide, so it is showing up oddly in the display.

player damage offset

Perhaps you can see how there is a vertical cut upward and then the explosion off to the right of it. The third shield shows it nicely. I think we’ll just nudge the explosion to the left a bit and maybe down.

    def interact_with_playershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = self._player_explosion_mask.get_rect()
            self._mask_copy.erase(self._player_explosion_mask, collider.offset() + Vector2(-e_rect.width/2, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

That looks pretty good:

damage looks centered

Let’s commit: Player shot does visible damage to shields. Not recommended to shoot your own shields.

The code is similar but not the same. We’d like to remove duplication.

    def interact_with_invadershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            self._mask_copy.erase(self._explosion_mask, collider.offset())
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

    def interact_with_playershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = self._player_explosion_mask.get_rect()
            self._mask_copy.erase(self._player_explosion_mask, collider.offset() + Vector2(-e_rect.width/2, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

Let’s reorder those lines for similarity and keeping related things together:

    def interact_with_invadershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            self._mask_copy.erase(self._explosion_mask, collider.offset())
            surf = self._mask_copy.to_surface()
            rect = mask.get_rect()
            self._map.blit(surf, rect)

    def interact_with_playerexplosion(self, explosion, fleets):
        pass

    def interact_with_playershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            e_rect = self._player_explosion_mask.get_rect()
            self._mask_copy.erase(self._player_explosion_mask, collider.offset() + Vector2(-e_rect.width/2, 0))
            surf = self._mask_copy.to_surface()
            rect = mask.get_rect()
            self._map.blit(surf, rect)

I wonder about that adjustment to the second erase. Why did we need it for the one case and not the other? Well, the basic reason is that the player shot is one (big) pixel wide, the explosion is 8 wide, but the invader shot is 3 wide and the explosion is 6. So odds are, the invader shot isn’t perfectly centered either.

Yes, I think it is a bit to the right. Sometimes the damage appears to kind of slide rightward from the shot.

Let us reason about this. With a difference of 1:8, we moved over 4 pixels, 8 / 2. We could have moved 5: either value would nearly center the explosion. We can’t get perfectly aligned without a fractional pixel. With a difference of 3:6, we should move over either 1 or 2 big pixels.

I think the thing we want is about the difference between the explosion size and shot size, not just the size of the explosion.

So we want 8 - 1 -> 4 or 5, and 6 - 3 -> 1 or 2. Looks to me like (explosion.w - shot.w)//2 might work.

Note:
It does sort of work but we do better below.

That’s a lot of questions about the shot but we have it in hand. After a bit of stumbling:

    def interact_with_invadershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = self._explosion_mask.get_rect()
            offset_x = (shot.rect.w - e_rect.w) // 2
            self._mask_copy.erase(self._explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

    def interact_with_playershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = self._player_explosion_mask.get_rect()
            offset_x = (shot.rect.width - e_rect.width) // 2
            self._mask_copy.erase(self._player_explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

PyCharm recognizes the duplication. I think this can be brought down to a function called twice, but I don’t quite see the refactoring yet. Let’s move the differences to the top.

A couple of Command+Option+V and some Option+Shift+up and I get this:

    def interact_with_invadershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            explosion_mask = self._explosion_mask
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.w - e_rect.w) // 2
            self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

    def interact_with_playershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            explosion_mask = self._player_explosion_mask
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.width - e_rect.width) // 2
            self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

Now these babies are seriously equal. PyCharm tells me the whole thing from collider = on down is duplicated. It doesn’t say what to do about it.

I think I’ll extract right after setting the explosion_mask temp. PyCharm doesn’t find the other duplicate for me so I do it manually:

    def interact_with_invadershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            explosion_mask = self._explosion_mask
            self.apply_shield_damage(collider, shot, explosion_mask)

    def interact_with_playershot(self, shot, fleets):
        collider = Collider(self, shot)
        if collider.colliding():
            explosion_mask = self._player_explosion_mask
            self.apply_shield_damage(collider, shot, explosion_mask)

    def apply_shield_damage(self, collider, shot, explosion_mask):
        mask: Mask = collider.overlap_mask()
        self._mask_copy.erase(mask, (0, 0))
        rect = mask.get_rect()
        e_rect = explosion_mask.get_rect()
        offset_x = (shot.rect.width - e_rect.width) // 2
        self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
        surf = self._mask_copy.to_surface()
        self._map.blit(surf, rect)

Duplication removed. Well past time for a commit: Both invader and player shots damage Shields with their individual explosion patterns.

I’m not happy with the extraction we just did, even though it’s committed. Let’s undo it and try again.

    def interact_with_invadershot(self, shot, fleets):
        explosion_mask = self._explosion_mask
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.w - e_rect.w) // 2
            self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

    def interact_with_playershot(self, shot, fleets):
        explosion_mask = self._player_explosion_mask
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.width - e_rect.width) // 2
            self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

Can I extract everything but the assignment of the mask?

    def interact_with_invadershot(self, shot, fleets):
        explosion_mask = self._explosion_mask
        self.process_shot_collision(shot, explosion_mask)

    def interact_with_playershot(self, shot, fleets):
        explosion_mask = self._player_explosion_mask
        self.process_shot_collision(shot, explosion_mask)

    def process_shot_collision(self, shot, explosion_mask):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.w - e_rect.w) // 2
            self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

Nearly good. Inline the masks:

    def interact_with_invadershot(self, shot, fleets):
        self.process_shot_collision(shot, self._explosion_mask)

    def interact_with_playerexplosion(self, explosion, fleets):
        pass

    def interact_with_playershot(self, shot, fleets):
        self.process_shot_collision(shot, self._player_explosion_mask)

    def process_shot_collision(self, shot, explosion_mask):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            rect = mask.get_rect()
            e_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.w - e_rect.w) // 2
            self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, rect)

That looks better. Commit: refactor shot collisions.

Now let’s look at that process_shot_collision method and see if we can make it more clear.

A couple of renames:

    def process_shot_collision(self, shot, explosion_mask):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            shot_rect = mask.get_rect()
            expl_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.w - expl_rect.w) // 2
            self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
            surf = self._mask_copy.to_surface()
            self._map.blit(surf, shot_rect)

Extract method, heading toward Composed Method pattern:

    def process_shot_collision(self, shot, explosion_mask):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            shot_rect = mask.get_rect()
            expl_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.w - expl_rect.w) // 2
            self._mask_copy.erase(explosion_mask, collider.offset() + Vector2(offset_x, 0))
            self.erase_visible_pixels(shot_rect)

    def erase_visible_pixels(self, shot_rect):
        surf = self._mask_copy.to_surface()
        self._map.blit(surf, shot_rect)

Extract temp variable:

    def process_shot_collision(self, shot, explosion_mask):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            shot_rect = mask.get_rect()
            expl_rect = explosion_mask.get_rect()
            offset_x = (shot.rect.w - expl_rect.w) // 2
            adjust_image_to_center = collider.offset() + Vector2(offset_x, 0)
            self._mask_copy.erase(explosion_mask, adjust_image_to_center)
            self.erase_visible_pixels(shot_rect)
Note:
I’m just following my nose here, not really trying to think it out. The machine refactorings make sure I won’t make a mistake, and if I don’t like the result, I’ll back out and do again.
    def process_shot_collision(self, shot, explosion_mask):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            shot_rect = mask.get_rect()
            self.erase_explosion_from_mask(collider, explosion_mask, shot)
            self.erase_visible_pixels(shot_rect)

    def erase_explosion_from_mask(self, collider, explosion_mask, shot):
        expl_rect = explosion_mask.get_rect()
        offset_x = (shot.rect.w - expl_rect.w) // 2
        adjust_image_to_center = collider.offset() + Vector2(offset_x, 0)
        self._mask_copy.erase(explosion_mask, adjust_image_to_center)

    def erase_visible_pixels(self, shot_rect):
        surf = self._mask_copy.to_surface()
        self._map.blit(surf, shot_rect)

Hm. Inline shot_rect:

    def process_shot_collision(self, shot, explosion_mask):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self._mask_copy.erase(mask, (0, 0))
            self.erase_explosion_from_mask(collider, explosion_mask, shot)
            self.erase_visible_pixels(mask.get_rect())

We could extract the one-liner to give it a name. How do you feel about that? Or maybe this?

    def process_shot_collision(self, shot, explosion_mask):
        collider = Collider(self, shot)
        if collider.colliding():
            mask: Mask = collider.overlap_mask()
            self.erase_shot_and_explosion_from_mask(shot, collider, mask, explosion_mask)
            self.erase_visible_pixels(mask.get_rect())

    def erase_shot_and_explosion_from_mask(self, shot, collider, shot_overlap_mask, explosion_mask):
        self._mask_copy.erase(shot_overlap_mask, (0, 0))
        self.erase_explosion_from_mask(collider, explosion_mask, shot)

    def erase_explosion_from_mask(self, collider, explosion_mask, shot):
        expl_rect = explosion_mask.get_rect()
        offset_x = (shot.rect.w - expl_rect.w) // 2
        adjust_image_to_center = collider.offset() + Vector2(offset_x, 0)
        self._mask_copy.erase(explosion_mask, adjust_image_to_center)

    def erase_visible_pixels(self, shot_rect):
        surf = self._mask_copy.to_surface()
        self._map.blit(surf, shot_rect)

Yes. The erase_shot_and_explosion_from_mask, plus the new name on its parameter, seems to me to tell the tale fairly well.

Commit: refactoring process_shot_collision. Let’s sum up.

Summary

There is still at least one thing that I don’t like. The invader shots don’t seem to dig down into the shields as much as one might like. We might want to increase the damage somehow.

The code is intricate, because it needs to update both the visible bits and the mask, separately for each shield, and because it impacts pixels with the shot itself and with the shot’s explosion. I wouldn’t argue that the current refactoring is perfect, or even excellent, but it does at least give you an idea about what each component is and a place to think about “OK, how is this doing erase_explosion_from_mask?”.

I noticed one other thing. Sometimes when shots collide, there is a red explosion and sometimes there isn’t, but the shots do disappear. I’ll add that story to the list.

Let’s look at our stories:

  1. Get one shield on the screen;
  2. Get all four on the screen;
  3. Shots ignore shields;
  4. InvaderShots die upon hitting shields;
  5. PlayerShots die upon hitting shields;
  6. InvaderShots do simple damage;
  7. InvaderShots do fancy damage;
  8. PlayerShots do simple damage;
  9. PlayerShots do fancy damage;
  10. Display invader shot explosion at shield?
  11. If explosion doesn’t look great, flash it?
  12. Should there be explosions when shot hits shot?
  13. Shouldn’t we have at least SOME tests for all this?
  14. Sometimes shot vs shot makes a red shot explosion, sometimes not. Whazzup?

It’s definitely a “whazzup”, too. It really seems to be happening. I think I’m seeing:

  1. Player shots can kill invader shots and then continue on up the screen, often then killing an invader. I suspect this could be a mask-related thing.
  2. Sometimes a red explosion appears and sometimes not.

Yes, I think I see it! When the InvaderShot is hit by a PlayerShot, it sets its position to ShotController.available. If the PlayerShot is intersected after the InvaderShot, the InvaderShot’s position will be changed and the InvaderShot will not know it should explode.

Interesting. Second timing bug. Must think about this.

Overall, though, we have some really nice damage happening, and it has all gone swimmingly. A good sequence of small steps, in my opinion, and I hope, in yours. Questions welcome on mastodon, of course.

For now, I’m done. See you next time!