Follow-up Comment #7, bug #16100 (project freeciv):

Assuming Ulrik's analysis is correct (and I see no reason to doubt it on the
current evidence), it looks like the ACTIVITY_PILLAGE/S_LAST/BASE_NONE
combination has somehow made it into the server's data structures, which
shouldn't now happen. I considered adding a sanity check for this but didn't
get round to it -- but it doesn't look like that would be enabled on a public
server anyway.

If you're adding logging to track this down, I think the assertion at
unittools.c:546 ("assert(!(tgt == S_LAST && base == BASE_NONE))" in
total_activity_targeted()) would actually fail first.

> Also, I don't like how unit_activity_assign_target() in this 
> case assigns activity itself (name of the function no way 
> implies that, on the contrary name mentions that only certain 
> "part" (target) of the activity information is set). I recommend 
> leaving that to caller, once it receives "failure" return value 
> from unit_activity_assign_target(). Another possibility is to 
> rename function (but I have no good suggestions what the name 
> could be)

I'd prefer to keep the function as it is; the two callers need the same
things to happen to the members (as would any others, I think), so I don't
see value in leaving it up to the caller.

I see now that the function name is unfortunately confusing; "target" is the
best English word, but I apparently didn't think that it's also the name of
one of the data members. Perhaps unit_activity_assign_goal()?


Reply to this item at:


  Message sent via/by Gna!

Freeciv-dev mailing list

Reply via email to