Python 193 - Try an object
Tomas Aschan inspires me to try an object. I don’t think I’m going to do exactly as he said, but when this works, he deserves much of the credit. If, perchance, it doesn’t work, it’ll be Chet’s fault. But it will work.
He suggested a smart class for points, sketching this:
class DrawablePoints:
def __init__(self, raw_points):
self.raw_points = raw_points
def draw(self, screen, scale, position, rotation):
# as before
Then he suggested that we could have game constants for the instances. He later went on to say:
I think you can make the ship case even simpler by explicitly choosing an instance to draw at draw time, rather than giving it a index. Something like
should_draw_flare = ...
points_to_draw = SHIP_WITH_FLARE if should_draw_flare else SHIP_WITHOUT_FLARE
points_to_draw.draw(self.scale, self.position, self.rotation)
Although those constants could now be made class variables, making the strong coupling (mediated by the index parameter) less of an issue, so mostly a matter of taste.
I want to make a few points here.
First, thanks to Tomas for reading and offering ideas. His thoughts here have encouraged me to push forward to discover something better than what we have now.
Second, even this remote, sporadic form of collaboration is immensely valuable. It’s much more valuable to have real teamwork, whether mobbing, pairing, just working in the same room, or even meeting frequently to talk about things. The best programming times in my life have been when a few of us have worked intimately together toward a shared vision. If I were the sort of person who recommends things, I would strongly recommend that any interested programmer find some community to work with, ideally in their actual job situation.
Third, note that this is at least the fourth or fifth different way we’ve used to solve the “simple” problem of displaying our objects. Each solution has its own merits, and each of us might have a different preference among them. What is important, I think, is to be aware that there are generally multiple ways to do things, and that they differ on dimensions that we care about. It is wise, I think, to try multiple ways, before, during, and after we pick one.
Fourth, the smaller our objects and methods are, the easier it is to change them and replace them. There may be reasons to write long procedures and huge objects, but those things are far less important than some folks think. Large objects and methods are rigid and hard to change. Small ones are flexible and are easily changed, rearranged, and replaced.
Finally, remember that this little program doesn’t deserve all the special treatment we’re giving it. It’s small, it doesn’t need a lot of future changes, and if it did, it’s modular enough that changes would be easy. However, this program serves as a model of almost any larger one. The issues we find here are much like the issues in a larger program that has to be modified and improved indefinitely. The things we see, the forces we feel, and the decisions we make here are very much like those in “real life”. Small examples teach us.
Now let’s see what Tomas has made me think.
Design Thinking
I drew a picture last night, for some reason.
I was thinking about Tomas’ idea, and about what I might do that would be similar to his idea. I was thinking that there would be three instances of some new object, maybe Painter, and I was exploring what aspects of those objects would be constant over the lifetime of the object being painted, and which aspects would change.
Pretty clearly, the points collection and basic scale are constant for all of them. We never change those as we run (except for the ship’s drop-in, which I have not forgotten (yet)).
As we draw, asteroid and screen need the screen and the position, while the ship needs screen, position, its angle, and (I thought then) an index saying which picture to draw, the one with flare or the one without.
In view of Tomas’ remarks, I realize that I can create two Painters for the ship, and select which one to call at run time.
Tentative Plan
So my tentative plan is this:
- We’ll have a Painter object
- Containing a set of points
- Containing a scale factor
- With a
drawmethod accepting parameters: - screen (required)
- position (required)
- angle (optional, default 0)
- scale adjustment (optional, default 1)
Painter will have custom-made class methods to create instances for asteroid, saucer, and ship. There will be two class methods for ship, one with and one without flare. (Note that this is a change from my original idea of passing in an index. Thanks to Tomas for putting a better notion into my head.)
We will do this incrementally, so that we can commit frequently.
That’s a lot of planning for a person like me, and we should label it speculative1.
Building
Let’s do Saucer first, because it is simplest. There is only one shape, and two sizes.
I propose to work by intention2 here, since there’s little or nothing to test-drive in this essentially graphical object.
First we’ll have our Saucer create its Painter:
class Saucer(Flyer):
def __init__(self, radius, score, sound, is_small, always_target, scale):
Saucer.direction = -Saucer.direction
x = 0 if Saucer.direction > 0 else u.SCREEN_SIZE
position = Vector2(x, random.randrange(0, u.SCREEN_SIZE))
velocity = Saucer.direction * u.SAUCER_VELOCITY
self._directions = (velocity.rotate(45), velocity, velocity, velocity.rotate(-45))
self._gunner = Gunner(always_target)
self._location = MovableLocation(position, velocity)
self._radius = radius * u.SCALE_FACTOR
self._score = score
self._ship = None
self._sound = sound
self._zig_timer = Timer(u.SAUCER_ZIG_TIME)
self.is_small_saucer = is_small
self.missile_tally = 0
self.missile_head_start = 2*self._radius
self._scale = scale * u.SCALE_FACTOR
self._painter = Painter.saucer(self._scale)
That last line is enough to drive out the Painter class. I’ll put it in with the raw points, which I think may become class members of Painter, in due time. That file is where the short-lived LinePainter lived, which I think we’ll replace with this Painter.
class Painter:
def __init__(self, points, scale):
self._points = points
self._scale = scale
@classmethod
def saucer(cls, scale):
return cls(raw_saucer_points, scale)
I am not sure about where the scale should be calculated but here we have it so we’ll use it.
This compiles and runs the tests, no surprise there, we’re just creating this new instance. Let’s use it. Here’s draw:
class Saucer(Flyer):
def draw(self, screen):
points = raw_saucer_points
scale = self._scale
position = self.position
draw_lines(screen, points, position, scale)
Let’s make Painter do the work. It has the points and the scale, so it just needs screen and position:
def draw(self, screen):
self._painter.draw(screen, self.position)
That certainly looks like a nice improvement, doesn’t it? We can implement draw directly, since it, too, can call draw_lines:
class Painter:
def __init__(self, points, scale):
self._points = points
self._scale = scale
def draw(self, screen, position):
draw_lines(screen, self._points, position, self._scale)
@classmethod
def saucer(cls, scale):
return cls(raw_saucer_points, scale)
def draw_lines(screen, points, position, scale, angle=0):
adjusted = [(point.rotate(-angle) * scale) + position for point in points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
I expect this to draw the saucer correctly. Must test in game. Once I import Painter into Saucer, it works just fine. I wonder why my tests ran before I did that?
We can commit. Shall we? In favor: it’s working and a step forward. Against: What it if doesn’t work for asteroid or ship and we have to back it out? I feel sure that it’ll work just fine. Commit: Saucer uses new Painter object.
Asteroid
In for a penny, let’s do Asteroid. Let’s allow Painter to select the asteroid form: why should the object know what it looks like?
class Asteroid(Flyer):
def __init__(self, size=2, position=None):
self._rock_index = random.randint(0, 3)
self.size = max(0, min(size, 2))
self._score = u.ASTEROID_SCORE_LIST[self.size]
self.radius = [16, 32, 64][self.size] * u.SCALE_FACTOR
position = position if position is not None else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self._location = MovableLocation(position, velocity)
self._painter = Painter.asteroid(self.size)
Size here is 0, 1, or 2. That’ll be sufficient for Painter, and suggests that in general it might be well to have it use abstract values. Note that I’m not using rock_index, I’m going to roll my own in Painter. We should make a note to clean that up. I use my last 3x5 card to start taking notes.
In looking at painter, I realize that I’d rather have the asteroid radius, so change the call to pass that.
self._painter = Painter.asteroid(self.radius)
Now:
@classmethod
def asteroid(cls, radius):
which_one = random.randint(0, 3)
scale = radius / 4
return cls(raw_rocks[which_one], scale)
And let’s try draw:
def draw(self, screen):
self._painter.draw(screen, self.position)
Test in game. I am nervous about this one for some reason. Works perfectly. Commit: asteroid uses Painter.
Now there’s just ship. I propose to give it two painters, one with and one without flare.
class Ship(Flyer):
def __init__(self, position, drop_in=2):
self.radius = 25 * u.SCALE_FACTOR
self._accelerating = False
self._acceleration = u.SHIP_ACCELERATION
self._allow_freebie = True
self._angle = 0
self._asteroid_tally = 0
self._can_fire = True
self._drop_in = drop_in
self._hyperspace_generator = HyperspaceGenerator(self)
self._hyperspace_timer = Timer(u.SHIP_HYPERSPACE_RECHARGE_TIME)
self._location = MovableLocation(position, Vector2(0, 0))
self._missile_tally = 0
self._shipmaker = None
self._ship_points = raw_ship_points
self._accelerating_ship_points = raw_ship_points + raw_flare_points
self._ship_painter = Painter.ship(self.radius)
self._accelerating_painter = Painter.ship_accelerating(self.radius)
This drives out first this method:
@classmethod
def ship(cls, radius):
scale = 4 * u.SCALE_FACTOR
return cls(raw_ship_points, scale)
I notice that the scale appears to be known as 4 for some reason. Curious, but probably OK. Let’s fix draw to just draw the plain ship.
def draw(self, screen):
self._ship_painter.draw(screen, self.position)
This will let me move the ship, but it will not rotate, nor will it drop in. I want to see this much in the game, however. Small steps. Oops, I need to do ship_accelerating since I called it. OK:
@classmethod
def ship(cls):
scale = 4 * u.SCALE_FACTOR
return cls(raw_ship_points, scale)
@classmethod
def ship_accelerating(cls):
scale = 4 * u.SCALE_FACTOR
return cls(raw_ship_points + raw_flare_points, scale)
Since the radius is unused, I remove it from both methods and from the calls. Now I should be able to see the ship, if not with turns and flares. It does draw the ship as intended.
I have made a few “little” mistakes. Let’s slow down. We’ll reflect a bit.
Reflection
This is actually doing quite well. The draw methods, so far, are down to one line, and that one line is simpler than the draw call. The complexity that was inside the objects is now handled all in one place, over in Painter.
But I think I’m rushing. You know that thing where you’re running and suddenly you’re going too fast and can’t catch your balance? It’s like that, with code. The trick is to slow down before you fall down. This moment of reflection will help me settle down.
We are left with the matter of the ship angle, its drop-in, and which ship picture to draw. I’ve thought about this and what I intend is two optional arguments on draw, the angle, which will default to zero, and the scale adjustment, which will default to 1. The Ship will decide which painter to call.
We’ll do that last.
Ship
First, extend the draw method:
class Painter:
def draw(self, screen, position, angle=0, scale_adjustment=1):
draw_lines(screen, self._points, position, self._scale*scale_adjustment, angle)
draw_lines is already prepared to deal with angle and we just multiply in our scale adjustment. Now the ship can draw itself like this:
class Ship(Flyer):
def draw(self, screen):
self._ship_painter.draw(screen, self.position, self._angle, self._drop_in)
Now it should drop in correctly, turn correctly, and still not show flare. That’s what it does. Now for flare, Ship has this method:
def select_ship_source(self):
if self._accelerating and random.random() >= 0.66:
return self._accelerating_ship_points
else:
return self._ship_points
We aren’t using that any more, since it was only called in the old draw. We make it return whichever Painter we want:
def select_ship_source(self):
if self._accelerating and random.random() >= 0.66:
return self._accelerating_painter
else:
return self._ship_painter
And in draw, we just do this:
def draw(self, screen):
self.select_ship_source().draw(screen, self.position, self._angle, self._drop_in)
Now we should see flare as intended. And we do. I’ll prepare a short film for your enjoyment, doubtless showing me getting whipped by the game.
We can commit this: Ship uses Painter (twice).
Now we have some cleaning up to do. My card reminds me of asteroid’s references to rock index and such. Removing that, Asteroid’s init and draw look like this:
class Asteroid(Flyer):
def __init__(self, size=2, position=None):
self.size = max(0, min(size, 2))
self._score = u.ASTEROID_SCORE_LIST[self.size]
self.radius = [16, 32, 64][self.size] * u.SCALE_FACTOR
position = position if position is not None else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self._location = MovableLocation(position, velocity)
self._painter = Painter.asteroid(self.radius)
def draw(self, screen):
self._painter.draw(screen, self.position)
I think that’s cleaned up. Commit: tidying.
Now Saucer. I don’t see anything in that class that really needs changing:
def __init__(self, radius, score, sound, is_small, always_target, scale):
Saucer.direction = -Saucer.direction
x = 0 if Saucer.direction > 0 else u.SCREEN_SIZE
position = Vector2(x, random.randrange(0, u.SCREEN_SIZE))
velocity = Saucer.direction * u.SAUCER_VELOCITY
self._directions = (velocity.rotate(45), velocity, velocity, velocity.rotate(-45))
self._gunner = Gunner(always_target)
self._location = MovableLocation(position, velocity)
self._radius = radius * u.SCALE_FACTOR
self._score = score
self._ship = None
self._sound = sound
self._zig_timer = Timer(u.SAUCER_ZIG_TIME)
self.is_small_saucer = is_small
self.missile_tally = 0
self.missile_head_start = 2*self._radius
self._scale = scale * u.SCALE_FACTOR
self._painter = Painter.saucer(self._scale)
def draw(self, screen):
self._painter.draw(screen, self.position)
One issue that we might want to deal with is the scale, which is currently provided as part of the asteroid class method that we use to construct them. I think that’s out of scope for this morning.
Now Ship. We have this:
class Ship(Flyer):
def draw(self, screen):
self.select_ship_source().draw(screen, self.position, self._angle, self._drop_in)
def select_ship_source(self):
if self._accelerating and random.random() >= 0.66:
return self._accelerating_painter
else:
return self._ship_painter
I think this would be nicer as a property:
def draw(self, screen):
self.select_ship_source.draw(screen, self.position, self._angle, self._drop_in)
@property
def select_ship_source(self):
if self._accelerating and random.random() >= 0.66:
return self._accelerating_painter
else:
return self._ship_painter
That’s a bit cleaner, I think. Commit: convert select_ship_source to property.
Now we’ll inspect for leftover things we can remove.
class Ship(Flyer):
def __init__(self, position, drop_in=2):
self.radius = 25 * u.SCALE_FACTOR
self._accelerating = False
self._acceleration = u.SHIP_ACCELERATION
self._allow_freebie = True
self._angle = 0
self._asteroid_tally = 0
self._can_fire = True
self._drop_in = drop_in
self._hyperspace_generator = HyperspaceGenerator(self)
self._hyperspace_timer = Timer(u.SHIP_HYPERSPACE_RECHARGE_TIME)
self._location = MovableLocation(position, Vector2(0, 0))
self._missile_tally = 0
self._shipmaker = None
self._ship_points = raw_ship_points
self._accelerating_ship_points = raw_ship_points + raw_flare_points
self._ship_painter = Painter.ship()
self._accelerating_painter = Painter.ship_accelerating()
Those points are no longer needed. I delete those and I think that’s all we need here. Commit: ship tidying.
Now let’s clean up our Painter a bit. We’ll begin by renaming the file to painter. Here’s the whole file, so that we can discuss it:
# SurfaceMaker
from pygame import Vector2
import random
import pygame
import u
raw_saucer_points = [
Vector2(-2.0, -1.0), Vector2(2.0, -1.0), Vector2(5.0, 1.0),
Vector2(-5.0, 1.0), Vector2(-2.0, 3.0), Vector2(2.0, 3.0),
Vector2(5.0, 1.0), Vector2(2.0, -1.0), Vector2(1.0, -3.0),
Vector2(-1.0, -3.0), Vector2(-2.0, -1.0), Vector2(-5.0, 1.0),
Vector2(-2.0, -1.0)
]
raw_ship_points = [Vector2(-3.0, -2.0), Vector2(-3.0, 2.0), Vector2(-5.0, 4.0),
Vector2(7.0, 0.0), Vector2(-5.0, -4.0), Vector2(-3.0, -2.0)]
raw_flare_points = [Vector2(-3.0, -2.0), Vector2(-7.0, 0.0), Vector2(-3.0, 2.0)]
raw_rocks = [
[
Vector2(4.0, 2.0), Vector2(3.0, 0.0), Vector2(4.0, -2.0),
Vector2(1.0, -4.0), Vector2(-2.0, -4.0), Vector2(-4.0, -2.0),
Vector2(-4.0, 2.0), Vector2(-2.0, 4.0), Vector2(0.0, 2.0),
Vector2(2.0, 4.0), Vector2(4.0, 2.0),
],
[
Vector2(2.0, 1.0), Vector2(4.0, 2.0), Vector2(2.0, 4.0),
Vector2(0.0, 3.0), Vector2(-2.0, 4.0), Vector2(-4.0, 2.0),
Vector2(-3.0, 0.0), Vector2(-4.0, -2.0), Vector2(-2.0, -4.0),
Vector2(-1.0, -3.0), Vector2(2.0, -4.0), Vector2(4.0, -1.0),
Vector2(2.0, 1.0)
],
[
Vector2(-2.0, 0.0), Vector2(-4.0, -1.0), Vector2(-2.0, -4.0),
Vector2(0.0, -1.0), Vector2(0.0, -4.0), Vector2(2.0, -4.0),
Vector2(4.0, -1.0), Vector2(4.0, 1.0), Vector2(2.0, 4.0),
Vector2(-1.0, 4.0), Vector2(-4.0, 1.0), Vector2(-2.0, 0.0)
],
[
Vector2(1.0, 0.0), Vector2(4.0, 1.0), Vector2(4.0, 2.0),
Vector2(1.0, 4.0), Vector2(-2.0, 4.0), Vector2(-1.0, 2.0),
Vector2(-4.0, 2.0), Vector2(-4.0, -1.0), Vector2(-2.0, -4.0),
Vector2(1.0, -3.0), Vector2(2.0, -4.0), Vector2(4.0, -2.0),
Vector2(1.0, 0.0)
]
]
class Painter:
def __init__(self, points, scale):
self._points = points
self._scale = scale
def draw(self, screen, position, angle=0, scale_adjustment=1):
draw_lines(screen, self._points, position, self._scale*scale_adjustment, angle)
@classmethod
def saucer(cls, scale):
return cls(raw_saucer_points, scale)
@classmethod
def asteroid(cls, radius):
which_one = random.randint(0, 3)
scale = radius / 4
return cls(raw_rocks[which_one], scale)
@classmethod
def ship(cls):
scale = 4 * u.SCALE_FACTOR
return cls(raw_ship_points, scale)
@classmethod
def ship_accelerating(cls):
scale = 4 * u.SCALE_FACTOR
return cls(raw_ship_points + raw_flare_points, scale)
def draw_lines(screen, points, position, scale, angle=0):
adjusted = [(point.rotate(-angle) * scale) + position for point in points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
We certainly want to get rid of that top-level function, assuming that it is no longer called from outside. There are three imports and only our usage here.
I’ll fix up the imports. Now the only reference is in draw. Will PyCharm inline that for me? It will!
def draw(self, screen, position, angle=0, scale_adjustment=1):
scale = self._scale * scale_adjustment
adjusted = [(point.rotate(-angle) * scale) + position for point in self._points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
I think we’ll let that ride for now. I’m tired enough that a break is called for.
Test in game just to be sure. We’re good. Commit: tidying. Remove public draw_lines.
I reckon that we could move the raw points definitions inside the class as class members, but I don’t see much payoff in that, and I am tired. Quit when I’m tired, that’s my motto3.
Summary
We have removed most of the knowledge about drawing points and scale away from the Flyer subclasses, and into Painter, a single class with a few class methods and one operational method, draw, that handles all the cases we have. In so doing, we’ve reduced the Flyers’ draw method down to a one-liner, with a quick dispatch in the case of the Ship’s flare, which is a run-time decision that the ship really seems to need to make.
We did it by intention, and were driven entirely by that, and of course the specific needs of the individual classes. Points selection is broken out by class method constructors in Painter, and the drawing is the same for all instances.
I think this has gone quite nicely. Three classes simplified, one very compact new class, removal of a top-level function, folding it into the class.
One particularly interesting aspect of the changes was that we had LinePainter for a while, which was not really bearing its weight. It got replaced by the draw_lines function, and then that got replaced by Painter, which did a better job of taking on enough responsibility to be worth keeping. (And, of course, we were encouraged by Tomas.)
I am pleased, and grateful to Tomas for the inspiration. And I really miss working with one or more bright colleagues all the time. Except that I don’t want them in my house, and I don’t want to go to the office. Can’t have everything4.
See you next time!
-
“Speculative”, you’ll recall, is a long word meaning “probably wrong”. ↩
-
I learned this idea from Kent Beck. We write calls to the code we wish we had and then let those calls tell us what to implement. It’s like Test-Driven Development without explicit tests but instead with production code that does not yet work. Anything messed up here is my fault, not Kent’s. ↩
-
A motto for every occasion, that’s my motto. ↩
-
Where would you keep it? ↩