annoying new bug in movement

annoying new bug in movement

[Jim Kingdon]

In the standard game, if I have an infantry in a bomber, and the
infantry tries to move (using a movement key, like "h") out of the
bomber, sometimes it fails.  Control-e (disembark) seems to still
work, but of course that puts me in a different hex than where I
wanted to go.  This is fairly new, probably with the new path code.

By "fails" I mean that the ACP now shows "0/1" but the infantry hasn't
gone anywhere.  It doesn't even go into reserve mode, which I could
recover from.

Unless my work with the debugger was mistaken, I tracked it down to
path_get_next_move returning -1 (although that leaves me with a
followup question: why would the caller burn ACP rather than going
into reserve in that case).

And as for "sometimes", I haven't been able to figure out when and
when not.  Sometimes the bomber is over sea, sometimes land.

I'm not sure what to try next.  The path code is quite complicated....

Re: annoying new bug in movement

[Hans Ronne]

>In the standard game, if I have an infantry in a bomber, and the
>infantry tries to move (using a movement key, like "h") out of the
>bomber, sometimes it fails.  Control-e (disembark) seems to still
>work, but of course that puts me in a different hex than where I
>wanted to go.  This is fairly new, probably with the new path code.

There are indeed some transport-related problems with the new path code.
Also see my post from December 4. It is my impression that the AI seldom
disembarks units from a transport, even when it would be logical to do so.
Peter has been working hard on his code, however, and has made some
progress. I think that since we now know what caused the synch error, the
chances of fixing the remaining problems are good.

Hans

Re: annoying new bug in movement

[Eric McDonald]

Hi Jim, Lincoln, others,

On Thu, 4 Dec 2003, Jim Kingdon wrote:

> wanted to go.  This is fairly new, probably with the new path code.

Yes.
Two things:
(1) check_move_action does not check for all the same things as 
do_move_action expects.
(2) do_move_action underwent a significant revision with a patch 
Peter provided on Nov 18. I checked in the patch more recently  
(Nov 29). In particular, the trouble seems to come from this part:

-    } else if (dist == 1
-              || (dist == 2 && border_slide_possible(u2, ox, oy, 
x, y))) {

+    } else if((dir = choose_move_direction(unit2, x, y, 0, 
CMD_NONE)) >= 0) {

This changed what sort of things fell through to the "else" 
branch. Of course, there was no "else" branch; flow simply 
returned to the trunk of the function, which had:

     if (alive(unit)) {
         acpcost = max(acpcost, u_acp_to_move(u2));
         if (acpcost < 1)
           acpcost = 1;
         use_up_acp(unit, acpcost);
     }

so that ACP was being deducted whether or not the unit was 
actually being moved.

> By "fails" I mean that the ACP now shows "0/1" but the infantry hasn't
> gone anywhere.  It doesn't even go into reserve mode, which I could
> recover from.

I have put in an "else" branch to catch anything that falls 
through. It does not correctly address all the possibilities; 
choose_move_direction() and the stuff inside its branch must 
likely be improved to address them properly.

But at least now a notification is provided to let the player know 
that the movement cannot be performed, and ACP is prevented from 
being deducted in such a circumstance.

Maybe you could give things a try now.

  Thanks,
    Eric

Re: annoying new bug in movement

[Lincoln Peters]

I just tested the new patch with bolodd2.g, and found that, although an
ACP is no longer burned when a move action fails, I still get an error
when I try to make a naval unit move into water:

Your 1st boat was unable to perform requested move to 30,13

If I try to perform an illegal move (e.g. move the boat into a land
cell, or move a ground unit into a water cell), it gives me a different
error (as it has given since at least 7.4.1):

Your 1st boat can never do this!


It would seem that a bug still exists somewhere in the pathfinding
code...

-- 
Lincoln Peters <sampln@sbcglobal.net>

Re: annoying new bug in movement

[Eric McDonald]

Hi Lincoln,

On Mon, 22 Dec 2003, Lincoln Peters wrote:

> I just tested the new patch with bolodd2.g, and found that, although an
> ACP is no longer burned when a move action fails, I still get an error
> when I try to make a naval unit move into water:
> 
> Your 1st boat was unable to perform requested move to 30,13

That is the error notification that I added to the code.
As I mentioned, but perhaps was not clear, I made no serious 
attempt to isolate the underlying problem. My "fix" was just to 
deaden the pain somewhat.

I don't really feel responsible for this bug (aside from checking 
in the patch (from someone else) that likely introduced it), and 
thus am not feeling highly motivated to fix it at this point.

