Python 57 - Simplify, then add lightness
Colin Chapman’s motto at Lotus might apply here. Let’s see if we can simplify this code. That will help us add lightness, I think.
My memory has a couple of code spots in mind, let’s see if they actually exist in the code.
Here’s one chunk:
class Game:
def asteroids_tick(self, delta_time):
self.check_saucer_spawn(self.saucer, self.saucers, delta_time)
self.check_ship_spawn(self.ship, self.ships, delta_time)
self.check_next_wave(delta_time)
self.check_missile_timeout(self.delta_time)
self.control_ship(self.ship, delta_time)
self.move_everything(delta_time)
self.process_collisions()
self.draw_everything()
if self.game_over: self.draw_game_over()
def check_missile_timeout(self, delta_time):
for missile in self.missiles.copy():
missile.update(self.missiles, delta_time)
for missile in self.saucer_missiles.copy():
missile.update(self.saucer_missiles, delta_time)
def move_everything(self, delta_time):
for the_saucer in self.saucers.copy():
the_saucer.move(delta_time, self.saucers, self.saucer_missiles, self.ships)
for missile in self.saucer_missiles:
missile.move(delta_time)
for the_ship in self.ships:
the_ship.move(delta_time)
for asteroid in self.asteroids:
asteroid.move(delta_time)
for missile in self.missiles:
missile.move(delta_time)
And here’s another:
class Collider:
def __init__(self, asteroids, missiles, saucers, saucer_missiles, ships):
self.movers = [asteroids, missiles, saucers, saucer_missiles, ships]
self.score = 0
def check_collisions(self):
for pair in itertools.combinations(self.movers, 2):
self.check_individual_collisions(pair[0], pair[1])
return self.score
With one small exception, the saucer, it appears that we could simplify and lighten move_everything
if we had a collection like movers
in Collider and just iterated over it.
And there’s similarity to be seen in missile_timeout
, where we call update
on two of our collections, passing delta_time
and the collection.
Suppose Game were to create the movers
collection once and for all. It should probably be a sequence, not a list, as there is no need to change it. Game could pass the sequence to Collider, which would then not need to create it, just use it.
Suppose that we were to call update
for all the space objects, not just the missiles, and suppose we always passed the object’s own collection, and included the other collections that saucer.update
needs in all the calls. (Or, possibly, find some more clever way to pass those collections just so saucer.)
Suppose that update
meant “do whatever else you need to do, and move”.
Things would be, I believe, simpler, and lighter.
Another possibility would be to leave move
for moving, and update
for updating, with perhaps a special call for the saucer with all its gimmickry.
Yet another possibility comes to mind. Suppose the sequence containing all the collections was “smart”, smart enough to answer asteroids
, returning the asteroids collection, and so on. Then we could pass the master sequence to every object, and since every object knows what it is, it could fetch and update its own collection if need be.
Interesting. Let’s start by seeing if we can simplify this method:
def move_everything(self, delta_time):
for the_saucer in self.saucers.copy():
the_saucer.move(delta_time, self.saucers, self.saucer_missiles, self.ships)
for missile in self.saucer_missiles:
missile.move(delta_time)
for the_ship in self.ships:
the_ship.move(delta_time)
for asteroid in self.asteroids:
asteroid.move(delta_time)
for missile in self.missiles:
missile.move(delta_time)
Why is saucer
so different?
class Saucer:
def move(self, delta_time, saucers, saucer_missiles, ships):
self.fire_if_possible(delta_time, saucer_missiles, ships)
self.check_zigzag(delta_time)
self.position += delta_time * self.velocity
x = self.position.x
if x < 0 or x > u.SCREEN_SIZE:
if self in saucers:
saucers.remove(self)
One reason is that it wants the saucers
collection in case it wants to remove itself. It wants the other two collections in case it wants to fire a missile.
Let’s have a new rule, that moving is about moving, not shooting, and that moving can involve you removing yourself if you feel the need. Compare the code above to that in Missile:
class Missile:
def __init__(self, position, velocity, missile_score_list, saucer_score_list):
...
self.timer = Timer(u.MISSILE_LIFETIME, self.timeout)
def move(self, deltaTime):
position = self.position + self.velocity * deltaTime
position.x = position.x % u.SCREEN_SIZE
position.y = position.y % u.SCREEN_SIZE
self.position = position
def update(self, missiles, delta_time):
self.timer.tick(delta_time, missiles)
def timeout(self, missiles):
missiles.remove(self)
Suppose we say that we’ll pass the object its own collection on move
, and that it will tick its timer or do whatever other things it wants to do if it wants to remove itself. Then we could just pass delta_time
and the collection to all the moves. That would remove the need for us to send an update message to missiles, but if we only send the one collection to the saucer, we’d lose firing the missiles.
Probably could be made to work. How about plan, what are we up to, about Plan G?
What if we make the smart sequence of all the collections, known by name, and provide that to a standard update
method, which can then do whatever it wants, including that it will call move
. The move
methods won’t have to change so long as the object’s own update
does the right thing. And then we can add lightness to the individual update
methods individually.
Let’s TDD a smart sequence thing.
class TestSpaceObjectsCollection:
def test_creation(self):
asteroids = ["asteroid"]
missiles = ["missile"]
saucers = ["saucer"]
saucer_missiles = ["saucer_missile"]
ships = ["ship"]
space_objects = SpaceObjects(asteroids, missiles, saucers, saucer_missiles, ships)
That demands a class.
class SpaceObjects:
def __init__(self, asteroids, missiles, saucers, saucer_missiles, ships):
self.collections = (asteroids, missiles, saucers, saucer_missiles, ships)
Test is green. Commit: initial SpaceObjects class.
Now I want accessors:
def test_access(self):
asteroids = ["asteroid"]
missiles = ["missile"]
saucers = ["saucer"]
saucer_missiles = ["saucer_missile"]
ships = ["ship"]
space_objects = SpaceObjects(asteroids, missiles, saucers, saucer_missiles, ships)
assert space_objects.asteroids() == asteroids
assert space_objects.missiles() == missiles
assert space_objects.saucers() == saucers
assert space_objects.saucer_missiles() == saucer_missiles
assert space_objects.ships() == ships
And:
class SpaceObjects:
def __init__(self, asteroids, missiles, saucers, saucer_missiles, ships):
self.collections = (asteroids, missiles, saucers, saucer_missiles, ships)
def asteroids(self):
return self.collections[0]
def missiles(self):
return self.collections[1]
def saucers(self):
return self.collections[2]
def saucer_missiles(self):
return self.collections[3]
def ships(self):
return self.collections[4]
But why should we require the () to fetch those? We can just make them properties of the class. We could use Python @property
but I think in this case we have no need of it.
- Aside:
- I will reverse that decision below. I was probably resisting
@property
because I had never tried it.
class TestSpaceObjectsCollection:
def test_access(self):
asteroids = ["asteroid"]
missiles = ["missile"]
saucers = ["saucer"]
saucer_missiles = ["saucer_missile"]
ships = ["ship"]
space_objects = SpaceObjects(asteroids, missiles, saucers, saucer_missiles, ships)
assert space_objects.asteroids == asteroids
assert space_objects.missiles == missiles
assert space_objects.saucers == saucers
assert space_objects.saucer_missiles == saucer_missiles
assert space_objects.ships == ships
class SpaceObjects:
def __init__(self, asteroids, missiles, saucers, saucer_missiles, ships):
self.asteroids = asteroids
self.missiles = missiles
self.saucers = saucers
self.saucer_missiles = saucer_missiles
self.ships = ships
self.collections = (asteroids, missiles, saucers, saucer_missiles, ships)
That’s passing. Sweet. Now I’d really like to be able to iterate SpaceObjects, each iteration returning the next of the five collections. I’m not sure how to build a Python iterator. Let’s defer that for now.
Let’s have Game create and save a SpaceObjects.
# noinspection PyAttributeOutsideInit
def init_space_objects(self):
self.asteroids = []
self.missiles = []
self.saucer = Saucer(Vector2(u.SCREEN_SIZE / 4, u.SCREEN_SIZE / 4))
self.saucers = []
self.saucer_missiles = []
self.ship = Ship(pygame.Vector2(u.SCREEN_SIZE / 2, u.SCREEN_SIZE / 2))
self.ships = []
self.space_objects = SpaceObjects(self.asteroids, self.missiles, self.saucers, self.saucer_missiles, self.ships)
I think part of adding simplicity will be that Game will no longer need the individual collections, but we’re not there yet.
We are close, however.
Now we can change Collider, I think.
class Collider:
def __init__(self, asteroids, missiles, saucers, saucer_missiles, ships):
self.movers = [asteroids, missiles, saucers, saucer_missiles, ships]
self.score = 0
We probably have some tests for this guy. We want to change his init. We’ll try Change Signature:
class Collider:
def __init__(self, space_objects):
self.space_objects = space_objects
self.score = 0
This does not work in the game: nothing collides with anything. I am starting to wish I had a fresher commit. I forgot to commit the last bit.
Anyway, we have tests for collider, let’s see what they have to say.
Most of the tests turn out not to be interesting or helpful. They all got converted to create a SpaceObjects but most of them only had two of the five collections, so I just had to replace the missing names with []. They all call mutual_destruction
, which does work.
There’s only one test that goes through process_collisions
and it is failing to score, which was what I saw in the game.
def test_collider_via_game_with_score(self):
game = Game(True)
asteroid = Asteroid(2, Vector2(100, 100))
game.asteroids=[asteroid]
missile = Missile.from_ship(Vector2(100, 100), Vector2(3, 3))
game.missiles=[missile]
game.process_collisions()
assert game.score == 20
class Game:
def process_collisions(self):
collider = Collider(self.space_objects)
self.score += collider.check_collisions()
class Collider:
def check_collisions(self):
for pair in itertools.combinations(self.space_objects.collections, 2):
self.check_individual_collisions(pair[0], pair[1])
return self.score
It’s acting like the lists are empty. A quick print in check_collisions
tells me that they are in fact empty. And I bet I know why: I bet I am setting them to []
somewhere in initialization, which will disconnect them from the SpaceObjects collection.
Yep:
# noinspection PyAttributeOutsideInit
def insert_quarter(self, number_of_ships):
self.asteroids = []
self.missiles = []
self.saucers = []
self.ships = []
self.asteroids_in_this_wave = 2
self.game_over = False
self.init_saucer_timer()
self.score = 0
self.ships_remaining = number_of_ships
self.set_ship_timer(u.SHIP_EMERGENCE_TIME)
self.init_wave_timer()
self.delta_time = 0
We want clear.
# noinspection PyAttributeOutsideInit
def insert_quarter(self, number_of_ships):
self.asteroids.clear()
self.missiles.clear()
self.saucers.clear()
self.ships.clear()
self.asteroids_in_this_wave = 2
self.game_over = False
self.init_saucer_timer()
self.score = 0
The game works now. For some reason the test does not. Ah. It can’t work:
def test_collider_via_game_with_score(self):
game = Game(True)
asteroid = Asteroid(2, Vector2(100, 100))
game.asteroids=[asteroid]
missile = Missile.from_ship(Vector2(100, 100), Vector2(3, 3))
game.missiles=[missile]
game.process_collisions()
assert game.score == 20
We cannot set game.missiles directly any more.
I can hack it this way:
def test_collider_via_game_with_score(self):
game = Game(True)
asteroid = Asteroid(2, Vector2(100, 100))
game.space_objects.asteroids.append(asteroid)
missile = Missile.from_ship(Vector2(100, 100), Vector2(3, 3))
game.space_objects.missiles.append(missile)
game.process_collisions()
assert game.score == 20
The test runs. Woot! Commit: Collider now uses Game.space_objects parameter instead of individual collections.
That was scary for a minute there. Note that again the bug was caused by the way that game was being initialized and re-initialized. It is still being done twice, because we haven’t removed the duplication yet.
That should be part of our adding lightness.
I’ve been at this for two hours now. Nearly time to wrap this up but I don’t feel quite done yet.
What if we were to change Game so that it doesn’t save the individual collections at all, and instead always refers to space_objects to get them? That would simplify Game by five member variables that are collections.
We might use @property to give it what looks like properties but that are delegated to the space_objects. We’d change this:
# noinspection PyAttributeOutsideInit
def init_space_objects(self):
self.asteroids = []
self.missiles = []
self.saucer = Saucer(Vector2(u.SCREEN_SIZE / 4, u.SCREEN_SIZE / 4))
self.saucers = []
self.saucer_missiles = []
self.ship = Ship(pygame.Vector2(u.SCREEN_SIZE / 2, u.SCREEN_SIZE / 2))
self.ships = []
self.space_objects = SpaceObjects(self.asteroids, self.missiles, self.saucers, self.saucer_missiles, self.ships)
To this:
# noinspection PyAttributeOutsideInit
def init_space_objects(self):
asteroids = []
missiles = []
saucer = Saucer(Vector2(u.SCREEN_SIZE / 4, u.SCREEN_SIZE / 4))
saucers = []
saucer_missiles = []
ship = Ship(pygame.Vector2(u.SCREEN_SIZE / 2, u.SCREEN_SIZE / 2))
ships = []
self.space_objects = SpaceObjects(asteroids, missiles, saucers, saucer_missiles, ships)
And then add these:
@property
def asteroids(self):
return self.space_objects.asteroids
@property
def missiles(self):
return self.space_objects.missiles
@property
def saucers(self):
return self.space_objects.saucers
@property
def saucer_missiles(self):
return self.space_objects.saucer_missiles
@property
def ships(self):
return self.space_objects.ships
I’d think everything should work as before. Except that, no, I shouldn’t have removed all the selfs with one big edit. I needed the ones for the individual saucer and ship:
# noinspection PyAttributeOutsideInit
def init_space_objects(self):
asteroids = []
missiles = []
self.saucer = Saucer(Vector2(u.SCREEN_SIZE / 4, u.SCREEN_SIZE / 4))
saucers = []
saucer_missiles = []
self.ship = Ship(pygame.Vector2(u.SCREEN_SIZE / 2, u.SCREEN_SIZE / 2))
ships = []
self.space_objects = SpaceObjects(asteroids, missiles, saucers, saucer_missiles, ships)
The tests and game work. Commit: asteroids, missiles, ships, saucers, and saucer_missiles collections now delegate to space_objects in Game.
Reflection
We might wrap here, but let’s at least reflect.
Most Notable:
I was bitten yet again by my dual initialization, once to create the Game and once to insert the quarter. We’ve improved things, in that we start right out running, but the code has not yet been consolidated and so, once again, I missed a change that needed to be made. Fortunately, I guessed quickly what was going on.
More Important:
We have just moved the individual collections of asteroids, missiles, saucers, saucer_missiles, and ships out of Game, and into SpaceObjects. The collections do exist as individuals in SpaceObjects but we know that they don’t need to. We could do it with properties. Like this:
class SpaceObjects:
def __init__(self, asteroids, missiles, saucers, saucer_missiles, ships):
self.collections = (asteroids, missiles, saucers, saucer_missiles, ships)
@property
def asteroids(self):
return self.collections[0]
@property
def missiles(self):
return self.collections[1]
@property
def saucers(self):
return self.collections[2]
@property
def saucer_missiles(self):
return self.collections[3]
@property
def ships(self):
return self.collections[4]
Commit: individual collections are virtual in SpaceObjects.
As I was saying before I was so rudely interrupted by myself: now there are no saved copies of the individual space object collections in the game. They are all virtual inside the SpaceObjects object.
We are on a good path to being able to simplify move_everything
, for example:
def move_everything(self, delta_time):
for the_saucer in self.saucers.copy():
the_saucer.move(delta_time, self.saucers, self.saucer_missiles, self.ships)
for missile in self.saucer_missiles:
missile.move(delta_time)
for the_ship in self.ships:
the_ship.move(delta_time)
for asteroid in self.asteroids:
asteroid.move(delta_time)
for missile in self.missiles:
missile.move(delta_time)
The change for that is just a tiny bit bigger than I want to do in this article, but very shortly we’ll be able to just say just this:
def move_everything(self, delta_time):
for collection in self.space_objects.collections:
for object in collection:
object.move(delta_time)
That will both simplify and add lightness. We’re definitely on the way!
! have only four commits, about one every thirty minutes, which is a bit too long to be ideal, but I missed at least one point where I could have committed, and probably two more more. This is one of my continuing weaknesses: I don’t commit as often as I could. It’s harmless to do so and often pays off by providing a better rollback point. Plus, improving my commit habit encourages me to find smaller steps.
That said, today’s steps were quite small, because I TDD’d the SpaceObjects class before using it.
A good morning. We’ll add more simplicity and lightness next time! See you then!