Thursday, July 20, 2023

class AppleWalls : public WallTest, UsingApple

So I had managed to get shell/walls interaction mostly fixed. I thought it would be wise to get it run 'unit tests' to ensure nothing got broken by the fix, but it would do more than a few seconds of tests before failing. In Walls/Walk test , a fairly recent addition to MapTests.cpp, for which I do not seem to have any commit where that test is a success.

The test failure will produce patterns such as

--(120,0)+<-508,0>:[-224,0]     's/7f.f, S/fe08.0, '
--(118,0)+<-512,0>     's/7f.f, S/fe08.0, '
--(116,0)+<-516,0>     's/7f.f, S/fe08.0, '

--(114,0)+<-520,0>:[-232,0]     's/77.f, S/fdf8.0, '
--(112,0)     's/77.f, S/fdf8.0, '
--(110,0):[-240,0]     's/77.f, S/fdf8.0, '
--(108,0)     's/77.f, S/fdf8.0, '
--(106,0):[-248,0]     's/6f.f, S/fdf8.0, '
--(104,0)     's/6f.f, S/fdf8.0, '
--(103,0)+<248,0>:[0,0]     's/6f.f, S/fdf8.0, '

which I, too, find cryptic. I should at least inform future self that they are from saved "last state" array, dumped on exception like assert failure during the test.
  • (%{x}, %{y}) is used to report new coordinates
  • <%{vx}, %{vy}> is used to report a new speed
  • [%{dx}, %{dy}] is used to report new delayed step

Together with the arena layout defined by walls1, it can start making sense. For instance, x=104 is the lowest valid position before entering the two-# "wall" of the upper platform, where the Gob-under-test is walking. Granted, this is arcane and confusing, and it would deserve a review fix even if it is only for me. Possibly even more confusing are the `s/%x.%x` codes reported between quotes afterwards. These are generated from ChiefInspector::report() call from the WalkerController.

  • lowercase s indicates that there has been a change of tile considered in doslopes(). hex values are the coordinates of the hotspot used to decide that.
  • uppercase S indicates that doslopes() reported we're on sloped ground. hex values are the speed defined during doslopes().

Even with that wrong report of sloped ground fixed, I still have my 'follower' capable of entering walls. It happens when we stop next to the wall, but do not cancel the 'step value still needed'. More investigation needed.

With some break getspeed and cond BREAKNO (x >> 8) < 105 && cdata[GOB_XSPEED] == -520 in an epic gdb session, I could trace precisely what happens, one GobController call after another, and how the speed, delayed steps and the like where further processed.

The issue turned to be linked to the 'maxmove computation', an engine feature added in 2020 that scans the animation commands ahead of think() calls, so that we don't ask doslopes() to check for something different than what we'll actually do, else the vertical move and the horizontal move won't match and we'll end up out of the slope. The code that decides what to do when we detect a move must be cancelled assumes that the motion debt stored in the 'delayed step' variable has already been validated by controllers. In case of failure, we're safe to move *at least there*.

That was true with the "school rush" engine, where it was helpful to ensure we actually get close enough to the walls for the testpoints to work as intended. It is no longer true if maxmove shortened some of it. How precisely this can be fixed still has needed a bit more time to be figured out.

PS: while looking for the 'inspector codes' in my 2020-notepad, I got my eyes caught by a character stating "this is contemporary to maxmove introduction"... The line below reads "hopefully, no need to tweak things (how many loops are allowed while processing animation instructions, btw) too much. All it takes is building an animation list with enough 'I_MOVETO(+1, *)' commands before it loops."

PPS: of course, fixing things here broke things there, but mostly because code was wrongly assuming that some things (like vertical position or FAIL counter)

No comments: