[Freeciv-Dev] Re: Remove map_adjust_[xy] invocations fromserver (PR#1003)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Oct 15, 2001 at 09:23:44PM -0400, Ross W. Wetmore wrote:
> At 08:34 AM 01/10/15 +0200, Raimar Falke wrote:
> >On Mon, Oct 15, 2001 at 12:22:23AM -0400, Ross W. Wetmore wrote:
> >> At 12:53 AM 01/10/13 -0400, Jason Dorje Short wrote:
> >> >"Ross W. Wetmore" wrote:
> >> >> At 05:18 AM 01/10/12 -0400, Jason Dorje Short wrote:
> >> >> >Raimar Falke wrote:
> [...]
> >> >Correct.  What Raimar is proposing is not a substitution of a macro for
> >> >identical code, but rather "fixing" the code to always pass normal
> >> >coordinates.
> >> 
> >> The code really isn't ready for this. You haven't yet added all the fixes
> >> for the case of guaranteed realness, let alone normalized.
> >
> >The code is ready. This is the result of various patches.
> 
> Yes, Raimar. You would perhaps care to explain how this code from
> tilespec.c worked its way back into the Oct 12 CVS, maybe?
> 
>   dither[0] = get_dither(ttype, tile_is_known(x, y-1) ? ttype_north :
> T_UNKNOWN);
>   dither[1] = get_dither(ttype, tile_is_known(x, y+1) ? ttype_south :
> T_UNKNOWN);
>   dither[2] = get_dither(ttype, tile_is_known(x+1, y) ? ttype_east :
> T_UNKNOWN);
>   dither[3] = get_dither(ttype, tile_is_known(x-1, y) ? ttype_west :
> T_UNKNOWN);
> 
> And what you expect to happen when you remove the normalize_map_pos() 
> "fix" from map.c. 
> 
> /***************************************************************
> Only for use on the client side
> ***************************************************************/
> enum known_type tile_is_known(int x, int y)
> {
>   if (!normalize_map_pos(&x, &y))
>     return TILE_UNKNOWN;
>   else
>     return (enum known_type) (MAP_TILE(x, y)->known);
> }
>    
> 
> This is far from the only case.

My testing was limited. So there are still issues in the code. Good so
we aren't out of work yet ;)

> >We want to break them ;) It is ok if the private tree got broken and a
> >cleanup fix is sent to freeciv-dev. This has been done in the past. We
> >are now ready to make the final step. Jason hasn't posted the
> >CHECK_MAP_POS patch yet. But if the patch got posted I urge everybody
> >to test it.
> 
> If your goal is to break Freeciv you are certainly on the correct path :-).
> 
> FYI: I did hundreds of hours of testing both in server and client autogame
> modes. I still occasionally run across code like this, and every once in
> a while find a new core dump. When you think you are done, you should 
> check to see if you have missed anything else.
> 
> Some bugs were decidedly non-trivial to figure out as they involved
> several layers of function calls betwen the bug and the failure point.
> Moreover like the above you really only find them in special conditions
> which most games do not exercise. 
> 
> The example above is very straightforward, will happen all the time at the
> poles. Unless you have set an assert, you may not really notice the dither
> is screwed up, unless memory allocations put the map in a bad spot so the
> off map reference is bad - but this is likely a dynamic condition and won't
> really be that reproducible if so.

So you agree that we should switch to an assert and take to time till
the next release to catch the remaining bugs.

> You have the confidence of immaturity, Raimar. Just don't write any airplane 
> control code, or medical programs for a few years, ok?

A broken freeciv doesn't harm people. If there is a reproducible bug
there will be a fix in short time (12 hours).

They wouldn't allow me to write airplane control code or medical
programs because I don't know Ada.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Like the ad says, at 300 dpi you can tell she's wearing a
  swimsuit. At 600 dpi you can tell it's wet. At 1200 dpi you
  can tell it's painted on. I suppose at 2400 dpi you can tell
  if the paint is giving her a rash."
    -- Joshua R. Poulson