[freeciv-ai] Re: aiair uses set_unit_activity() directly

[Per Inge Mathisen <per@xxxxxxxxxxx>]

On Mon, 30 Aug 2004, Jason Dorje Short wrote:
> Per Inge Mathisen wrote:
> > We should whenever possible gate all our server calls through the
> > appropriate handle_*() functions, instead of doing direct changes to the
> > server state.
>
> One problem is the handle_* functions generally fail silently if they
> are given invalid input.Obviously when getting data from a network
> this is correct.But when an AI is involved we'd usually like an
> assertion instead.
>
> One idea is to change code in the handle_* functions from
>
>  if (!normalize_map_pos(&x, &y)) return;
>
> to
>
>  if (!normalize_map_pos(&x, &y)) {
>    assert(pplayer->conn != NULL);
>    return;
>  }
>
> which will get the assertions to happen for AI players only

That is a little obscure, I think. It would need a comment for each
explaining this strange behaviour.

> Another idea is to have wrappers like ai_handle_* to do the work.

We already have this for movement. See ai_unit_*() in aitools.c. This is
generally the best solution, I think. Note that these functions often
duplicate the sanity checks done later in the handle function, and check
the result after the handle_*() call, doing hopefully intelligent
responses to whatever happens or does not happen.

> Yet another idea is to have the handle_* functions be the wrappers that
> do sanity checking only and then call an internal function base_handle_*
> to do the real work, with assertions.

This is also done in some places.

> Yet another idea is just to ignore the problem - but this can give
> bizarre errors (like the one where handle_unit_load() was failing
> silently, causing ferry code to crash obscurely).

This is a bad idea, and you know it ;-)

  - Per