P-253 - ReservePlayer
Python Asteroids+Invaders on GitHub
Let’s do ReservePlayer. It has essentially no behavior. We’ll TDD it anyway, and will be glad we did.
The idea is that there is a new Flyer, ReservePlayer, representing, well, a player held in reserve. You know the deal, you get three or four players per quarter. The ReservePlayer represents one of those players. Each ReservePlayer will have an index, 0, 1, 2, 3, …, and it will display itself down at the bottom of the screen, spaced out horizontally according to its index. It will have no other behavior, but will offer interactions like a good little FLyer.
Anther object, which does not yet exist, PlayerMaker, will accept ReservePlayer interactions and will remember the ReservePlayer that goes by with the highest index. PlayerMaker will also interact with Player, simply noting whether one goes by. If none goes by, PlayerMaker, using TimeCapsule, will remove the remembered ReservePlayer from the mix and add in a real Player. Voila! One reserve disappears and “moves” to the field of battle.
This morning, we’ll just do ReservePlayer, unless I am more ambitious than seems likely. In any case, we’ll do it first.
Controversial Decision
I propose that the method interact_with_reserveplayer will not be made abstract. That will allow the one object that wants to interact with it to do so, and allow us to avoid editing all the other classes to add in an empty method. This is controversial because it means most of the objects will be inheriting concrete behavior, although the behavior is just to do nothing. I could find people who feel quite strongly that inherited behavior is undesirable. I myself think it is something to use with great caution. Here, I think we’ll be fine.
Let’s get to it.
ReservePlayer
I’m going to TDD this thing, since yesterday I build a trivial object without a test for what seemed like trivial behavior, and I made three consecutive mistakes that could only be found by running the game, but that could have been avoided by a single very simple test. I wouldn’t go so far as to say that I have finally learned my lesson, but I’m going to try to habitually write at least one or two tests for everything.
class TestReservePlayer:
def test_exists(self):
ReservePlayer(0)
That should drive out the little devil,
class ReservePlayer:
pass
The test now runs, but fails, because RP—we’ll call it RP for short—doesn’t expect a parameter. Fix that:
class ReservePlayer:
def __init__(self, reserve_number):
self.reserve_number = reserve_number
Test is green. Commit: initial test and code for ReservePlayer. We’re on our way!
One thing we could definitely test is the calculation of the coordinates where the RP will reside. I’m not at all sure what those should be, which makes it harder to write the test. A quick print tells me that it is 64 pixels wide and 32 high. That could be right. The Player code tells me that the Player “left” position is 64 plus half_width, or 96. Let’s start our leftmost RP there and space them, oh, 1/4 width apart.
Let’s use the test to drive out some of the Flyer behavior while we’re at it, specifically the rect that InvadersFlyer instances require. Then I can use that as the excuse for inheriting InvadersFlyer and doing some work.
def test_first_position(self):
player = ReservePlayer(0)
rect = player.rect
assert rect.centerx == 64 + rect.width // 2
That may be right. In any case it requires me to implement rect. I respond more heavily than I might by inheriting InvadersFlyer and then filling in some blanks.
PyCharm fills in a raft of interact-withs, and these, which I want to deal with:
class ReservePlayer(InvadersFlyer):
def __init__(self, reserve_number):
self.reserve_number = reserve_number
@property
def mask(self):
pass
@property
def rect(self):
pass
def draw(self, screen):
pass
A new test has failed, surely the one that is checking for compliance. I’ll add ReservePlayer to the exceptions. Right:
ignores = ["BeginChecker", "EndChecker", "TimeCapsule", "PlayerExplosion", "ReservePlayer"]
Now I am going to do a bit more than I need to to make this test pass. I could do the fake it till you make it ritual, but here I know pretty well what has to happen. I can borrow most of it from Player.
class ReservePlayer(InvadersFlyer):
def __init__(self, reserve_number):
self.reserve_number = reserve_number
maker = BitmapMaker.instance()
players = maker.players # one turret, two explosions
self.player = players[0]
self._rect = self.player.get_rect()
half_width = self.rect.width / 2
left = 64 + half_width
self.rect.center = Vector2(left, u.SCREEN_SIZE - 16)
My tests pass. Commit: ReservePlayer first position correct.
Let’s do one more test for the next position. I think we’ll space them 16 apart and see how that looks. As you can see above, I positioned it - 16 from the bottom. That’s going to be too low, we’ll need to adjust upward by 1/2 height or something. I just picked that value and picked wrongly. But our eyes have to tell us what we need.
class TestReservePlayer:
def test_exists(self):
ReservePlayer(0)
def test_first_position(self):
player = ReservePlayer(0)
rect = player.rect
assert rect.centerx == self.first_position(rect)
def test_second_position(self):
player = ReservePlayer(1)
rect = player.rect
assert rect.centerx == self.first_position(rect) + 64 + 16
def first_position(self, rect):
return 64 + rect.width // 2
I extracted the first_position method since I wanted to use the same value. I think I can improve those magic numbers but for now they’ll do. Implement:
class ReservePlayer(InvadersFlyer):
def __init__(self, reserve_number):
self.reserve_number = reserve_number
maker = BitmapMaker.instance()
players = maker.players # one turret, two explosions
self.player = players[0]
self._rect = self.player.get_rect()
half_width = self.rect.width / 2
left = 64 + half_width
x = left + reserve_number*(5*self._rect.width//4)
self.rect.center = Vector2(x, u.SCREEN_SIZE - 16)
The tests pass. Commit: ReservePlayer calculates position as intended (may need visual adjustment).
The test magic numbers need improvement but I want to draw and see how it looks, because they may need changing.
We’ll have to add a draw, and add some RPs to the coin.
def draw(self, screen):
screen.blit(self.player, self.rect)
And
def invaders(fleets):
fleets.clear()
left_bumper = 64
fleets.append(Bumper(left_bumper, -1))
fleets.append(Bumper(960, +1))
fleets.append(TopBumper())
fleets.append(InvaderFleet())
fleets.append(TimeCapsule(InvaderPlayer(), 2))
fleets.append(ShotController())
for i in range(3):
fleets.append(ReservePlayer(i))
half_width = 88 / 2
spacing = 198
step = 180
for i in range(4):
place = Vector2(half_width + spacing + i*step, 800-16)
fleets.append(Shield(place))
Run it, see what it does. Not bad at all:
I might be happy to raise them up just 16 pixels. Yes, that works. Improve the test magic numbers while we’re here.
class TestReservePlayer:
def test_exists(self):
ReservePlayer(0)
def test_first_position(self):
player = ReservePlayer(0)
rect = player.rect
assert rect.centerx == self.first_position(rect)
def test_second_position(self):
player = ReservePlayer(1)
rect = player.rect
step_between_rps = rect.width + rect.width // 4
assert rect.centerx == self.first_position(rect) + step_between_rps
def first_position(self, rect):
return 64 + rect.width // 2
Green. Commit: ReservePlayers in the mix and display along bottom left of screen.
Let’s reflect.
Reflection
I actually managed to write simple but useful tests for the ReservePlayer. Not that it was difficult to do, but it took an initial act of will to decide to do it, because the object was going to be so simple that surely even a fool like me could just code it up. And I’m sure that I could have.
However, the tests did provide the occasion for three commits instead of one, which I think is a good thing because it shows that there were three points where everything was good. And working with the test on positioning helped me focus on the numbers and their meaning. I don’t think the tests ever showed that I had made a mistake, but they did give structure to my progress. Plus I feel pretty good just because I did it.
I wonder if we should do PlayerMaker or a start at it. I don’t feel tired and I’ve only been at this for about an hour.
Yes, not much programming for an hour, but cut me a break here, I’m also writing an article and including pictures in it. Plus I’m eating a banana.
PlayerMaker
How do we get new players now?
class InvaderPlayer(InvadersFlyer):
def hit_by_shot(self, fleets):
fleets.append(PlayerExplosion(self.position))
fleets.remove(self)
fleets.append(TimeCapsule(InvaderPlayer(), 2))
We just toss one back in, in a time capsule.
- Note:
- This will come back to bite me. I should have removed this right here, but I was more interested in seeing how it worked and didn’t realize that with this code still in there, we’d get extra players. No major harm, but a yellow card at least.
It is tempting to enhance Player to check the reserves and make the decision. Our design really calls for a separate watcher, but just to see how bad things get, let’s decide to enhance player to do this.
But wait. If I do it with PlayerMaker, I can TDD the new object easily. If I do it in Player, I’ll have to wire in some weird tests, or just hack it in. No, doing it in Player just isn’t the thing to do. We’ll TDD up a PlayerMaker.
Whew. I hope everyone is proud of me for doing the better thing. That was a close call.
class TestPlayerMaker:
def test_exists(self):
PlayerMaker()
I really like this habit of writing a trivial “exists” test. It gets me started with a test class without actually having to think much or do much work. Once I have the test class open, it’s much easier to do subsequent more significant tests.
class PlayerMaker:
pass
Test passes. Commit: initial test and code for PlayerMaker.
Done. Ship it. And, by the way, we could have shipped product at any point this morning. We have at no time broken the game, just made it a bit closer to the game we want to wind up with.
OK, let’s think about PlayerMaker, what it does, and how we might test it.
It’s going to watch for InvaderPlayer instances and if it sees none it wants to rez a new player. It watches for ReservePlayer instances and saves the one with the highest reserve_number. If it wants to rez a new player and has a reserve, it creates a new player, and a two-second TimeCapsule that will remove the reserve and add the player.
That will require a new feature on the TimeCapsule, to remove something. Currently it only adds new things.
Let’s do that first. I’d like to have an optional parameter on TimeCapsule. Maybe the thing to add should be optional as well. Should we change it so that the time is first? Here is its init now:
class TimeCapsule(InvadersFlyer):
def __init__(self, flyer, time):
self.flyer = flyer
self.time = time
Let’s change signature to put time first and have an added and removed parameter. Change Signature does this:
class TimeCapsule(InvadersFlyer):
def __init__(self, time, added_flyer, removed_flyer=None):
self.flyer = added_flyer
self.time = time
Now I think I’d like a test.
def test_append_remove(self):
reserve = ReservePlayer(2)
player = InvaderPlayer()
capsule = TimeCapsule(2, player, reserve)
fleets = Fleets()
fi = FI(fleets)
fleets.append(reserve)
fleets.append(capsule)
assert fi.reserve_players
assert not fi.invader_players
fleets.tick(2.1)
assert not fi.reserve_players
assert fi.invader_players
This would be fine if FI understood reserve_players and invader_players. It doesn’t, yet, but they are easy to add. Here’s RP, just so you can see how easy it is:
@property
def reserve_players(self):
return self.select_class(ReservePlayer)
Test of course fails. I think it should be failing on the RP still being there. Yes:
> assert not fi.reserve_players
E assert not [<reserveplayer.ReservePlayer object at 0x115478310>]
E + where [<reserveplayer.ReservePlayer object at 0x115478310>] = <tests.tools.FleetsInspector object at 0x115478410>.reserve_players
First, I’ll reverse the order of those last two checks, just to be sure that it did add the player. Yes, same failure. Now in TimeCapsule:
class TimeCapsule(InvadersFlyer):
def __init__(self, time, added_flyer, removed_flyer=None):
self.to_add = added_flyer
self.to_remove = removed_flyer
self.time = time
def tick(self, delta_time, fleets):
self.time -= delta_time
if self.time <= 0:
fleets.remove(self)
fleets.append(self.to_add)
if self.to_remove:
fleets.remove(self.to_remove)
Green. Commit: TimeCapsule can now remove one flyer as well as add one.
I think possibly the pythonic thing to do there would be to make those flyer parms keyword parms but that is not as yet part of our coding standard here chez Ron.
We were testing PlayerMaker and needed a feature in TimeCapsule. That’s done. Back to the PlayerMaker with a harder test than just existing. Let me review what I wrote before because there was a point to it.
It’s going to watch for InvaderPlayer instances and if it sees none it wants to rez a new player. It watches for ReservePlayer instances and saves the one with the highest
reserve_number. If it wants to rez a new player and has a reserve, it creates a new player, and a two-second TimeCapsule that will remove the reserve and add the player.
The point was, I’d like to come up with a simpler test than another of those ones with Fleets and FI and all that. I think I will try calling the various interactions directly.
I’m going to try a fairly invasive test, property-based. First let’s test that it can capture the right reserve player as they go by:
def test_captures_reserve(self):
maker = PlayerMaker()
maker.interact_with_reserveplayer(ReservePlayer(1))
maker.interact_with_reserveplayer(correct :=ReservePlayer(2))
maker.interact_with_reserveplayer(ReservePlayer(0))
assert maker.reserve == correct
He should save the middle correct one.
He has no methods to speak of. Give him this:
class PlayerMaker:
def __init__(self):
self.reserve = None
def interact_with_reserveplayer(self, reserve):
if not self.reserve:
self.reserve = reserve
elif reserve.reserve_number > self.reserve.reserve_number:
self.reserve = reserve
This is good. We should ensure that he clears the reserve in begin. I’ll add that to the exiting test, as if a second cycle was beginning.
def test_captures_reserve(self):
maker = PlayerMaker()
maker.interact_with_reserveplayer(ReservePlayer(1))
maker.interact_with_reserveplayer(correct :=ReservePlayer(2))
maker.interact_with_reserveplayer(ReservePlayer(0))
assert maker.reserve == correct
maker.begin_interactions(None)
assert not maker.reserve
And:
class PlayerMaker:
def __init__(self):
self.reserve = None
def begin_interactions(self, _fleets):
self.reserve = None
def interact_with_reserveplayer(self, reserve):
if not self.reserve:
self.reserve = reserve
elif reserve.reserve_number > self.reserve.reserve_number:
self.reserve = reserve
Nice enough. We’ll need the superclass soon but so far so good.
Test to see if a player has been seen.
def test_notices_player(self):
maker = PlayerMaker()
maker.begin_interactions(None)
assert maker.player_missing
maker.interact_with_invaderplayer(None, None)
assert not maker.player_missing
I code this which helps me recognize an issue:
class PlayerMaker:
def __init__(self):
self.reserve = None
self.player_missing = True
def begin_interactions(self, _fleets):
self.reserve = None
self.player_missing = True
def interact_with_invaderplayer(self, _player, _fleets):
self.player_missing = False
def interact_with_reserveplayer(self, reserve, _fleets):
if not self.reserve:
self.reserve = reserve
elif reserve.reserve_number > self.reserve.reserve_number:
self.reserve = reserve
The interact_with_foo methods take two parameters, not one. Fix up the tests and green.
class TestPlayerMaker:
def test_exists(self):
PlayerMaker()
def test_captures_reserve(self):
maker = PlayerMaker()
maker.interact_with_reserveplayer(ReservePlayer(1), None)
maker.interact_with_reserveplayer(correct :=ReservePlayer(2), None)
maker.interact_with_reserveplayer(ReservePlayer(0), None)
assert maker.reserve == correct
maker.begin_interactions(None)
assert not maker.reserve
def test_notices_player(self):
maker = PlayerMaker()
maker.begin_interactions(None)
assert maker.player_missing
maker.interact_with_invaderplayer(None, None)
assert not maker.player_missing
I felt badly at first, passing in those None parameters instead of ginning up a Fleets and an InvaderPlayer and all that, but in fact passing the None gives us important information: the class ignores those parameters. I think I might like that.
I am tempted to try something a bit naff. Let’s test just a bit more and then try it.
def test_rezzes_time_capsule(self):
maker = PlayerMaker()
maker.begin_interactions(None)
maker.interact_with_reserveplayer(ReservePlayer(0), None)
maker.end_interactions(fleets := FakeFleets())
assert fleets.appends
capsule = fleets.appends[0]
assert isinstance(capsule.to_add, InvaderPlayer)
assert isinstance(capsule.to_remove, ReservePlayer)
There is no end_interactions yet, so:
def end_interactions(self, fleets):
if self.player_missing and self.reserve:
capsule = TimeCapsule(2, InvaderPlayer(), self.reserve)
fleets.append(capsule)
Test passes. At first I left the parens off InvaderPlayer and the test caught that.
Green, so commit: PlayerMaker makes suitable time capsule in end_interactions.
Here’s the poor idea. Instead of having PlayerMaker inherit from InvadersFlyer, let’s see if we can duck type the whole thing. To do that, I’ll put it into the coin, remove the player that’s there, and see what breaks. The first thing will probably be interact_with but there will be others. I just want to see how a simple class like this would look if we duck typed it.
def invaders(fleets):
fleets.clear()
left_bumper = 64
fleets.append(Bumper(left_bumper, -1))
fleets.append(Bumper(960, +1))
fleets.append(TopBumper())
fleets.append(InvaderFleet())
fleets.append(PlayerMaker())
fleets.append(ShotController())
for i in range(3):
fleets.append(ReservePlayer(i))
half_width = 88 / 2
spacing = 198
step = 180
for i in range(4):
place = Vector2(half_width + spacing + i*step, 800-16)
fleets.append(Shield(place))
I’ll put in a few things that I know I’ll need, save myself some runs, since I have to run the game to root out all these methods.
No, what am I doing???
This is just dumb. PyCharm will create them for me. Belay the duck typing idea, it’s harder than doing it right! Tiny fool!
Right. Did that, put PlayerMaker in that list of ignored classes, and now I’m ready to find out why it doesn’t work.
Well, it doesn’t work: no Player ever appears. But why not? Are we getting any events at all? A quick print tells me that we are getting tick events. A second quick print tells me we aren’t getting begin events.
That turns out to be happening because somehow I got two copies of the methods in the class, as part of moving things around. Now I’m getting them but I’m not seeing the action.
Ah. I’m not seeing the reserve. Why? Because it doesn’t issue its interact_with.
class ReservePlayer(InvadersFlyer):
def interact_with(self, other, fleets):
other.interact_with_reserveplayer(self, fleets)
Now everyone will be getting that call, so I put a pass in InvadersFlyer for everyone to inherit. Yes, I know some of you, and part of me, thinks that’s a bit off. I am inclined to make it the new standard for a while just to balance things out.
And now we find the real bug. Whenever the PlayerMaker notices there is no player, it rezzes a time capsule to create one. Even if it just did that millisecond ago. How can we fix this?
One possibility is for the PlayerMaker to remove itself and create a new one with another TimeCapsule. Let’s do that.
def end_interactions(self, fleets):
if self.player_missing and self.reserve:
fleets.remove(self)
capsule = TimeCapsule(2, InvaderPlayer(), self.reserve)
fleets.append(capsule)
fleets.append(TimeCapsule(2.1, PlayerMaker()))
Kludge? Hack? Elegant solution in full accord with our distributed design scheme? Why not all three?
Works perfectly. Commit: PlayerMaker creates new players until they run out. No game over yet.
I kind of wish I had played a bit more before committing that. I do not know why, but the first player works correctly. The second player’s shots can kill two invaders, and the third player’s shots can kill three.
What that has to mean is that there are somehow multiple players in the mix. But there can’t be: we have come up with player missing, which means that PlayerMaker saw zero players during interactions.
A quick print in the firing code tells me that yes I am seeing two or three calls to fire, from two different players.
Are they in the mix? They must be, because a print in “move” shows more than one moving.
Oh. I forgot to remove the other creation:
class InvaderPlayer(InvadersFlyer):
def hit_by_shot(self, fleets):
fleets.append(PlayerExplosion(self.position))
fleets.remove(self)
fleets.append(TimeCapsule(2, InvaderPlayer()))
That had me more confused than I needed. Remove that. Now it’s good. Commit: Fix defect creating extra simultaneous players.
A break and then let’s sum up.
Summary
Aside from the amusing multiple player thing, this went very nicely. We have fairly decent tests for the TimeCapsule and ReservePlayer and the PlayerMaker has really rather nice tests. I suppose we could add a test to Player showing that it no longer adds a player back. I’ll do that later, if I remember. I’m tired and ready for Sunday breakfast now.
The multiple player thing had me confused for a bit, but I moved a few quick prints around and soon saw that there really were multiple players in the mix. That led me by the nose to the deletion of the hit player, where (slaps forehead) the addition of the new one was right there in front of me.
A better sequence of testing might have been:
- Show that after a hit, there is no player;
- Etc., all the stuff we did test.
But I didn’t think of that. Life’s tough, then you unfortunately don’t get another player as far as we know.
I think this went well, and now we have a nice looking player-rezzing feature. See you next time!