[REVIEW] Killbots moved to KDE Review

[Parker Coates]

On Mon, Oct 6, 2008 at 13:56, Robert Knight <robertknight at gmail.com> wrote:
> First impressions of the code are good.  Easy to read and mostly
> self-documenting.  Fiddly parts like the
> Killbots::Engine::moveIsSafe() method have good comments to explain
> the algorithm - although if it
> goes on to more than a page or so it might be worth putting it in a
> text file somewhere else.

I had flipped back and forth on this point. I eventually decided to
leave it in the code for simplicities sake, but said that if another
enormous comment like this was needed, I'd start a new document for
them.

> I'd suggest using a QSize or a custom class rather than a QPoint to
> represent a vector,
> since its about magnitude in different directions.

I'm pretty sure QPoint is the right type for the job, but maybe
"vector" is the wrong name. My "vectors" are simply offsets that can
be added to a QPoint to get a neighbouring point.

> Consider using a switch() statement instead.  This has the advantage
> that the compiler will
> warn you if there any enum values which you haven't handled.

I knew eventually somebody was going to call me on that. I have an
aversion to switch statements: I find them clunky and hard to read (no
braces) and I have a tendency to forgot break statements which can be
pretty painful to debug. I also work with Python quite a bit, which
doesn't bother with a switch statement. But when in C++land do as the
C++ers do, I guess.

> Regarding the game itself, I think it would be useful to have a
> simple, preferably graphical tutorial available for people who
> have not played the BSD robots game that KillBots is based on.  This
> applies to quite a lot of games that are
> shipped with Ubuntu.  The description often reads something like:
>
> "Remake of the classic BSD/Famicom/Spectrum ZX81/Babbage Machine game XYZ"
>
> Which leaves the rest of us a bit mystified.

Well there is the handbook :) , but I realise that almost nobody reads
the documentation. I had looked at doing a tutorial earlier in the
development cycle, but put it aside as a "maybe later". The issue with
tutorials and demo modes is that they tend to be pretty invasive on
the codebase. Game logic that used to read cleanly is mucked up with
lots of "if(m_tutorial && m_tutorialStep == 4)" constructs. But now
that the codebase has settled down a bit, maybe I'll give it another
go. I don't really think this is feasible for 4.2 though.

Thanks for taking the time to give it a look over.

Parker