> Your 1st boat can never do this!

I think my error message is more descriptive. :-)

> It would seem that a bug still exists somewhere in the pathfinding
> code...

Yes. Or more generally, the movement code.

Eric

Re: annoying new bug in movement

[Lincoln Peters]

On Mon, 2003-12-22 at 20:25, Eric McDonald wrote:
> That is the error notification that I added to the code.
> As I mentioned, but perhaps was not clear, I made no serious 
> attempt to isolate the underlying problem. My "fix" was just to 
> deaden the pain somewhat.
> 
> I don't really feel responsible for this bug (aside from checking 
> in the patch (from someone else) that likely introduced it), and 
> thus am not feeling highly motivated to fix it at this point.

It is certainly less painful, now that it doesn't burn an ACP.  And I'm
still trying to find a pattern that would indicate the exact location of
the bug.

Meanwhile, I also noticed that I no longer seem to be able to issue move
commands to units if the move command would leave them inside another
unit (e.g. I tell a battledroid to move into a base).  However, I can
still have one unit occupy another if they are adjacent to each other. 
I'm not yet sure if this odd behavior is restricted to bolodd2.g or not.

-- 
Lincoln Peters <sampln@sbcglobal.net>

Re: annoying new bug in movement

[Eric McDonald]

On Mon, 22 Dec 2003, Lincoln Peters wrote:

> Meanwhile, I also noticed that I no longer seem to be able to issue move
> commands to units if the move command would leave them inside another
> unit (e.g. I tell a battledroid to move into a base).  However, I can
> still have one unit occupy another if they are adjacent to each other. 
> I'm not yet sure if this odd behavior is restricted to bolodd2.g or not.

Didn't you previously report that a unit would not take a path 
that would lead it through a potential transport, such as a city 
or a base? If so, it is probably related to this. dist > 1 
logic has changed rather significantly due to the new 
pathfinding and its attendant details.

Eric

Re: annoying new bug in movement

[Lincoln Peters]

On Mon, 2003-12-22 at 20:54, Eric McDonald wrote: 
> > Meanwhile, I also noticed that I no longer seem to be able to issue move
> > commands to units if the move command would leave them inside another
> > unit (e.g. I tell a battledroid to move into a base).  However, I can
> > still have one unit occupy another if they are adjacent to each other. 
> > I'm not yet sure if this odd behavior is restricted to bolodd2.g or not.
> 
> Didn't you previously report that a unit would not take a path 
> that would lead it through a potential transport, such as a city 
> or a base? If so, it is probably related to this. dist > 1 
> logic has changed rather significantly due to the new 
> pathfinding and its attendant details.

Yes, I did.  Although I didn't have any difficulty telling a unit to
enter a transport until now (I only had difficulty when a unit tried to
pass through a transport).

Perhaps that is the root of the problem?  Unless, of course, there are
multiple bugs at work here...

-- 
Lincoln Peters <sampln@sbcglobal.net>

Re: annoying new bug in movement

[Jim Kingdon]

> (2) do_move_action . . .
> But at least now a notification is provided to let the player know 
> that the movement cannot be performed, and ACP is prevented from 
> being deducted in such a circumstance.

I tried this with the test case I've been using, and it does have the
intended effect (now ACP is not deducted, and I get the "%s was unable
to perform requested move to %d,%d." message).

Not a full fix, but better than it was.