[Freeciv-Dev] Re: (PR#8852) savefile positions should all be native

["rwetmore@xxxxxxxxxxxx" <rwetmore@xxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8852 >



Jason Short wrote:

> <URL: http://rt.freeciv.org/Ticket/Display.html?id=8852 >
> 
> All tile positions in savefiles should be in the same coordinate system. 
>   Otherwise it's insanely hard to edit the savefile/scenario manually.
> 
> Map positions are no good at all.  The choice is between native and 
> natural positions.  Natural positions are more intuitive on the local 
> scale but native positions work better on the global scale.

Absolutely wrong!

You really need to stop trying to change things on casual inspection without 
stopping to understand the underlying rules and standardizing procedures.

Map positions are the core way to transmit all coordinates and the default
arguments for all functions. These are always he default for such parameters.

Natural should *never* be used except in specialized local settings, and
even native that are the main form for save games and such usage are
probably not appropriate in this case.

> I talked to a scenario author and he said native positions were fine. 
> Since we already have some native positions I think this will do.

Wrong person to talk to.

> So the remaining map positions should be converted to native.

No!!!

Don't fix things that aren't broken and propose an RFC for discussion of 
significant changes to things that have backwards compatibility and many 
other ramifications.

> As for compatability, there are two options:
> 
> - Add a capabity and add checks in several places.
> 
> - Don't add a capability.  If this is done backwards-compatability with 
> old iso-maps will be lost.  Of course there aren't very many old 
> iso-maps so this may be acceptable.
> 
> The attached patch demonstrates how this can be done for starting 
> positions.  Of course there are other places that need to be changed.
> 
> jason

Cheers,
RossW
=====

> ------------------------------------------------------------------------
> 
> Index: server/savegame.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
> retrieving revision 1.155
> diff -u -r1.155 savegame.c
> --- server/savegame.c 27 May 2004 22:14:19 -0000      1.155
> +++ server/savegame.c 27 May 2004 22:54:05 -0000
> @@ -309,11 +309,15 @@
>                                  map.num_start_positions
>                                  * sizeof(*map.start_positions));
>    for (i = 0; i < map.num_start_positions; i++) {
> +    int nat_x, nat_y;
>      char *nation = secfile_lookup_str_default(file, NULL, "map.r%dsnation",
>                                             i);
>  
> -    map.start_positions[i].x = secfile_lookup_int(file, "map.r%dsx", i);
> -    map.start_positions[i].y = secfile_lookup_int(file, "map.r%dsy", i);
> +    nat_x = secfile_lookup_int(file, "map.r%dsx", i);
> +    nat_y = secfile_lookup_int(file, "map.r%dsy", i);
> +
> +    native_to_map_pos(&map.start_positions[i].x, &map.start_positions[i].y,
> +                   nat_x, nat_y);
>  
>      if (nation) {
>        /* This will fall back to NO_NATION_SELECTED if the string doesn't
> @@ -480,8 +484,11 @@
>    secfile_insert_bool(file, game.save_options.save_starts, 
> "game.save_starts");
>    if (game.save_options.save_starts) {
>      for (i=0; i<map.num_start_positions; i++) {
> -      secfile_insert_int(file, map.start_positions[i].x, "map.r%dsx", i);
> -      secfile_insert_int(file, map.start_positions[i].y, "map.r%dsy", i);
> +      do_in_native_pos(nat_x, nat_y,
> +                    map.start_positions[i].x, map.start_positions[i].y) {
> +     secfile_insert_int(file, nat_x, "map.r%dsx", i);
> +     secfile_insert_int(file, nat_y, "map.r%dsy", i);
> +      } do_in_native_pos_end;
>  
>        if (map.start_positions[i].nation != NO_NATION_SELECTED) {
>       const char *nation = get_nation_name(map.start_positions[i].nation);