I’m still pecking away at improving the surface creation. I’m wondering if my caching object, tiny as it is, is more than I need. We’ll see. I think I should take advantage of the limitations of the current situation.

I rather like how the CachedMaker works, but I think it’s more general than it needs to be, and in being more general, harder to use. Let’s look:

    def test_fetch(self):
        maker = CachedMaker()
        make_count = 0

        def make(kind, shape, size):
            nonlocal make_count
            make_count += 1
            return "made {0}-{1}-{2}".format(kind, shape, size)

        made = maker.fetch("ship", 5, 7, make)
        assert made == "made ship-5-7"
        assert make_count == 1
        made = maker.fetch("ship", 5, 7, make)
        assert made == "made ship-5-7"
        assert make_count == 1

class CachedMaker:
    def __init__(self):
        self.cache = dict()

    def key(self, kind, shape=0, size=1):
        return "{0}-{1}-{2}".format(kind, shape, size)

    def fetch(self, kind, shape, size, action):
        key = self.key(kind, shape, size)
        try:
            return self.cache[key]
        except KeyError:
            new_one = action(kind, shape, size)
            self.cache[key] = new_one
            return new_one

Concerns include:

  1. The maker knows how to make the key. It shouldn’t know that: if it does, it’s coupled to its user and really can only serve one kind of user.
  2. It has to know how to call the action. Perhaps we could just pass in a bunch of args and pass them on to action?
  3. With basically one method, it may not be worth creating a special object.

Let’s recast the test and code a bit to see what we might do. The new rules will be, pass the key, the action, and the arguments to be sent to the action if it is triggered.

    def test_fetch(self):
        maker = CachedMaker()
        make_count = 0

        def make(kind, shape, size):
            nonlocal make_count
            make_count += 1
            return "made {0}-{1}-{2}".format(kind, shape, size)

        made = maker.fetch("ship-5-7", make, "ship", 5, 7)
        assert made == "made ship-5-7"
        assert make_count == 1
        made = maker.fetch("ship-5-7", make, "ship", 5, 7)
        assert made == "made ship-5-7"
        assert make_count == 1

Here we pass in the key we want, the function to call, and the three component arguments that our particular make requires. Let’s make that work. We’ll need to do variable args, however Python does it. Excuse me while I look that up.

Test wasn’t quite right. Do again:

    def test_fetch(self):
        maker = CachedMaker()
        make_count = 0

        def make(args):
            nonlocal make_count
            make_count += 1
            return "made {0}-{1}-{2}".format(args[0], args[1], args[2])

        made = maker.fetch("ship-5-7", make, "ship", 5, 7)
        assert made == "made ship-5-7"
        assert make_count == 1
        made = maker.fetch("ship-5-7", make, "ship", 5, 7)
        assert made == "made ship-5-7"
        assert make_count == 1

And to make it work:

class CachedMaker:
    def __init__(self):
        self.cache = dict()

    def fetch(self, key, action, *args):
        try:
            return self.cache[key]
        except KeyError:
            new_one = action(args)
            self.cache[key] = new_one
            return new_one

I removed the test for the key method and removed the key method as well, resolving issue #1 up above. Test is green.

I’m not saying I’ll use that object, and I’m not saying I won’t. I am saying that the test and code show me how to do roughly what I need to do.

Let’s take a look at SurfaceMaker again, and see what we might really need.

