[Freeciv-Dev] Re: [Patch] Map coord. to city map transformation and back

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Sep 26, 2001 at 11:45:16PM -0400, Ross W. Wetmore wrote:
> At 10:16 AM 01/09/26 +0200, Raimar Falke wrote:
> >On Wed, Sep 26, 2001 at 12:17:52AM -0400, Ross W. Wetmore wrote:
> >> /**************************************************************************
> >> ...
> >> **************************************************************************/
> >> int is_valid_city_coords(const int city_x, const int city_y)
> >> {
> >>   if ((city_x == 0 || city_x == CITY_MAP_SIZE-1)
> >>    && (city_y == 0 || city_y == CITY_MAP_SIZE-1))
> >>     return 0;
> >>   if ( !RANGE_CHECK_0(CITY_MAP_SIZE, city_x)
> >>     || !RANGE_CHECK_0(CITY_MAP_SIZE, city_y) )
> >>     return 0;
> >> 
> >>   return 1;
> >> }
> >
> >Can you make a patch with RANGE_CHECK and RANGE_CHECK_0 and the
> >changes to is_valid_city_coords?
> 
> The AI cleanups are stable now, smallpox behaviour is controlled and
> the AI moves on to more advanced forms of government and growth without
> too much chaotic behaviour. It even defends itself without relying on 
> hordes of military cannon fodder to screen its cities from direct attack.
> I particularly like the fact that wounded units go to the nearest 
> friendly hospital to heal instead of limping off after the next chance 
> to suicide against something. It is still pretty easily distracted into 
> a war, but the Civs will go for Alpha Centauri without the xenophobic 
> need to first eliminate all known forms of game life.
> 
> But there are a couple minor things I want to look at before the next
> drop of the corecleanup drop. I'm shooting for this weekend.

This has nothing to do with the RANGE_CHECK and is_valid_city_coords
changes.

> Since you plan to rewrite all patches yourself, 

> it shouldn't be too much trouble to extract what you need from the
> drop instead for these little bits. Just unpack a fully patched
> version off to the side and grep through for the things you want to
> move over. It probably isn't worth my time to prepare and test
> pretty patches that don't get used, at least not until the system
> rules get clarified and followed consistently

No. We had this discussion in the past. It goes like this:
 - Ross put a huge set of patches in incoming
 - Ross says "take what you want"
 - Raimar sees something interresting which is interwoven with other
 changes
 - Raimar asks Ross if he can extract the interesting piece
 - Ross says no
 - times passes
 - Raimar does the interesting piece by itself
 - Ross says "this is like I have done it" and "why haven't you take
 the code from my patches" and "it was all there all the time in my
 patches"

If I extract the interresting piece by myself I would have the same
work and I will also post a diff the freeciv-dev for review.

It should go something like this: 
 - Ross has an interesting idea
 - if Ross has time and he likes to adventure: goto 1:
 - Ross posts an RFC to freeciv-dev
 - abort if there are negative responses
1:
 - Ross makes patch and post it
 - Raimar applies patch

> , or Freeciv development moves from the despotic to a higher form of
> government itself :-).
> 
> The other aspect is that the patches needed to make resyncing useful
> are big, or there are simply too many little ones if it is going to
> take week's of wrangling and petty adjustments to get each in. When 
> you have digested enough of the rationale and background to be more
> comfortable with what is in the corecleanups, I will help put together
> what is needed to move significant chunks of it in in a tested clean 
> way. 

> In the meantime you have the drops to look over and cull for smaller
> bits that are immediately useful.

Ross, I don't have the time. Even in the time I have free for freeciv
there are other issues like working on my personal TODO list, working
on the client side ai and working on any bug reports. I won't go
fishing in a large patch for something interesting.

> As you said, I should consider my version to be where things are going
> and just run it in tandem a while longer.

You should make small packets available to freeciv-dev after you
resolved the issue in your tree. It would be useful to make such a
packet immediately after you resolved the issue in your tree and it
works ok in your tree. Don't collect them and sent them together.

> >> /**************************************************************************
> >> Given a real map position and a city, finds the city map coordinates of the
> >> tiles.
> >> Returns TRUE (== 1) if the real map position was inside the city map.
> >> **************************************************************************/
> >> int get_citymap_xy(const struct city *pcity, const int x, const int y,
> >>            int *city_x, int *city_y)
> >> {
> >>   /* Get x,y relative to the NW corner of pcity */
> >>   *city_x= (x - pcity->x + CITY_MAP_SIZE/2);
> >>   *city_y= (y - pcity->y + CITY_MAP_SIZE/2);
> >> 
> >>   /* If coords can wrap, we can be off by +/- xsize,ysize. */
> >>   if( has_mapwrap(MAP_TYPE_WRAPX) )
> >>     *city_x= map_wrap_x(*city_x);
> >
> >Your map_wrap_x method does really work with city coordinates
> >(0,...,4)?
> 
> The wrapping factor is the map.xsize (or map.ysize when you reach that
> level). Use the standard one.
> 
> Note that the adjustment to the NW corner, i.e. x - (pcity->x - CITY..)
> means that the citymap origin is now at 0,0 and you compute the delta
> from this. 

> The wrap adjustment may move a value at -1 to map.xsize-1 in this
> coordinate system but this is rejected by the is_valid test in
> either case.

This is a bit non-intuitive.

> Only the first CITY_MAP_SIZE values in either coordinate will lie in 
> the citymap locality and can test true. All delta offsets will be +ve
> after wrapping.
> 
> Another way to think about it is that there really is no hard wrapwall
> in a wrapped world. This is something imposed on the map to reflect
> external tile storage boundaries or the need to choose some line like the 
> Greenwich meridian to start counting from. In the case of determining
> citymap coordinates it is useful to define the 0'th meridian to coincide 
> with the citymap origin.

Overall: this discussion if useless because:

  %   cumulative   self              self     total
 time   seconds   seconds    calls  ms/call  ms/call  name
...
  2.20     32.27     2.20 24840101     0.00     0.00  base_local_city_map_to_map
...
  0.20     89.65     0.20   133881     0.00     0.00  base_map_to_local_city_map
...
  0.02     98.23     0.02   133881     0.00     0.00  map_to_local_city_map
...
-----------------------------------------------
                0.00    0.00      10/133881      emergency_reallocate_workers 
[452]
                0.00    0.03   14698/133881      update_city_tile_status_map 
[108]
                0.01    0.11   56647/133881      check_cities [150]
                0.01    0.12   62526/133881      server_set_tile_city <cycle 1> 
[114]
[197]    0.3    0.02    0.26  133881         map_to_local_city_map [197]
                0.20    0.06  133881/133881      base_map_to_local_city_map 
[206]
-----------------------------------------------
                0.20    0.06  133881/133881      map_to_local_city_map [197]
[206]    0.3    0.20    0.06  133881         base_map_to_local_city_map [206]
                0.06    0.00 1471089/107846938     normalize_map_pos [38]
-----------------------------------------------

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  "Heuer's Law: Any feature is a bug unless it can be turned off."