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

[Justin Moore <justin@xxxxxxxxxxx>]

> > > I'm not sure about the correct way to solve this. "How much" tech do
> > > barbarians need?
> >
> > Do barbarians research at all?
>
> That is the question.

   I would say no.  I think it's possible for them to get techs, but not
to research them.

> > Why did you put the assert in the first place?
>
> get_num_human_and_ai_players() returns 0 for them. Divide by zero.

   Where is the division happening in that function?

> > > A robust solution IMHO is:
> > >
> > > + if(!is_barbarian(pplayer)) {
> > >   if (game.rgame.tech_leakage == 0) {
> > > ...
> > >     exit(1);
> > >   }
> > > + }
> >
> > Sounds fine.  However since the bracketed statement is rather long, I
> > would prefer another exit point:
> >
> > if (is_barabrian(pplayer)) {
> >   return cost;
> > }
>
> I initially decided against it since it may happen that we add another
> modifier like the game.year test after the leakage. But you are right
> this doesn't matter for barbarians.

   I've attached a small patch which may help.  It allows you to set a
tech for a barbarian, but won't let you update the amount of technology
they've received from their cities.

> > BTW, I know it's too late now, but I remember suggesting it before and
> > the suggestion was obviously ignored.  It would look so much better IMHO
> > if the "if - else if - else if"s in base_total_bulbs_required were
> > converted into a switch statement.
>
> 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.

   As a minor nitpick, wouldn't it make sense to switch the ordering of 1
and 2 so as to order them by increasing degree of tech_leakage?  Fx:

0: No leakage
1: Leakage from allies
2: Leakage from everyone

Just my $0.02.
-jdm

Department of Computer Science, Duke University, Durham, NC 27708-0129
Email:  justin@xxxxxxxxxxx

barbarian-tech.patch
Description: tech patch