[Freeciv-Dev] (PR#6216) Assertion `punit->transported_by != -1' failed

["John Wheeler" <jdwheeler42@xxxxxxxxx>]

[jdorje - Mon Sep 22 05:01:41 2003]:

> Here is a quick-fix patch.  Unfortunately this just makes the current 
> ugliness uglier.  Fixing the ugliness is harder and bug-prone: we have 
> to update player unit lists, city supported unit lists, fog, contact, 
> sentry wakeup, city tile status maps, and additional syncing with the 
> client.

I assumed that taking care of all that stuff was why unit transfers were
done as creating a new unit, instead of just changing the owner field.
 
> I wonder what other values aren't initialized in this transfer?

According to LXR, create_unit_full() is just called in 3 other places:
server/citytools.c, line 1101 in remove_city(); server/diplomats.c, line
442 in diplomat_bribe(); and server/unittools.c, line 1471 in
create_unit(), which is just a wrapper for create_unit_full() that hides
some details that aren't needed the other places a unit is created.

Because of this, I'm thinking create_unit_full() should take as
parameters all the fields defined for a unit.  This might even take care
of some undiscovered bugs!

It's been a while since I've programmed in straight C, so I've forgotten
whether you can have default arguments, e.g.

void twisted(int spirals, bool right=FALSE);

so both

twisted(4);

and

twisted(4, TRUE);

are valid calls.  If they are, then it would be possible just to add a
default "int transported_by = -1" to create_unit_full().  If that's
going to break any compiler compatibility, or is just considered bad
form, it's not too much trouble to change everything that calls
create_unit_full().  So, which approach would be better?

-- 

++JohnWheeler