[Freeciv-Dev] Re: foggy problems

[Thue Janus Kristensen <thue@xxxxxxx>]

> All of this is quite convoluted, and I could easily have missed something.

Tell me about it :)
But my problems do rather go in direction of writing a 5-line comment
about why I should do a thing and then immediately do the opposite :)

Your fixes seemed reasonable to me (for what that means).
Just one thing: If resolve_unit_stack was working properly the infinite
loop wouldn't have occured, cause the unit fartest from home would have
been moved, not the one in the city. This is a bug in the while loop in
resolve_unit_stack, cause it starts with 
struct city *pcity = find_closest_owned_city(get_player(punit->owner), x, y);
struct city *ccity = find_closest_owned_city(get_player(cunit->owner), x, y);   
  
and intend for those to be updated each time, but they are not if my C
understanding is correct. But the bug would still exist in the case where
a player have no cities left (in which case resolve_unit_stack
currently crashes from another bug :) )
Both fixed (but not tested at all) in the prepatch I sent along with a
few others.
But you are correct we shouldn't call resolve_unit_stack for units in a
city; the way transfer_city_units work it does not create stack conflics
in cities (I think, since transfer_city_units gives units in cities to
the city owner, but then how does the loop then come to be in the first
place?). Perhaps we should build that condition into resolve_unit_stack,
as any use of it on a city square can be hazardous. We should at least
print a warning.
I'll look at why the loop happens at all tomorrow when I am thinking
clearer...

We (I) should get this patch and the one I sent immidiatly in response
to your email together and then do some serious testing of the
transfer_city* code.

note that the patch I sent changed resolve_unit_stack, and not
transfer_city_units as I wrote (I was in a hurry as I invisioned you
working on the excact same code at the very moment. You will see that one
of the things you changed is actually also done in my patch)

-Thue