Re: side_controls_unit vs. tk interface

[Stan Shebs <shebs at shebs dot cnchost dot com>]

Jim Kingdon wrote:
> 
> Here's a revised patch.  It assumes that if you set a modalhandler,
> when you come back the unit will not have changed.  Hope that's true.

Actually not, that being one of the reasons to have modal handlers;
if you sit at the prompt a long time, the unit in question might
be killed or captured.  But that shouldn't be a problem, because
all the modal handlers already have in_play and side_controls_unit
tests already, and your change affects only command invocation.

Also, you're going to hate this :-), but it's not necessary to add an
arg to REQUIRE_UNIT, since the global dside is available, and
indeed already used in the macro.  If you're too annoyed to make
this additional change, just say so and I'll take care of it.

Stan

> 2000-10-05  Jim Kingdon  <http://sourceforge.net/users/kingdon>
> 
>         * tkcmd.c (REQUIRE_UNIT): Call side_controls_unit.
>         (do_distance, do_occupant, and do_recenter): Don't call
>         REQUIRE_UNIT.
>         * tkcmd.c: Add paramater side to REQUIRE_UNIT.
> 
> Index: tcltk/tkcmd.c
> ===================================================================
> RCS file: /cvs/xconq/xconq/tcltk/tkcmd.c,v
> retrieving revision 1.39
> diff -u -r1.39 tkcmd.c
> --- tkcmd.c     2000/08/24 12:21:18     1.39
> +++ tkcmd.c     2000/10/06 18:49:34
> @@ -46,12 +46,17 @@
> 
>  /* Use this macro in any command if it requires a current unit. */
> 
> -#define REQUIRE_UNIT(map)  \
> +#define REQUIRE_UNIT(side,map)  \
>    if (!in_play((map)->curunit) || (map)->curunit_id != (map)->curunit->id) {  \
>      (map)->curunit = NULL;  \
>      (map)->curunit_id = 0;  \
>      cmd_error(dside, "No current unit to command!");  \
>      return;  \
> +  } \
> +  if (!side_controls_unit (side, (map)->curunit)) { \
> +    cmd_error(side, "%s cannot control %s", \
> +             side_name(side), unit_handle(side, (map)->curunit)); \
> +    return; \
>    }
> 
>  #define DURING_GAME_ONLY()  \
> @@ -87,7 +92,7 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      u = unit->type;
>      numtypes = 0;
>      for_all_terrain_types(t) {
> @@ -198,7 +203,7 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      map->argunitid = unit->id;
>      ask_position(side, map, "Attack where?", aux_attack);
>  }
> @@ -240,7 +245,7 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      u = unit->type;
>      if (!can_build(unit)) {
>         cmd_error(side, "%s can't build anything!", unit_handle(side, unit));
> @@ -380,7 +385,7 @@
>      char *arg, *rest;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      mtocollect = NONMTYPE;
>      if (nummtypes == 0) {
>         cmd_error(side, "No materials to collect");
> @@ -413,7 +418,7 @@
>      Map *map = side->ui->curmap;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      aux_move_dir(side, map, map->curunit);
>  }
> 
> @@ -423,7 +428,7 @@
>      Map *map = side->ui->curmap;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      map->prefixarg = 9999;
>      aux_move_dir(side, map, map->curunit);
>  }
> @@ -464,7 +469,14 @@
>  {
>      Map *map = side->ui->curmap;
> 
> -    REQUIRE_UNIT(map);
> +    if (!in_play(map->curunit)
> +       || map->curunit_id != map->curunit->id) {
> +       map->curunit = NULL;
> +       map->curunit_id = 0;
> +       cmd_error(dside, "No current unit to command!");
> +       return;
> +    }
> +
>      map->argunitid = map->curunit->id;
>      ask_position(side, map, "Distance to where?", aux_distance);
>  }
> @@ -510,7 +522,7 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
> 
>      if (map->inpch == '\0') {
>        cmd_error(side, "must use keystroke");
> @@ -590,7 +602,7 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
> 
>      if (inside_area(x, y)) {
>         if (x != unit->x || y != unit->y) {
> @@ -641,7 +653,7 @@
>      Side *side2;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      if (between(0, map->prefixarg, numsides)) {
>         side2 = side_n(map->prefixarg);
>         really_do_give_unit(map->curunit, side2);
> @@ -808,7 +820,7 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      map->argunitid = unit->id;
>      ask_position(side, map, "Move to where?", aux_move_to);
>  }
> @@ -848,7 +860,7 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      map->argunitid = unit->id;
>      sprintf(tmpbuf, "New name for %s:", unit_handle(side, unit));
>      ask_string(side, map, tmpbuf, unit->name, aux_name);
> @@ -897,7 +909,14 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    if (!in_play(map->curunit)
> +       || map->curunit_id != map->curunit->id) {
> +       map->curunit = NULL;
> +       map->curunit_id = 0;
> +       cmd_error(dside, "No current unit to command!");
> +       return;
> +    }
> +
>      nextocc = find_next_occupant(unit);
>      if (nextocc != unit) {
>         set_current_unit(map, nextocc);
> @@ -1097,8 +1116,15 @@
>  do_recenter(Side *side)
>  {
>      Map *map = side->ui->curmap;
> +
> +    if (!in_play(map->curunit)
> +       || map->curunit_id != map->curunit->id) {
> +       map->curunit = NULL;
> +       map->curunit_id = 0;
> +       cmd_error(dside, "No current unit to command!");
> +       return;
> +    }
> 
> -    REQUIRE_UNIT(map);
>      recenter(map, map->curunit->x, map->curunit->y);
>  }
> 
> @@ -1128,7 +1154,7 @@
>      Unit *unit = map->curunit;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      u = unit->type;
>      numtypes = 0;
>      for_all_terrain_types(t) {
> @@ -1231,7 +1257,7 @@
>      Unit *unit, *other;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    REQUIRE_UNIT(side,map);
>      unit = map->curunit;
>      if (valid(check_repair_action(unit, unit, unit))) {
>         net_prep_repair_action(unit, unit, unit);
> @@ -1310,7 +1336,10 @@
>      Map *map = side->ui->curmap;
> 
>      DURING_GAME_ONLY();
> -    REQUIRE_UNIT(map);
> +    /* Should SIDE be able to tell a unit which isn't controlled by SIDE,
> +       but which trusts SIDE, to form up on one of SIDE's units?
> +       For now, no.  */
> +    REQUIRE_UNIT(side,map);
>      map->argunitid = map->curunit->id;
>      ask_position(side, map, "Form up on who?", aux_set_formation);
>  }