Python 047 - Scoring?
In the unlikely event that I shoot down the saucer, I’m supposed to get points. Let’s make that happen.
Let’s review how we score points on asteroids, and see how that might apply to the saucer. We’d better write tests for this: I’m not very good at playing the game and I have a limited supply of quarters allocated for testing.
PyCharm happens to be open on the code I need:
def mutual_destruction(self, target, targets, attacker, attackers):
if self.within_range(target, attacker):
self.score += target.score_against(attacker)
self.score += attacker.score_against(target)
attacker.destroyed_by(target, attackers)
target.destroyed_by(attacker, targets)
return True
else:
return False
We use the True/False return to break the collision checking loop. The important thing for our purposes this morning is the score_against method. Looking at this code, I’m honestly not sure what score_against means. We’ll have to look at the implementations anyway but that name may need improvement: do we get a score when it’s missile.score_against(asteroid), or the reverse?
The answer is immediately clear:
class Asteroid:
def score_against(self, attacker):
return attacker.score_list[self.size]
The implementations in missile, ship, and saucer are all the same:
def score_against(self, _):
return 0
So the sentence in the code should read asteroid.score_when_killed_by(missile), it seems to me. And we’ll find that only the missile has an interesting score_list:
class Missile:
def __init__(self, position, velocity, score_list):
self.score_list = score_list
self.position = position.copy()
self.velocity = velocity.copy()
self.radius = 2
self.time = 0
@classmethod
def from_ship(cls, position, velocity):
return cls(position, velocity, u.MISSILE_SCORE_LIST)
@classmethod
def from_saucer(cls, position, velocity):
return cls(position, velocity, [0, 0, 0])
So. To do this, we’ll want the saucer to implement score_against, or whatever we rename it to, and to ask the attacker for a saucer_score_list, and we want all the objects other than missile to return a list of [0, 0], and the missile to have a list of [200, 1000] or [0, 0], depending on whether it is a ship missile or a saucer missile.
There’s no real chance that the saucer could shoot itself down, but we should probably provide for the possibility just the same. The new collision code does check the saucer against its own missiles: remember that we’re doing all combinations now.
OK, let’s have a test or two. To my surprise we have a test for asteroid-missile:
def test_missile_asteroid_scores(self):
pos = Vector2(100, 100)
asteroid = Asteroid(2, pos)
asteroids = [asteroid]
missile = Missile.from_ship(pos, Vector2(0, 0))
missiles = [missile]
collider = Collider(asteroids, missiles, [], [], [])
collider.mutual_destruction(asteroid, asteroids, missile, missiles)
assert not missiles
assert collider.score == 20
assert len(asteroids) == 2
In fact we have more:
def test_missile_ship_does_not_score(self):
def test_asteroid_ship_does_not_score(self):
def test_asteroid_saucer_does_not_score(self):
Let’s have two more, saucer vs ship missile, and saucer vs saucer missile.
def test_saucer_ship_missile_scores(self):
pos = Vector2(100, 100)
saucer = Saucer(pos)
saucers = [saucer]
missile = Missile.from_ship(pos, Vector2(0, 0))
missiles = [missile]
collider = Collider([], missiles, saucers, [], [])
collider.mutual_destruction(saucer, saucers, missile, missiles)
assert not missiles
assert not saucers
assert collider.score == 200
I think that’s righteous. And it’s failing on the 200, as expected. So in Saucer:
def score_against(self, attacker):
return attacker.saucer_score_list[self.size - 1]
Now we need that method in all the space objects (except saucer, I suppose). And since saucer size is 1 or 2, the list should be [1000, 200], not the reverse, in missile.
class Missile:
def __init__(self, position, velocity, score_list):
self.score_list = score_list
self.saucer_score_list = [1000, 200]
self.position = position.copy()
self.velocity = velocity.copy()
self.radius = 2
self.time = 0
That makes the test run. Another is broken because the other objects don’t have this list. This test is failing:
def test_asteroid_saucer_does_not_score(self):
game = Game(True)
pos = Vector2(100, 100)
asteroid = Asteroid(2, pos)
asteroids = [asteroid]
saucer = Saucer(pos)
saucers = [saucer]
collider = Collider(asteroids, [], saucers, [], [])
collider.mutual_destruction(asteroid, asteroids, saucer, saucers)
assert not saucers
assert game.score == 0
The error is:
self = <saucer.Saucer object at 0x106037390>
attacker = <asteroid.Asteroid object at 0x1060372d0>
def score_against(self, attacker):
> return attacker.saucer_score_list[self.size - 1]
E AttributeError: 'Asteroid' object has no attribute 'saucer_score_list'
However, when I paste in this:
class Asteroid:
def __init__(self, size=2, position=None):
self.size = size
if self.size not in [0,1,2]:
self.size = 2
self.radius = [16, 32, 64][self.size]
self.position = position if position is not None else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
self.velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self.offset = Vector2(self.radius, self.radius)
self.surface = SurfaceMaker.asteroid_surface(self.radius * 2)
self.saucer_score_list = [1000, 200]
The test passes! I would have expected that I would have to put in [0, 0], lest I get a large score in that test.
Ah! It’s checking game. But we’re not calling Game, it won’t have the score yet. Change the test to be correct:
def test_asteroid_saucer_does_not_score(self):
pos = Vector2(100, 100)
asteroid = Asteroid(2, pos)
asteroids = [asteroid]
saucer = Saucer(pos)
saucers = [saucer]
collider = Collider(asteroids, [], saucers, [], [])
collider.mutual_destruction(asteroid, asteroids, saucer, saucers)
assert not saucers
assert collider.score == 0
Now it fails with score of 200. Then I change to this:
class Asteroid:
def __init__(self, size=2, position=None):
self.size = size
if self.size not in [0,1,2]:
self.size = 2
self.radius = [16, 32, 64][self.size]
self.position = position if position is not None else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
self.velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self.offset = Vector2(self.radius, self.radius)
self.surface = SurfaceMaker.asteroid_surface(self.radius * 2)
self.saucer_score_list = [0, 0]
And the test passes. What happened—I think—was that when the Collider went in, I modified the tests to use the collider, and missed fixing that one. I’ll double check the others.
We’re not done yet but we are green. Let’s commit: Ship missiles score 200 on small saucer, 1000 on large. (Latter untested.)
We need the test for 1000 and to ensure that saucer missiles don’t score against the saucer.
def test_saucer_vs_saucer_missile_does_not_score(self):
pos = Vector2(100, 100)
saucer = Saucer(pos)
saucers = [saucer]
missile = Missile.from_saucer(pos, Vector2(0, 0))
missiles = [missile]
collider = Collider([], missiles, saucers, [], [])
collider.mutual_destruction(saucer, saucers, missile, missiles)
assert not missiles
assert not saucers
assert collider.score == 0
This fails, giving us the opportunity to fix up creation of the missile. It starts like this:
class Missile:
def __init__(self, position, velocity, score_list):
self.score_list = score_list
self.saucer_score_list = [1000, 200]
self.position = position.copy()
self.velocity = velocity.copy()
self.radius = 2
self.time = 0
@classmethod
def from_ship(cls, position, velocity):
return cls(position, velocity, u.MISSILE_SCORE_LIST)
@classmethod
def from_saucer(cls, position, velocity):
return cls(position, velocity, [0, 0, 0])
We should pass both collections from the constructors:
class Missile:
def __init__(self, position, velocity, missile_score_list, saucer_score_list):
self.score_list = missile_score_list
self.saucer_score_list = saucer_score_list
self.position = position.copy()
self.velocity = velocity.copy()
self.radius = 2
self.time = 0
@classmethod
def from_ship(cls, position, velocity):
return cls(position, velocity, u.MISSILE_SCORE_LIST, [1000, 200])
@classmethod
def from_saucer(cls, position, velocity):
return cls(position, velocity, [0, 0, 0], [0, 0])
We are green. Now the saucer score list should be put into u.
@classmethod
def from_ship(cls, position, velocity):
return cls(position, velocity, u.MISSILE_SCORE_LIST, u.SAUCER_SCORE_LIST)
And of course I put the list into u.
SAUCER_SCORE_LIST = [1000, 200]
I am somewhat surprised that I don’t seem to need those lists in other objects and I’d like to understand why. It seems to me that everything destroys everything, so that when … ah. I’m missing a test: ship vs saucer!
def test_saucer_ship_does_not_score(self):
pos = Vector2(100, 100)
saucer = Saucer(pos)
saucers = [saucer]
ship = Ship(pos)
ships = [ship]
collider = Collider([], [], saucers, [], ships)
collider.mutual_destruction(saucer, saucers, ship, ships)
assert not ships
assert collider.score == 0
This fails, as I had hoped:
> return attacker.saucer_score_list[self.size - 1]
E AttributeError: 'Ship' object has no attribute 'saucer_score_list'
Perfect.
class Ship:
def __init__(self, position):
self.score_list = [0, 0, 0]
self.saucer_score_list = [0, 0]
self.position = position.copy()
self.velocity = Vector2(0, 0)
self.can_fire = True
self.radius = 25
self.angle = 0
self.acceleration = u.SHIP_ACCELERATION
self.accelerating = False
ship_scale = 4
ship_size = Vector2(14, 8)*ship_scale
self.ship_surface, self.ship_accelerating_surface = SurfaceMaker.ship_surfaces(ship_size)
Green. Past time to commit: enhanced tests, ship vs saucer does not score.
Reflect
I want to think about this. Asteroids score, so everything that isn’t an asteroid must answer score_list. I draw a little table to help me think:
| object | asteroid_score_list | saucer_score_list |
|---|---|---|
| asteroid | - | 0 |
| missile | 20, 50, 100 | 1000, 200 |
| saucer | 0 | - |
| ship | 0 | 0 |
What I really want to do is to rename some things. But let me check the objects first.
And you know what? It’s really not legitimate to be pulling these values out as members. We should be calling functions. Let’s work toward that as well.
And … I think the “interface” should be that all the objects respond to both messages.
I’ll write a test for that. At first a rudimentary one to force the methods:
def test_everyone_supports_score_lists(self):
asteroid = Asteroid()
missile = Missile.from_ship(Vector2(100, 100), Vector2(100, 100))
saucer = Saucer()
ship = Ship(Vector2(200, 200))
assert asteroid.get_asteroid_scores()
assert missile.get_asteroid_scores()
assert saucer.get_asteroid_scores()
assert ship.get_asteroid_scores()
assert asteroid.get_saucer_scores()
assert missile.get_saucer_scores()
assert saucer.get_saucer_scores()
assert ship.get_saucer_scores()
This of course fails perfectly.
It’s also tedious to change. First, I’ll split it.
def test_everyone_supports_asteroid_score_lists(self):
asteroid = Asteroid()
missile = Missile.from_ship(Vector2(100, 100), Vector2(100, 100))
saucer = Saucer()
ship = Ship(Vector2(200, 200))
assert asteroid.get_asteroid_scores()
assert missile.get_asteroid_scores()
assert saucer.get_asteroid_scores()
assert ship.get_asteroid_scores()
def test_everyone_supports_saucer_score_lists(self):
asteroid = Asteroid()
missile = Missile.from_ship(Vector2(100, 100), Vector2(100, 100))
saucer = Saucer()
ship = Ship(Vector2(200, 200))
assert asteroid.get_saucer_scores()
assert missile.get_saucer_scores()
assert saucer.get_saucer_scores()
assert ship.get_saucer_scores()
I’ve been putting the method get_asteriod_scores in everywhere. Other tests are failing, probably fetching score lists directly?
I tick through them all, adding the new methods. Now it turns out that only Missile has any values in those list:
class Asteroid:
def get_asteroid_scores(self):
return [0, 0, 0]
def get_saucer_scores(self):
return [0, 0]
class Missile:
def get_asteroid_scores(self):
return self.score_list
def get_saucer_scores(self):
return self.saucer_score_list
class Saucer:
def get_asteroid_scores(self):
return [0, 0, 0]
def get_saucer_scores(self):
return [0, 0]
class Ship:
def get_asteroid_scores(self):
return [0, 0, 0]
def get_saucer_scores(self):
return [0, 0]
That’s fine. I’d rather have the duplication than leave gaps in the protocol. In principle every object needs to provide this information, even though, as the game is now defined, the answers are mostly zero. We could imagine something different. If the saucer hits an asteroid or shoots down the ship, it could score negative points. It just happens that it doesn’t.
We are green. Commit: saucer scoring complete, refactoring, extensive cross-check testing.
I’m still not happy with the naming.
What if we renamed get_asteroid_scores to scores_for_hitting_asteroid? Would that be better?
And what if we renamed score_against to score_for_hitting?
That would look like this:
def mutual_destruction(self, target, targets, attacker, attackers):
if self.within_range(target, attacker):
self.score += target.score_for_hitting(attacker)
self.score += attacker.score_for_hitting(target)
attacker.destroyed_by(target, attackers)
target.destroyed_by(attacker, targets)
return True
else:
return False
I try renaming, but PYCharm can’t totally cut the mustard. I have tests failing. I’ll have to rename in all the individual classes. I do so and get green. Commit: rename score_against to score_for_hitting.
Now the list method names? I rename them to scores_for_hitting_asteroid and scores_for_hitting_saucer with global replace. Tests are green. Commit: rename get_scores methods to scores_for_hitting.
I haven’t run the game since early in the article. The tests are really giving me confidence. To close out the morning, I do run it and it is just fine. I even manage to shoot down the saucer a couple of times.
Let’s sum up.
Summary
Getting the scoring in place was simple enough. I spent most of my time adding tests. There was just one real surprise, when I found that test that had not been edited correctly and wasn’t testing anything. I only noticed because I expected it to fail when I pasted the wrong values (on purpose), and it didn’t fail.
I decided to change from accessing a member variable to get the score lists, to calling a method. I think in Python it’s considered OK to access someone else’s member variable, but it seems off to me. There is a convention in Python to prefix private members with underbar. Perhaps I should be doing that throughout.
There are other direct accesses, at least in the tests, and perhaps elsewhere, checking position and value. For example:
class Collider
def within_range(target, attacker):
in_range = target.radius + attacker.radius
dist = target.position.distance_to(attacker.position)
return dist <= in_range
I think I’d argue that that is righteous in Python terms, because radius, position, and velocity are required members on any space object, but I don’t think that argument holds a lot of water. In Smalltalk we’d say that the accessor methods are required: you can’t ever see another object’s members. Very Victorian, is Smalltalk.
We’ll let it ride for now, as it is coming up on time for a code review anyway.
For now, we’ve improved the product with Saucer scoring, and we’ve improved the code with a bit of naming as well. Oh, but we do owe ourselves a test for scoring a small saucer.
def test_small_saucer_ship_missile_scores(self):
pos = Vector2(100, 100)
saucer = Saucer(pos, 1)
saucers = [saucer]
missile = Missile.from_ship(pos, Vector2(0, 0))
missiles = [missile]
collider = Collider([], missiles, saucers, [], [])
collider.mutual_destruction(saucer, saucers, missile, missiles)
assert not missiles
assert not saucers
assert collider.score == 1000
It runs. Commit: test small saucer scoring.
I think it’s a good morning. See you next time!