[Freeciv-Dev] Re: Reproducable core dump (PR#1051)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

Gregory Berkolaiko wrote:

>  --- jdorje@xxxxxxxxxxxxxxxxxxxxx wrote: 
>
> > Gregory Berkolaiko wrote
> >
> >
> > > --- jdorje@xxxxxxxxxxxxxxxxxxxxx wrote: 
> > >
> > > > Gregory Berkolaiko wrot
> > My current implementation isn't completely minimalist: it will refuse
> > to fly through unknown tiles but will happily fly through known, fogged
> > tiles assuming no enemy is there.  Is this wise?  Really, a fogged tile
> > is just as likely to contain an enemy as an unknown tile.
> Exactly.
> But Raimar is right, it would be good to make it a constant, something
> AIR_ASSUMES_UNKNOWN_SAFE
> and
> AIR_ASSUMES_FOGGED_SAFE

I'm not too happy with adding constants up in a header file that will 
only be used in this one function.  If it's the preferred way to do 
things, though, I'll do it.  Where would the enumeration be placed?

> We should also introduce a check for ai.control: if the plane is
> controlled by AI (which doesn't happen now but will be surely added at
> some point), it wouldn't care if the tile is not seen -- AI cheats and
> peeks under fog.
> But for that we should also introduce a constant
> AI_SEES_ALL
> and later it should probably become a variable in struct player.

This I disagree with.  True, the current AI cheats by looking under the 
fog of war, but isn't that something to fix rather than promote?
If you must go this route, one idea would be to add a new server 
variable, aicheats, that is user-settable.  Enabling it would up the 
difficulty.  But I don't think any of the AI's (no matter how difficult) 
should cheat by default.

> > The plane is a problem in particular because it has limited feul.  Does
> > this mean it should rush to its destination, or be careful that it 
> > doesn't run out of feul?  A plane could try to fly through a straight 
> > gap only to find its way completely blocked by enemy units, and not
> > have enough feul to make it back to a safe point.  But I don't really 
> > see that this is avoidable.
> A manually controlled plane is equally liable to get trapped like this.

True, but that would be the controller's choice.  But I don't have a 
particular problem with this - the chances of getting trapped are in 
general very slim.

> > How about this version?  I improve the A* search even more, and make 
> > most of the other changes you suggest.  A new function, 
> > airspace_looks_usable(), is created.
> v.g. idea
>
> /**************************************************************************
>
> > + Returns true if the given map position has airspace that _looks_
> > + usable to the player.  The airspace is unusable if the player
> > + believes there is an enemy unit on it.  This is tricky, since we have
> > to
> > + consider what the player knows/doesn't know.
> +**************************************************************************/
>
> > +int airspace_looks_usable(int x, int y, struct player *pplayer)
> > +{
> > +  struct tile * ptile = map_get_tile(x, y);
> > +
> > +  /* If the tile's unknown, we assume/hope it's empty. */
> > +  if (!map_get_known(x, y, pplayer)) {
> if (!map_get_known(x, y, pplayer) 
> 	&& (!pplayer->ai.control || !AI_SEES_ALL) 
> 	&&  AIR_ASSUMES_UNKNOWN_SAFE)
> I am sure you can find more elegant for though

The implementation of the function is unnecessarily verbose, but since 
it's so short anyway I don't consider that a problem.  The important 
thing is that the logic be clear.  Should I shorten it into one long 
"if" statement?
If AIR_ASSUMES_UNKNOWN_SAFE is used, how can we differentiate between 
land and sea tiles?


> /* Planes _always_ use SINLGE_MOVE a time so here we can just use 1
> instead of SINGLE_MOVE */

Wait, is SINGLE_MOVE 1 for planes?  Should I use that instead?


> > +  return -1;
> > }
> verdict: 90% ready
>
> Sorry about nitpicking though ;)

No problem...

I'll make another patch as soon as we decide these issues.

jason