<URL: http://bugs.freeciv.org/Ticket/Display.html?id=35532 >

I believe the function adjust_building_wants_by_effects() may not always 
work as intendend (but then I might get the intention wrong).

For example, when the function is called for harbours, the following 
happens.  The only effect of harbours (in the default ruleset) is 
[effect_harbour].  For this effect the presence of all requirements is 
checked (except for the requirement that there is a harbour).  According 
to effects.ruleset the two requirements to be checked are (1) "Terrain, 
Ocean, Local" and (2) "OutputType, Food, Local".  These checks are done 
via is_req_active(), which always returns false in both cases.  The 
reasons are:

For (1): The tile argument given to is_req_active() is NULL.  Therefore 
the terrain requirement cannot be checked.  But even if pcity->tile is 
supplied as argument, the function is_terrain_in_range() returns false 
even if there is an ocean tile nearby. This happens because the terrain 
requirement is marked "Local", rather than "Adjacent".

For (2): Checks for OutputType requirements always fail because the 
output_type argument given to is_req_active() is NULL, too.  But 
checking for OutputType
requirements seems unnecessary at this point in the code anyway.

There are similar issues for other effects with terrain requirements.  
So I propose to

(1) change "Local" into "Adjacent" for terrain requirements in the 
default effects.ruleset (this applies to harbours and offshore platforms),

(2) supply pcity->tile as argument to is_req_active(),

(3) ignore OutputType requirements.

If, after these changes have been applied, 
adjust_building_wants_by_effects() is called for harbours, the "if 
(useful)" condition is satisfied and the [harbour_effect] is evaluated.  
However, this evaluation actually consists in ignoring the effect.  So 
for the default ruleset the behavior of the ai does not change at all 
when the proposed changes are applied. 

Nevertheless I think the changed code would be nicer because effects 
like [harbour_effect] wouldn't be ignored "by accident", but 
intentionally and in a more comprehensible way.  It would also handle 
certain custom rulesets better.

If the developers in charge agree with the above, I would happily 
provide some patches.

Sorry if this posting has a bad length-to-relevance ratio.


Freeciv-dev mailing list

Reply via email to