Comments on Iain Lobb: A code review of PewPew by Mike Chambers

Very nice break-down of the game. I very much like...

2010-07-26T00:51:24.750+01:00

Very nice break-down of the game. I very much like the amount of detail you've included here, excellent work. I also learned about the tooltip option you mentioned in FlashDevelop; I'd done it before, but on accident, didn't notice what was allowing it as I usually use // myself. Overall, excellent break-down, thanks for the good read, and thanks to Mike for sharing his code.

Also, Gamepad is rad. I don't have anything to...

2010-07-23T21:06:56.292+01:00

Also, Gamepad is rad. I don't have anything to say about it beyond that. Your style is impeccable. Much cleaner than mine, which I don't really have much pride in ;)

Readable, functional and generous. It's a wonderful utility. I gave up writing my own because of it.

I wouldn't commonly consider getting a code re...

2010-07-23T21:03:25.530+01:00

I wouldn't commonly consider getting a code review a good thing, mostly because hey, if you learn nothing from it and it becomes a religious debate, what good was it to anyone? We never did code reviews in any company I've worked for (I suspect due to them not really taking Flash seriously as a development challenge), but I've had third parties taking over code maintenance do them, often taking over code that was never even intended to be maintained in the first place. The results are harrowing.

The best way to learn about your own style and where it sits in the ecosystem, i think, is to code with someone else and work together on projects as equals. Granted that's also somewhat rare in our business, so those moments are to be cherished. It's so much better to review decisions BEFORE you commit to them rather than look at them in retrospect and feel like a jackass when someone else calls you out on your mistakes.

I dunno man. I think I might be antagonizing code reviews a little much, but I really believe there's a time and place where they are valuable, and without context that isn't it.

It's a rare project when you can look back at it and be completely happy with everything you did, which is why it's so nice to go back with new knowledge, look at how you did things and recognize how you can now do them better. Sometimes your old solutions will actually impress you, I guarantee it :)

We should make a game sometime, it'd be fun to compare techniques.

Hi Andreas. Thanks for your input. I am not sorry ...

2010-07-23T19:53:27.113+01:00

Hi Andreas. Thanks for your input. I am not sorry for offending you :)

If you have worked at a company that actually does code reviews, then you are one of the lucky ones. Most of us are stuck coding away in a corner without a clue how other people work or better ways things could be done.

There is an element of subjectivity in this of course, but isn't that what makes the discussion interesting?

I wasn't trying to suggest that you shouldn't use inheritance at all, just that a combination of polymorphism and composition is best. I've never yet seen composition over-used. The problem with Mike's code is the number of levels of inheritance rather than the fact that he is using it at all. Inheritance gives you code reuse within a project, but too much hierarchy absolutely makes your code harder move to another project. Imagine trying to add a ChaserEnemy from PewPew to another game. You'd also have to bring along all it's parents including PewPewGameObject, etc, which you really don't want in your new game. Now if you had a flat structure where everything is just an Entity, you could port it across no problem. Or imagine if you wanted to reuse some functionality from Enemy in a class that doesn't extend enemy, e.g. the PlayerShip. You will have to right the same code twice as Flash doesn't have multiple inheritance. Check out Ben Garney's articles about PushButtonEngine for some more info on why this is a good idea.

Re. Signals, I use them because I like to have events as part of my code-contract/interface, so that they show up on autocompletion menus etc. That's why for speed I suggested Turbo Signals, or just passing a reference to your main Game class to each entity and calling it's methods directly. Native AS3 events just aren't sexy, for all the reasons Robert Penner lists in his introduction to Signals.

Agree with you 100% re. buttons on touch screen. Sliding between screens etc is much nicer.

Re. doing a post-mortem on my old projects, that's a notion too horrifying to even consider :)

