[Freeciv-Dev] Re: (PR#2930) building requirements for units v2

["Raahul Kumar via RT" <rt@xxxxxxxxxxxxxx>]

--- "Per I. Mathisen via RT" <rt@xxxxxxxxxxxxxx> wrote:
> 
> This patch adds a new field to units.ruleset called "impr_req", which
> makes the building of a unit conditional upon the presence of the given
> building.
> 

This patch looks wrong to my eye. Where the hell is the default ruleset data
that you sent in with the original?

> > Index: ai/advmilitary.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/ai/advmilitary.c,v
> retrieving revision 1.134
> diff -u -r1.134 advmilitary.c
> --- ai/advmilitary.c  30 Jan 2003 19:53:50 -0000      1.134
> +++ ai/advmilitary.c  30 Jan 2003 20:14:20 -0000
> @@ -954,7 +954,17 @@
>              best_choice->choice = unit_type;
>              best_choice->want = want;
>              best_choice->type = CT_ATTACKER;
> -          } /* FIXME: else find out why we can't build it and do something
> */
> +          } else if (can_build_improvement(pcity,
> +                            get_unit_type(unit_type)->impr_requirement)) {
> +            Impr_Type_id id = get_unit_type(unit_type)->impr_requirement;
> +
> +            CITY_LOG(LOG_DEBUG, pcity, "building %s to build %s",
> +                     get_improvement_type(id)->name,
> +                     get_unit_type(unit_type)->name);
> +            best_choice->choice = id;
> +            best_choice->want = want;
> +            best_choice->type = CT_BUILDING;
> +          }
>          }
>        }
>      }

Is this useful for troubleshooting other AI insanity? I can see way too much
info accumulating in debug mode now.

>    u->defense_strength   = p->defense_strength;
>    u->move_rate          = p->move_rate;
>    u->tech_requirement   = p->tech_requirement;
> +  u->impr_requirement   = p->impr_requirement;
>    u->vision_range       = p->vision_range;
>    u->transport_capacity = p->transport_capacity;
>    u->hp                 = p->hp;

I always thought both tech and impr requirement looked out of place here.

>  #define CAPABILITY "+1.14.0 conn_info +occupied team tech_impr_gfx " \
> -                   "city_struct_minor_cleanup obsolete_last"
> +                   "city_struct_minor_cleanup obsolete_last +impr_req"
>    
>  /* "+1.14.0" is protocol for 1.14.0 release.
>   *
> @@ -95,6 +95,8 @@


>  void init_our_capability(void)
>  /**************************************************************************
>  Whether given city can build given unit,
>  ignoring whether unit is obsolete.
>  **************************************************************************/
>  bool can_build_unit_direct(struct city *pcity, Unit_Type_id id)
>  {
> -  if (!can_player_build_unit_direct(city_owner(pcity), id))
> +  Impr_Type_id impr_req;
> +
> +  if (!can_player_build_unit_direct(city_owner(pcity), id)) {
> +    return FALSE;
> +  }
> +  impr_req = get_unit_type(id)->impr_requirement;
> +  assert(impr_req <= B_LAST && impr_req >= 0);

Can this happen? If so, you should be checking for this when loading the
ruleset.


> --- server/ruleset.c  30 Jan 2003 19:26:08 -0000      1.132
> +++ server/ruleset.c  30 Jan 2003 20:14:23 -0000
> @@ -651,7 +651,11 @@
>    /* main stats: */
>    unit_type_iterate(i) {
>      u = &unit_types[i];
> -    
> +
> +    u->impr_requirement = find_improvement_by_name(
> +                           secfile_lookup_str_default(file, "None", 
> +                           "%s.impr_req", sec[i]));
> +

I'm going to triple your workload with this request, but don't use _default
here. secfile_lookup_str is your friend ; ). People may want to send in
rulesets other than default using your idea.  I know I want to at any rate.
Getting Default changed can be an uphill battle. 

Aloha,
RK.

There are those who will claim that not believing in an afterlife is as much a
leap of faith as believing in one. Believing that there *aren't* invisible
three-winged sky fairies floating over everyone's heads, by this logic, is just
as much a leap of faith as believing that there are.  It is no coincidence that
the "logic" afterlife believers hold is so flawed. Religion teaches you not to
question. --Raahul


__________________________________________________
Do you Yahoo!?
New DSL Internet Access from SBC & Yahoo!
http://sbc.yahoo.com