<URL: http://bugs.freeciv.org/Ticket/Display.html?id=18010 >

I've tracked the problem to:
  r11378 | jdorje | 2005-12-23 -- PR#14365, battlegroups, and
  r11391 | jdorje | 2005-12-26 -- PR#14992, unitlists.

The code is still almost identical in both S2_1 and trunk.

There are quite a few problems with various degrees of seriousness.

  1) The tiles array is uninitialized; its state is indicated by is_init.
     Would make more sense to have initially NULL.  Would also make more
     sense to use fc_calloc, instead of whole_map_iterate() setting zero.

     (It seems horribly wasteful to have such a large array, one for every
     possible tile and direction, but that's for another time.)

  2) Another semantic duplication: is_active appears to track
     goto_map_list_size(goto_maps) > 0, with duplicated assert().

  3) Each unit is saved in the map by id, and converted back to a pointer.
     Would make more sense to save the pointer in the first place.  Is
     this a workaround for units that are destroyed during goto selection?

  4) As already discovered, is_goto_valid seems to be based on the final
     unit in the goto_map_iterate() list.  It doesn't work, as some units
     can be valid, and others invalid.

  5) In addition, connect_initial is also based on the final unit in the
     goto_map_iterate() list.  It seems that the value saved in each path
     is based on the previous unit(s).  Makes no sense.

  6) The parameter of is_valid_goto_destination(ptile) is unused.  Clear
     evidence of partial implementation.

  7) There are an awful lot of useless asserts.  For example, just after
     goto_map_new(), where num_parts is set to 0, an assert checks that
     it's zero.  More evidence of implementation problems in the past,
     and attempts at finding the solution.

  8) There are "waypoints" that cannot possibly work for battlegroups that
     are not already in the same stack.  Moreover, there is no method for
     communicating waypoints to the server.  Such waypoints could become
     obsolete as roads are connected during the following turns.

  9) The code seems to contemplate multiple paths drawn simultaneously,
     but doesn't work correctly.

Freeciv-dev mailing list

Reply via email to