Re: (PR#12422) stupid AI leaves its city in disorder

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

On Sat, Mar 05, 2005 at 02:42:19PM -0800, Jason Short wrote:
> You can argue that the current behavior (where cm_query_result calls 
> generic_city_refresh before "saving" the city value) is buggy.  But you 
> can also argue that any other behavior is also buggy.  If the query is 
> passed in an un-refreshed city it must do a full refresh of the city 
> before beginning any calculations.  This refresh doesn't have to be 
> "saved" (we could restore the old value afterwards) but then we have the 
> problem that the unrefreshed city is just passed on to the next caller 
> (again a bug).  The only real "solution" that doesn't have 
> cm_query_result ever clobber the city is to *require* that all cities be 
> passed in refreshed.

My claim is that it's a bug to have anything cm_query_result does be
visible except through the cm_result answer, as per the comment:

    /*
     * Will try to meet the requirements and fill out the result. Caller
     * should test result->found_a_valid. cm_query_result will not change
     * the actual city setting.
     */

So the current behaviour isn't to spec.  We can change the spec, but I
prefer changing the behaviour: anything called "query" doesn't sound
like it should visibly change the data structure I'm querying.

As for whether and when cities should be valid: yes, it does seem like a
bug that a city would be sent in invalid.  I don't think we've ever
really understood when that happens.

        -- Benoît