Re: Major bug and what to do about it (long)

[Eric McDonald <mcdonald@phy.cmich.edu>]

Hans Ronne wrote:

> To follow up, this problem already exists with the current code. If I
> switch off the AI and try to hit the same bogus unit view with a manual 'f'
> command I get the anser "No visible unit there" (and no acp is consumed).
> So probing the terrain for free is already possible, precisely because
> do_hit_unit_task tries to do a fire_at_action (or an attack_action) instead
> of a fire_into_action (or an overrun_action). Only the latter are really
> safe to use in a unit view context.

Unless this has been changed, the patch I made to the Tcl/Tk interface 
on 2003/12/29 addresses this. (This is what I was referring to earlier; 
I guess it was a comment in which I suggested the "attempted fire at" 
function, and not necessarily an email message.) If you look at the 
comments I put in the code, you can see that I was already thinking of 
ways to address the present problem back then.

2003-12-29  Eric McDonald

         Squash two firing bugs in Tcl/Tk interface with one fix.
         * tkcmd.c (common_fire_at): Get rid of call to unit_at, which
         would return the first unit, friend or foe. Instead iterate
         through view stack and pick out the first unit not on
         attacker's side, if there is any such unit. This accomplishes
         two things. First, this prevents a player from cheating by
         probing all cells in firing range, hoping to uncover an unseen
         enemy unit (the code would report "Nothing to fire at!" from
         unit_at()'s omniscient perspective rather than a non-see-all
         side's less knowledgable one). Second, this allows a player
         to fire into a cell which contains his or her own units. This
         is a mixed blessing, since friendly fire incidents can occur
         because of the way the firing code is currently set up.

RCS file: /cvs/xconq/xconq/tcltk/tkcmd.c,v
retrieving revision 1.59
retrieving revision 1.60
diff -u -r1.59 -r1.60
--- xconq/tcltk/tkcmd.c	2003/12/25 19:22:31	1.59
+++ xconq/tcltk/tkcmd.c	2003/12/29 23:57:36	1.60
@@ -624,16 +624,50 @@
  common_fire_at(Unit *unit, int x, int y)
  {
      int rslt;
-    Unit *other;
-
-    /* (should only be able to target unit if covered...) */
-    other = unit_at(x, y);
+    Unit *other = NULL;
+    UnitView *uview = NULL;
+    int methere = FALSE, minethere = FALSE;
+
+    /* Check only those units which we can see. */
+    for_all_view_stack(unit->side, x, y, uview) {
+        /* And which are not on our side. Note that we are allowed to
+           hit other trusted sides, though doing so may make them not
+           trusted for long. :-) */
+        if (unit->side != side_n(uview->siden)) {
+            other = uview->unit;
+            /* Assume we have the right target. (Bad assumption,
+               but what else can one do with the info presently 
available?) */
+            break;
+        }
+        /* If one of our units is in the cell, we make note of this, in
+           case there turns out to be no visible enemies there. */
+        else {
+            if (uview->unit == unit)
+              methere = TRUE;
+            else
+              minethere = TRUE;
+        }
+    }
+    /* Target may be a ghost (view of dead or moved unit), but the player
+       cannot know that, and hence we should proceed with a fire-into
+       action instead. */
+    /* (This code should not be in the interface. Instead the referee
+        code in the kernel should accept UnitView * instead of
+        Unit *, and perform these checks for us, and guide the
+        fire-into action to be safer in such cases. Or perhaps just run
+        an attempted_fire_at procedure, which would expend ammo, draw the
+        fire lines, and do not much else.) */
+    if (other && (!in_play(other) || (other->x != x) || (other->y != y))) {
+        common_fire_into(unit, x, y);
+        return;
+    }
      if (other == NULL) {
-	cmd_error(dside, "Nothing there to fire at!");
-    } else if (other == unit) {
-	cmd_error(dside, "You can't fire at yourself!");
-    } else if (other->side == unit->side) {
-	cmd_error(dside, "You can't fire at one of your own units!");
+        if (methere)
+          cmd_error(dside, "You can't fire at yourself!");
+        else if (minethere)
+          cmd_error(dside, "You can't fire at one of your own units!");
+        else
+	  cmd_error(dside, "Apparently nothing there to fire at!");
      } else {
  	rslt = check_fire_at_action(unit, unit, other, -1);
  	if (valid(rslt)) {


Eric