Re: A patch

[Jens Granseuer <jensgr@xxxxxxx>]

Finally got around to looking at the patches.

On 02.05.2007 18:17, haruspex wrote:
> > Thanks for your contribution. To make reviewing patches easier, it
> > would be nice if you could submit separate patches per issue in the
> > future. Please also use unified format (ideally diff -up).
> 
> Hope this is what is expected from me (: I'm not quite sure if these
> patches are going to work - some of them touch exactly the same parts
> of the code as the others, and I didn't test them separetely as
> thoroughly as during the coding.

Well, yes, mostly. That some of them had overlapping/duplicate parts
made it a tad more complicated, but luckily they were all pretty small.

> Anyway, here they are:
> animAttack.diff - cursor confirmation animation when attacking an enemy

Nice in general. What I'd really like would be something like grayscaling
the unit instead of just using a full white overlay. Or maybe shading it?
Not sure. Lighter probably looks better, but it's hard to say without
actually seeing it.
I'm wondering about the wait() function, though. What's the reason
you're not simply using SDL_Delay() there?

> animMove.diff - cursor confirmation animation when moving a unit

I like this one. Really neat solution.

> barXP.diff - display of the numerical value of XP

The consensus so far seems to be that this is not desirable. Makes
me happy. ;-)

> enemyInfo.diff - commented out two lines allowing to peek into an
> enemy unit info
> enemyRange.diff - ability to select enemy units to see how far they can move

Still undecided on these. Let's see where the discussion leads.

> MinGW.diff - a line I needed to compile with MinGW (not sure whether
> this applies to everyone, but maybe this could be of any use)

Now this looks odd. Maybe there's some header where we could get that
define from instead of hardcoding it ourselves? I'd rather not start
duplicating the Windows system API...

> turn.diff - units turn in the direction of their attack

Not sure, either. It's a rather small thing, but it somehow feels odd
to me. Maybe it's just being used to units not turning, don't know.
Other opinions?

Jens