[Freeciv-Dev] Re: [patch]full fog of war

[Thue Janus Kristensen <thue@xxxxxxx>]

> I looked at it fairly quickly.  It looks pretty good, but I did notice a
> few things:

He said, and I was hopefull...

> - Capability string missing.

I don't  think it is needed; but with the changes I need to make it would soon
be needed, I think.
I have made the savegames made with unit fog of war incompatible (1.10
savegames will work, of course). Is this ok?
I could make them compatible, but I don't want to clutter the format for
compatibility with 3 weeks worth of CVS savegames.

> - Some headers are missing from new routines.  Even if the header is empty
> (as so many are), it is good to have the header as a marker of the
> beginning of each routine.
> 
> - There are some long lines.  It's nice to break long lines before 80 or 90
> columns.
> 
> - I think I noticed a mis-ordered #include in maphand.c and plrhand.c.
> 
> - The two new routines declared at the top of maphand.c should be declared
> static.
> 
> - I believe a debug printf() was left in unithand.c.  Another in cityhand.c.
> 
> - Description of fogofwar server option needs to be updated.
> 
> - I ran an all-AI game, and had some "Get_def bugged at (X,Y)" errors
> reported by the server.  There don't seem to be cities at the reported X,Y
> -- did you change the way unit information is reported to or used by the
> AIs?  It's like the AI is trying to attack a place where a unit was but no
> longer is (just a guess).  Eventually I get a thousand or so of these
> messages.

Hmm; the ai should be unaffected.

> - If spy tries industrial sabotage without first doing an investigate city,
> she doesn't see the correct set of city improvements.

I should have seen this coming myself...

> - If I load an old save-game, set fogofwar to 1 immediately, then connect
> and start the game, I see all the infrastructure in the discovered world,
> but only a random subset of cities.  What should happen, and why is it
> currently not either all or none?  Humm...  It seems I can see all of some
> nations, but none of others.

simple enought error; Du'h

> - This may be related to the previous bug, but if I load the save file, set
> fogofwar to 1, then connect and start, then set fogofwar to 0, then end the
> turn, the fog goes away and I can see all the units -- but I can't see any
> city that I couldn't see before!  After a second turn-done, all the
> invisible cities pop into view.
> 
> - Third in the series...  Setting fogofwar back to 1 from 0 doesn't hide
> the cities that I shouldn't know about.

It would be tricky to hide the cities; I think it will remain that if you have
seen it once it doesn't later disappear. (thought you will get no updates)

> - Keying the loading of player maps off of the fogofwar value in the save
> file may not be correct.  If the file was saved with the new code, it will
> have the player map regardless of the setting of fogofwar.

The way fog of war is turned off is by adding 1 to the seen value of all tiles.
And then just let the code run as usual. barring errors no tile will ever get
down to 0. I think it is quite need using the same code this way. (and allows
ingame fog toggling)
This has the consequence that player maps exist regardless of fogofwar setting.
So I simply save them. So this is not an error.
You could, however, add a little exstra code and avoid saving them (they
contain no info), so having a smaller savegame size.
My initial thought is that it isn't worth it. comments?

> - Also, I occasionally see "square X,Y has a seen value > 60000 (wrap)"
> errors.  So far, this only happened when I was toggling the fogofwar server
> option.

Umm; debug time for me :)


Thanks for testing it. I did start out by saying that it wasn't rigorously
tested. :P
And the bugs just kept on coming...
Looks like I should have run some more ai test games.
I'll get 'em squished!

-Thue