Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2002:
[Freeciv-Dev] Re: civserver segfault with new research system (PR#1221)
Home

[Freeciv-Dev] Re: civserver segfault with new research system (PR#1221)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: civserver segfault with new research system (PR#1221)
From: rf13@xxxxxxxxxxxxxxxxxxxxxx
Date: Sun, 13 Jan 2002 03:34:55 -0800 (PST)

On Sat, Jan 12, 2002 at 07:53:30PM +0000, Gregory Berkolaiko wrote:
>  --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> > On Sat, Jan 12, 2002 at 05:14:51PM +0000, Gregory Berkolaiko wrote:
> > >
> > > Do barbarians research at all?
> > 
> > That is the question.
> 
> Maybe Tony can help?
> They certainly possess knowledge but do they research?
> 
> > 
> > > Why did you put the assert in the first place?
> > 
> > get_num_human_and_ai_players() returns 0 for them. Divide by zero.
> 
> I see
> Wait, what do you mean "returns 0 _for_them_"?  It would return 0 in a
> barbarian-only game, would it not terminate?  Probably not, it will carry
> on forever...

You are right: 0 will only be returned in an barbarian-only game.

> > It was initially a switch statement. I converted it. Because there
> > were duplicated code: future techs are the same as style 0 and style 2
> > is style 1 is there are unset values.
> 
> Yes, I thought about it and arrived to the conclusion that introducing
> the variable tech_style = game.rgame.tech_cost_style, doing two ifs:
> if (tech > game.num_tech_types) {
>   /* Future techs evaluated in style 0 */
>   tech_style = 0;
> }
> 
> if (tech_style == 2 && advances[tech].preset_cost == -1) {
>   /* No preset, using style 1 */
>   tech_style = 1;
> }
> 
> and then doing the switch is still better.  In this way the exceptions 
> are clearly separated and documented, not mixed in with the main ifs.

I agree: nice solution.

> Personally I always struggle to understand else-if statements.
> 
> And in the tech_leakage case there are no exceptions to care about! 
> Unless you decide to do 
> 
> if (is_barbarian(pplayer)) {
>   /* Barbarians don't talk to anyone (can't speak that's why) */
>   tech_leakage = 0;
> }
> 
> which is also not too bad.

> Also occured to me: you don't have embassy with yourself, do you?
> Thus in 
>   cost = ((players - players_with_tech) * cost) / players;
> we are potentially dividing by zero (if have embassy with noone).

And another ack.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 Windows: Where do you want to go today?
 Linux: Where do you want to go tomorrow?
 BSD: Are you guys coming or what?



[Prev in Thread] Current Thread [Next in Thread]