class SurfaceMaker:
    def __init__(self):
        self._two = 2
        pass

    def adjust(self, point, center_adjustment, scale_factor):
        return (point + center_adjustment) * scale_factor

    def get_ship_points(self):
        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)]
        return [self.adjust(point, Vector2(7, 4), 4) for point in ship_points]

    def get_flare_points(self):
        flare_points = [Vector2(-3.0, -2.0), Vector2(-7.0, 0.0), Vector2(-3.0, 2.0)]
        return [self.adjust(point, Vector2(7, 4), 4) for point in flare_points]

    def make_ship_surface(self, ship_points):
        ship_surface = pygame.Surface((60, 36))
        ship_surface.set_colorkey((0, 0, 0))
        pygame.draw.lines(ship_surface, "white", False, ship_points, 3)
        return ship_surface

    def make_accelerating_surface(self, flare_points, ship_points):
        ship_accelerating_surface = self.make_ship_surface(ship_points)
        pygame.draw.lines(ship_accelerating_surface, "white", False, flare_points, 3)
        return ship_accelerating_surface

    def prepare_surfaces(self):
        ship_points = self.get_ship_points()
        flare_points = self.get_flare_points()
        return (self.make_ship_surface(ship_points)), (self.make_accelerating_surface(flare_points, ship_points))

    def prepare_surface(self):
        surface = pygame.Surface((128, 128))
        surface.set_colorkey((0, 0, 0))
        adjusted = [self.adjust(point, Vector2(4, 4), 16) for point in raw_rocks[1]]
        pygame.draw.lines(surface, "white", False, adjusted, 3)
        return surface

    def asteroid_surface(self, shape, size):
        return self.prepare_surface()

We can certainly commbine prepare_surface and asteroid_surface. Let’s inline:

    def asteroid_surface(self, shape, size):
        surface = pygame.Surface((128, 128))
        surface.set_colorkey((0, 0, 0))
        adjusted = [self.adjust(point, Vector2(4, 4), 16) for point in raw_rocks[1]]
        pygame.draw.lines(surface, "white", False, adjusted, 3)
        return surface

Let’s test and commit before it’s too late. Commit: Simplify CachedMaker, tidy SurfaceMaker.

Let’s think about what we really want in surface making. Let’s assume that our SurfaceMaker does caching, so that when we ask for a surface it won’t recreate it, instead returning the one already created. Will that always work? I think yes, we never change an asteroid’s shape, and when we rotate the ship we make a copy from the original. So caching is OK. I panicked there for a moment.

Let’s assume that we’ll provide a key for the object we want, and all the parameters needed to make it.

Observation
There’s no need to be thinking about caching at this point. We’ll have to have a function that, given the parameters, creates the surface. Whether it caches or not is irrelevant from the outside.

Let’s think about what it takes to build a surface:

  1. The list of points (Vector2);
  2. The offset from center to top left;
  3. The scale factor to apply.

Let’s make sure we have that function now, somewhere inside SurfaceMaker. We presently have these three cases:

    def asteroid_surface(self, shape, size):
        surface = pygame.Surface((128, 128))
        surface.set_colorkey((0, 0, 0))
        adjusted = [self.adjust(point, Vector2(4, 4), 16) for point in raw_rocks[1]]
        pygame.draw.lines(surface, "white", False, adjusted, 3)
        return surface

    def make_ship_surface(self, ship_points):
        ship_surface = pygame.Surface((60, 36))
        ship_surface.set_colorkey((0, 0, 0))
        pygame.draw.lines(ship_surface, "white", False, ship_points, 3)
        return ship_surface

    def make_accelerating_surface(self, flare_points, ship_points):
        ship_accelerating_surface = self.make_ship_surface(ship_points)
        pygame.draw.lines(ship_accelerating_surface, "white", False, flare_points, 3)
        return ship_accelerating_surface

Those are a bit entangled, which is part of why I’ve left them alone. Let’s get a better structure here.

We start with a prepared surface of a given size and with a zero colorkey. Only in the case of the ship do we draw on the same surface twice. And we do need to draw the flare separately, unless we want to rearrange the points. We’ll let that be.

We have a starting collection of points (or two collections in the case of ship). We adjust those according to provided parameters, then draw them on the surface.

Let’s create a function. We’ll pass it the dimensions of the surface, the offset and scale, and one or more lists of points. I think this is it:

    def create_scaled_surface(self, dimensions, offset, scale, *point_lists):
        surface = pygame.Surface(dimensions)
        surface.set_colorkey((0, 0, 0))
        for point_list in point_lists:
            adjusted = [self.adjust(point, offset, scale) for point in point_list]
            pygame.draw.lines(surface, "white", False, adjusted, 3)
        return surface

