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

["Jason Short via RT" <rt@xxxxxxxxxxxxxx>]

Raahul Kumar via RT wrote:
> 
> --- "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?

It wasn't committed; the default ruleset remains the same.  This data 
was used for testing only.

>>>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.

This should only happen when the build_req is used.

>>   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.

But where else could they go?

>> 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.

No, it should be impossible - that is the point of the assert.

jason