I am quite proud of my Open Source Gamepad project, and I would genuinely appreciate your thoughts (http://github.com/iainlobb/Gamepad) The hard part is getting someone to go to the effort of actually looking at your code!

Pretty cool article, though like most code reviews...

2010-07-23T19:14:51.373+01:00

Pretty cool article, though like most code reviews I've seen/been subjected to, it often comes down to subjective beliefs, for instance composition vs inheritance. And every time I see signals pop up when performance is concerned I have to scratch my head; Performance intensive code has no business being event driven in the first place, and at that point the performance benefits to omitting native events become entirely negligible. Beyond that, a rudimentary observer pattern will have you covered to the max if you really need events and want messaging. The described scenario goes something like thousands of events make for lots of cloned event objects and therefore more left to GC and poorer performance, but I have no idea who would fully justify that kind of code in any project requiring fast real-time performance, or even dispatching that many events either. It's just a weird edge case that I refuse to believe we actually struggle with.

In Flash we don't love inheritance because it "makes us feel like proper programmers". We love it because it's a fantastic tool and composition aside completely and utterly indispensable for anything resembling a sizable project.

I find it a little odd that you point out that inheritance makes for less reusable code, which i just can't agree with; Poor planning is what makes for less reusable code. I agree that excessive inheritance doesn't bode well for anything, but composition and inheritance go hand in hand; It's not an either-or proposition.

Formal code reviews (I realize this is informal ;P) are really only useful if both sides of the table have intimate knowledge of the goals, time frame and history of the code in question. For instance when reviewing CHANGES to a product, and evaluating them to ensure the quality and stability of the product isn't compromised. When it boils down to dude A talking about dude B's code without any context, I have to admit I actually find it a tad offensive.

Another pet peeve of mine is reliance on on-screen "buttons" for touchscreen control. This isn't "optimized for touch screens", it's "shoehorned into touch screens". Touch UI has nothing to do with buttons, and the more designers choose to go with that paradigm the worse the world is off for it.

End rant :) I'd love to see you do a post mortem of one of your own games next, perhaps one you did a year or so ago, with the same granularity.

Awesome write up. Im not going to go over all of ...

2010-07-23T17:17:23.578+01:00

Awesome write up.

Im not going to go over all of the points, but I did have a few clarifications.

--
For entities there is an inheritance chain of MovieClip > GameObject > PewPewGameObject > Enemy > ChaserEnemy. I've used this kind of approach on many games, and it works up to a point, but it doesn't make your code very reusabl
--

I completely agree with you on this, and this is one of the main things I meant when I talked about over engineering. If / when I do another game, I will almost certainly not use inheritance so extensive for game items. It really leads to a lot of complexity and refactoring as you develop the game.

Transitions between screens : i actually originally had this in about a year ago, but had to remove them for performance reasons. However, they should work fine now on the new android devices.

I use a circle based collision detection, which as you noticed is not always exactly accurate. This was again done for performance (on the iphone). I actually had versions of the game over the past year which used just about every collision detection strategy imaginable. I suspect, that, once again, using pixel perfect collision detection would work just fine on the android devices (maybe ill add that back in).

Ill fix the "include hidden layers" issue.

Anyways, great write up. This is why I released to code. I really wanted to help generate this type of discussion.

mike chambers

mesh@adobe.com

Nice post Iain. I like your point on composition ...

2010-07-23T16:48:00.655+01:00

Nice post Iain.

I like your point on composition vs excessive inheritance. I see this all the time along with overuse (abuse!) of interfaces. It seems like some people think more OOP = better.

I disagree with you on your suggestion of passing game entities a reference to the main game class. To me this is just tighter coupling which should be avoided. I think events or signals (as you mentioned) are preferable to this.

Thanks for the code Mike! I always learn something new when I look at other people's code.

Hi Iain, great post, thank you for this - I'...

2010-07-23T15:30:01.109+01:00

Hi Iain,

great post, thank you for this - I've been meaning to review this code so thanks for saving me the trouble! And hats off to Mike - I still find it scary revealing my source to the world :)

Framework included in the source? Totally agree. We'd use SVN externals to do this. You can include one SVN repo's folder within another, and even specify a revision. Not sure how you'd do this with GIT though, someone mentioned submodules.

And totally agree re hidden layers / guides. Use guides!

I don't mind the app class in the root, at least it's quick to find. My personal preference is to name it after the app rather than Main - we're not Java coders ;-)

Agree re the control mechanism - it's actually quite hard to get touch screen joystick control right and many Flash games really suffer from this on mobile (the fact you can't use cursor keys sadly renders most Flash games unplayable on mobile). One game that's got it really right is geometry wars on iPad and iPhone. Not sure about the directional firing but the motion controller with the left hand is spot on.

Totally agree re linking code to MovieClips - we just don't do it anymore and it greatly helps the collaborative process between art and code.

I think you get a little picky when it comes to transitions, I guess it would benefit from extra polish in that area but not that important. And interesting points about other types of polish, particles etc. I think you'd start to find the limitations of the mobile devices if you went too crazy in those areas, but should def try!

And instant death really isn't a problem if the rest of the playability is adjusted for that. Call me old fashioned if you like!

But thanks again for a great write up and thanks Mike for allowing us to delve around in your private space :-)

Seb