AGI: DG: The AGIMouse Adventure (Fan Game) freeze

[raziel-]

ScummVM 2.1.0git (Oct 10 2018 08:30:53)
Features compiled in: Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC FreeType2 JPEG PNG cloud (servers, local)

On clicking on the top right placed icons "suitcase" (inventory?) and "street map" (map?) the game and ScummVM freezes solid. No RTL, Quit or any other command possible, the ScummVM window cannot be closed anymore, cpu activity is at 100%.

I don't get a crash or assertion, but something bad happens.

This might very well be a coding error in the game itself

DG: The AGIMouse Adventure (English v1.1) (DOS/English)

AmigaOS4 - PPC - SDL - BE
gcc (adtools build 8.1.0) 8.1.0

[raziel-]

The game

[digitall]

Whether this is an engine crash / lockup, it should not lock up exiting via ScummVM GMM. This is because there is a tight loop somewhere in the AGI engine without a shouldQuit() check.

Replicated under GDB and then broke the lockup from the suitcase using CTRL-C and got a backtrace:

#0  0x00005555555db5e2 in Agi::GfxMgr::getColor (this=0x5555564f25d0, x=124, 
    y=63) at engines/agi/graphics.cpp:474
#1  0x00005555555e8f20 in Agi::PictureMgr::draw_FillCheck (
    this=0x555555a99060, x=124, y=63) at engines/agi/picture.cpp:892
#2  0x00005555555e8da9 in Agi::PictureMgr::draw_Fill (this=0x555555a99060, 
    x=59, y=107) at engines/agi/picture.cpp:861
#3  0x00005555555e8c28 in Agi::PictureMgr::draw_Fill (this=0x555555a99060)
    at engines/agi/picture.cpp:832
#4  0x00005555555e8245 in Agi::PictureMgr::drawPictureV2 (this=0x555555a99060)
    at engines/agi/picture.cpp:552
#5  0x00005555555e7c7f in Agi::PictureMgr::drawPicture (this=0x555555a99060)
    at engines/agi/picture.cpp:350
#6  0x00005555555e9158 in Agi::PictureMgr::decodePicture (this=0x555555a99060, 
    resourceNr=97, clearScreen=true, agi256=false, pic_width=160, 
    pic_height=168) at engines/agi/picture.cpp:939
#7  0x0000555555618d18 in Agi::cmdDrawPic (state=0x55555659b5b8, 
    vm=0x55555659b510, parameter=0x7ffffffb7d54 "")
    at engines/agi/op_cmd.cpp:1158
#8  0x000055555561cf53 in Agi::AgiEngine::runLogic (this=0x55555659b510, 
    logicNr=97) at engines/agi/op_cmd.cpp:2413
#9  0x0000555555618ae6 in Agi::cmdCall (state=0x55555659b5b8, 
    vm=0x55555659b510, parameter=0x7ffffffb7e55 "a")
    at engines/agi/op_cmd.cpp:1122
#10 0x0000555555618b97 in Agi::cmdCallF (state=0x55555659b5b8, 
    vm=0x55555659b510, parameter=0x7ffffffb7eb4 "")
    at engines/agi/op_cmd.cpp:1133
#11 0x000055555561cf53 in Agi::AgiEngine::runLogic (this=0x55555659b510, 
    logicNr=0) at engines/agi/op_cmd.cpp:2413
#12 0x000055555561024a in Agi::AgiEngine::interpretCycle (this=0x55555659b510)
    at engines/agi/cycle.cpp:149

Will investigate and see if I can patch this lockup...

[m-kiewitz]

Does it freeze when using the original interpreter?

[m-kiewitz]

Ah, I guess there may be some tight inner loop waiting for a global to change, which may never happen and thus freezing.

I added some code to get through such inner loops, when the code is waiting for a tick change. This way require something similar.

[raziel-]

Tried with DOSBox and the orignal interpreter, no freeze.

And the "Map" is actually the Menu :-)

[digitall]

I did some further checking and this is not stuck in a tight loop, but something is hung up, likely as m-kiewitz indicates. However, I am concerned that this manages to lock ScummVM to the point of not allowing exit or access to GMM; that is not good and the engine should be fixed to allow that, even if there is a game data bug.

[digitall]

Right... Have worked out a patch which fixes the lockup and fixes the GMM and exiting as well. Not sure if it is the "right" answer though, so would be good if @m-kiewitz could review:

diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp
index 2b3bba89db..33d34dd831 100644
--- a/engines/agi/picture.cpp
+++ b/engines/agi/picture.cpp
@@ -883,6 +883,8 @@ int PictureMgr::draw_FillCheck(int16 x, int16 y) {
        byte screenColor;
        byte screenPriority;
 
+       ((AgiEngine *)_vm)->processScummVMEvents();
+
        if (x < 0 || x >= _width || y < 0 || y >= _height)
                return false;

[digitall]

The issue is basically that ScummVM events are not processed during the AGI engine main loop except in some very limited locations which are not always in the main game loop. I think this is the right solution, but not sure where the best location for this call in the AGI engine code would be.

[m-kiewitz]

That source location is really weird, because that's the one for drawing background pictures. There definitely should not be an events call.

I really wonder what's going wrong.
Could be some tight inner loop somehow.

[m-kiewitz]

The main loop is processing script code until a frame is basically done.
That's how original AGI also did its job too. And original AGI also didn't read from the keyboard all the time.

There really has to be some script code that waits for some event and it doesn't trigger in ScummVM and thus creates some kind of endless script code loop.

I had to write some detection code for tight inner loops that wait for the timer count to change, which of course never happened because the timer count is updated by regular code in ScummVM and was incremented by an IRQ in DOS. Maybe it's even something like that and the detection simply fails to trigger and thus timer isn't incremented.

[raziel-]

Maybe another game who shows the same behaviour (but does not freeze ultimately) will help?

It's from the same author (Robin Gravel) and on clicking the menu item will make the screen shake (why, i don't really know). It will sit there shaking for about 30-40 seconds (without a chance to input anything) and finally show the menu.

I was about to add this game to the original report, but as it's not freezing, i left it out.

There is a third game, which does this too, but i have to look it up again.

The game is "Half Death: Terror at White-Mesa", i'll attach a save game as well, as the intro is rather long and boring.

[raziel-]

Half Death

[raziel-]

Half Death savegame

[raziel-]

Here you go:
Naturette 2 and
Isabella Coq

Both feature that "AGI Mouse Interface"

but those work perfectly fine (except that little wait when returning to the game --> "Back")

[raziel-]

Isabella

[raziel-]

Naturette 2

[raziel-]

Referencing #12744 as it prevents from checking if this crash still applies.

[raziel-]

ScummVM 2.3.0git (Aug 25 2021 00:46:01)
Features compiled in: Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC A/52 FreeType2 FriBiDi JPEG PNG GIF cloud (servers, local) TinyGL OpenGL

Still the same freeze and ScummVM lockup with todays build

[sluicebox]

This is a bug in our AGIMOUSE implementation. It is unrelated to picture drawing, but it *is* related to event processing, which is why the earlier patch happened to exit the infinite loop even though that patch is in an unrelated area of code.

This is not a script bug; this infinite loop would have happened even with the sample AGIMOUSE script that Brian provided in 2001: ​https://web.archive.org/web/20011127012952/http://agisci.cjb.net/

// Is mouse in the area of the button?
if(mouse_y > 32 && mouse_y < 68 && mouse_x > 56 && mouse_x < 103) {
  // Is the mouse button pressed
  if(mouse_button == mb_left) {
    // If it is, draw the mouse button down, then jump to room 3
    set.loop(o2,2);
    new.room(3);
  } else {
    // Otherwise, draw the button as a mouse over graphic
    set.loop(o2,1);
  }
} else {
  // Draw the mouse button in normal state
  set.loop(o2,0);
}
draw(o2);

The problem is that we are not updating mouse state when changing rooms; specifically, the mouse button state. Just like the sample code above, this game checks to see if the mouse is clicked over a button, and if so, it calls new.room. We don't reset the button state, so it sees the same click on the next cycle, calls new.room again, ......

Investigating this five year old ticket led me to discover that we apply AGIMOUSE behavior to all games, even if they don't use AGIMOUSE. "Won't that break other games?" It sure will! It's the cause of this three year old ticket where Phil's Quest instantly game-overs: #12747 .

The solution will be to either update input on new.room, or to reset the mouse state on new.room, or something like that. I have an AGIMOUSE PR pending for the other bug, and once that's done I'll continue with this.

[sluicebox]

Fixed in ​https://github.com/scummvm/scummvm/commit/44373c95a4b4a1b4853a5b20a12e0c036086a8e9