[Freeciv-Dev] Re: Cleaning up server/savegame.c

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Oct 08, 2001 at 03:01:49PM -0400, Jason Dorje Short wrote:
> Raimar Falke wrote:
> > 
> > On Mon, Oct 08, 2001 at 01:50:41PM -0400, Jason Dorje Short wrote:
> 
> > > Unfortunately this is not always the case, and the number of macros
> > > would then grow.  Additionally, in some cases during load the code skips
> > > over data if it's not present, while not in others - this would require
> > > an addional macro or extra parameter to the existing one.  In summary,
> > > it's not pretty.
> > 
> > This encoding of the special and the known field is ugly, very ugly.
> 
> Yeah.  I'm looking at another macro that would allow writing
> 
>   DEC2HEX(map_get_tile(x, y)->special, 3)
> 
> instead of
> 
>   dec2hex[(map_get_tile(x, y)->special >> 12) & 0xf]
> 
> or (what is now used)
> 
>   dec2hex[(map_get_tile(x, y)->special & 0xf000) >> 12]
> 
> But, DEC2HEX is a bit of a misnomer.
> 
> > I don't see any code which skips over data (well savegame.c is
> > huge). So I don't see the problem.
> 
> One example:
> 
> http://www.freeciv.org/lxr/source/server/savegame.c#L158
> 
> If a line is not there, it is skipped.  This is done for each line. 
> Other usages should (but do not) assert that each line exists (rather
> than being a null pointer) and possibly that it is long enough as well.

It may be possible that such lines exists or doesn't exists at
all. Yes the check at every line is silly. And yes some asserts would
be nice.

It turns out that map.n<number> is used a second time in
<http://www.freeciv.org/lxr/source/server/savegame.c#L260>. 

*looking more* It looks to me that the transformation
map_rivers_overlay_load does can be done on already read data. But
only if we have the "specials" capability. Which isn't part of the
current capabilities. mhhhh Big problem is that we can't clean up this
stuff because we don't know what kind of savegames are you there.

> > Wrong. ' ' is a valid terrain char. You have to think of another one.
> 
> No, the character is skipped entirely upon loading.  It is just included
> as a convenience so that (1) indexing can be done with the x value
> directly and (2) the file is more human-readable.  The only restriction
> is that it can't be '\0', which would terminate the string.

Yes. My mistake.

> My comment was supposed to indicate this, but apparently isn't
> descriptive enough.  It would be easier to see if the loading macro were
> also written.
> 
> If it makes you more comfortable I'll choose a different character,
> although this will make the files less human-readable.

I think a '#' would do nicely.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "- Amiga Y2K fixes (a bit late, wouldn't you say?)"
    -- Linus Torvalds about linux 2.4.0 at 4 Jan 2001