[Freeciv-Dev] Re: Civserver upgrade building (PR#1585)

[Daniel L Speyer <dspeyer@xxxxxxxxxxx>]

The differenece is in the handling of border cases (case actually -- only
the beginning end is different).  This has a very real effect on games,
because of the way the code is called.  The switch to a for loop is, in
hindsight, gratuitous, and a simple

-  return -1;
+  latest_ok = -1;

would (I now think), suffice.

I haven't tested the above, however, and I'm terrible at fenceposting, so
someone please test before using (that's why I did the for thing
originally, I found it easier to check -- I didn't realize there would
be no change).

--Daniel Speyer
If you *don't* consider sharing information to be morally equivalent to 
kidnapping and murder on the high seas, you probably shouldn't use the
phrase "software piracy."

On Thu, 20 Jun 2002, Jason Short wrote:

> Raimar Falke wrote:
> > On Mon, Jun 17, 2002 at 12:55:11PM -0700, dspeyer@xxxxxxxxxxx wrote:
> > 
> >>Full_Name: Daniel Speyer
> >>Version: CVS 
> >>Distribution: Built from source
> >>Client: Both (or N/A)
> >>OS: Gnu/Linux -- Debian woody
> >>Submission from: (NULL) (12.243.200.32)
> >>
> >>
> >>If you develope musketeers before pikemen, your cities will continue to 
> >>build
> >>phalanxes.  This can be very annoying.  This patch should fix it (apply to
> >>freeciv/server/cityturn.c):
> > 
> > 
> > Looks like I miss something here. The patch looks like it transforms a
> > while in a for loop. Where is the catch?
> 
> The code works the same in that it follows the list of "obsoleted by" 
> units upward from the given unit to see if there is a possible upgrade.
> 
> The patch changes a "while" loop to a "for" loop.  It also changes the 
> boundaries of the iteration: the old code never iterated over the 
> original unit, while the new code does.  Neither of these should change 
> the effect of the loop.
> 
> At first glance I thought that the old code would stop and return -1 as 
> soon as it came to an unbuildable unit; thus if you cannot build pikemen 
> phalanx will not upgrade to musketeers.  The new code removed this check.
> 
> But on closer inspections I see no such constraint in the original code. 
>   Is it possible this has already been "fixed" in CVS?  So I'm with 
> Raimar: where is the catch?
> 
> Does anyone have a saved game that can demonstrate this "bug"?
> 
> -----
> 
> As for the proposed change itself, I'm all for it.  Even if it is 
> allowed to build units that have been obsoleted, or if musketeers don't 
> actually "obsolete" phalanx if you don't have pikemen (Raahul's "upgrade 
> path" argument), I think as soon as you discover gunpowder the current 
> *production* of phalanx should be upgraded to musketeers.
> 
> The only problem is that (I think) the current code checks all the time 
> for an upgrade, not just when musketeers become available.  My preferred 
> behavior here would be to allow the production of phalanx (this makes 
> for more stable rulesets but makes the interface harder), but to make a 
> one-time production upgrade when musketeers are discovered.  The player 
> could then change production back if necessary.  But, so long as players 
> aren't allowed to build obsolute units the lack of an upgrade here is a 
> bug (and this should be checked at the server and, too!).
> 
> jason
> 
> 
> 
>