Let’s call it, first for the asteroid:

    def asteroid_surface(self, shape, size):
        return self.create_scaled_surface((128, 128), Vector2(4, 4), 16, raw_rocks[0])

    def create_scaled_surface(self, dimensions, offset, scale, *point_lists):
        surface = pygame.Surface(dimensions)
        surface.set_colorkey((0, 0, 0))
        for point_list in point_lists:
            adjusted = [self.adjust(point, offset, scale) for point in point_list]
            pygame.draw.lines(surface, "white", False, adjusted, 3)
        return surface

Works as advertised. Commit: asteroid_surface uses create_scaled_surface.

Now the ship:

    def make_ship_surface(self, ship_points):
        return self.create_scaled_surface((60,36), Vector2(7, 4), 4, raw_ship_points)

We don’t have raw_ship_points, they are embedded in a function. I’ll pull them out and put them near the rocks.

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)]

That works. Do the flare:

    def make_accelerating_surface(self, flare_points, ship_points):
        return self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points, raw_flare_points)

Works just fine. Remove unused methods.

class SurfaceMaker:
    def __init__(self):
        self._two = 2
        pass

    def adjust(self, point, center_adjustment, scale_factor):
        return (point + center_adjustment) * scale_factor

    def make_ship_surface(self):
        return self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points)

    def make_accelerating_surface(self):
        return self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points, raw_flare_points)

    def prepare_surfaces(self):
        return (self.make_ship_surface()), (self.make_accelerating_surface())

    def asteroid_surface(self, shape, size):
        return self.create_scaled_surface((128, 128), Vector2(4, 4), 16, raw_rocks[0])

    def create_scaled_surface(self, dimensions, offset, scale, *point_lists):
        surface = pygame.Surface(dimensions)
        surface.set_colorkey((0, 0, 0))
        for point_list in point_lists:
            adjusted = [self.adjust(point, offset, scale) for point in point_list]
            pygame.draw.lines(surface, "white", False, adjusted, 3)
        return surface

We can do better. Extract two variables from this:

    def prepare_surfaces(self):
        return (self.make_ship_surface()), (self.make_accelerating_surface())

To this:

    def prepare_surfaces(self):
        ship_surface = self.make_ship_surface()
        accelerating_surface = self.make_accelerating_surface()
        return ship_surface, accelerating_surface

Then inline:

    def prepare_surfaces(self):
        ship_surface = self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points)
        accelerating_surface = self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points, raw_flare_points)
        return ship_surface, accelerating_surface

Rename method:

    def ship_surfaces(self):
        ship_surface = self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points)
        accelerating_surface = self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points, raw_flare_points)
        return ship_surface, accelerating_surface

And the whole class is down to this:

class SurfaceMaker:
    def __init__(self):
        pass

    def adjust(self, point, center_adjustment, scale_factor):
        return (point + center_adjustment) * scale_factor

    def ship_surfaces(self):
        ship_surface = self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points)
        accelerating_surface = self.create_scaled_surface((60, 36), Vector2(7, 4), 4, raw_ship_points, raw_flare_points)
        return ship_surface, accelerating_surface

    def asteroid_surface(self, shape, size):
        return self.create_scaled_surface((128, 128), Vector2(4, 4), 16, raw_rocks[0])

    def create_scaled_surface(self, dimensions, offset, scale, *point_lists):
        surface = pygame.Surface(dimensions)
        surface.set_colorkey((0, 0, 0))
        for point_list in point_lists:
            adjusted = [self.adjust(point, offset, scale) for point in point_list]
            pygame.draw.lines(surface, "white", False, adjusted, 3)
        return surface

That seems nearly good. Definitely enough for now, let’s sum up.

Summary

I’ve kind of digressed with the caching stuff, but we’ll see what we do. I think we’ll put it in in some form, but not really sure yet. Another scheme would be to build all the surfaces once and for all in the game initialization, but I rather like the lazy approach.

Thinking about caching got me thinking about what it takes to name a surface and what it takes to create one, and that led to the create_scaled_surface, which is clearly useful so far. The asteroid_surface still needs its parameters for which rock style and size, and there will be a bit of fiddling there, but it should all be nicely inside or under that method.

Possibly I could have done some extract methods to build that create method, but I found it easy to write and I think it would have been a lot of steps while writing the function was easy. We’ll never know, and it worked readily.

Progress. The program is smaller and a bit more clear. Gotta like that.

See you next time!