Dennis Schridde schreef: > Am Sonntag, 28. Januar 2007 schrieb Giel van Schijndel: > >> The Watermelon schreef: >> >>> On 1/28/07, *Giel van Schijndel* <[EMAIL PROTECTED] <mailto:[EMAIL >>> PROTECTED]>> >>> >>> wrote: >>> >>>> I assume ai.c's changes belong to 4? >>>> > I assume changeset 4 was not yet applied and you implemented it in r683 in a > different way? Because Per allready commited a patch with the same > description in r676... > (I was a bit confused, since it sounded as if r683 was a patch on > Watermelon's > changeset 4, initally commited by Per.) > Yes and no, I'm thinking that ai.c's changes belonged to changeset 4. This because that set's description best matched the changes. So I think Per didn't apply the entire changeset number 4.
Watermelon: could confirm or clear this up please? I'm guessing we're already seeing why separating patches has its benefits. >>>> Anyway, on line 74-86 of your patch you modify the droid's target update >>>> code to make sure all targets are updated. >>>> >>>> I'm just wondering as to why you wrap it in an if-statement (not the >>>> existing one, but the one you've created). Since even if >>>> `psDroid->numWeaps =< 1' (the else condition), the for loop would still >>>> do exactly the same as your one-time function call. And yes I know this >>>> piece of code probably saves us some CPU instructions, about 4 or so. >>>> >>>> This however isn't good enough a reason for making the code that much >>>> harder to read, that simple 3-line for-loop iteration is better to read >>>> and will most certainly have no exponential performance hit, nor linear, >>>> only static (which is negligible). >>>> >>>> Well, since that's all my comments on that part of the patch ( i.e. >>>> change 4), I've committed it without the if-wrap around the >>>> for-iteration in r683. >>>> >>>> -- >>>> Giel >>>> >>> the '<=1' check is intended,because a utility droid without weapon >>> will still have one valid target...if you just use 'for(i = 0;i < >>> psDroid->numWeaps;i++)' for both weapon and utiltiy droids,the target >>> update for utility droid will get skipped and cause undesired effects >>> such as repairing 'died' droid >>> and constructing destroyed building foundation I think. >>> >> Thanks for your answer, fixed it in r684. Also added an explaining >> comment that updateAttackTarget always has to be called on the first >> weapon-slot (i.e. 0). >> >> PS all you where originally checking in your patch was >> `psDroid->numWeaps != 0' for the for-iteration or ==0 for the other call. >> > r684 does look a bit ...weird... > Why not do > --- > updateAttack(0) > for( i=1; i < numWeaps) > updateAttack(i) > --- > ??? > Same code and a lot more readable, IMHO... > Yep, you're quite right, didn't thought of that method. And so I've learned a bit more today. Applied it in r685. -- Giel
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
