AGOS: SIMON2: Character animation regression at MucSwamplings

[dwatteau]

I'm seeing a Simon 2 regression between ScummVM 2.6.0 and current ScummVM 2.7.0git, on any platform.

1. Install French Simon the Sorcerer 2 from GOG Galaxy (by the way, this now causes a fatal setup.shr error out-of-the-box, unless you manually change the Music device in the Audio settings)
2. Skip the intro cutscenes
3. Head to MucSwamplings and watch the cutscene when Simon goes there for the first time

In current ScummVM 2.7.0git, Simon will gitch a bit and then two Simons will appear for a while (see the attached screenshot), which wasn't an issue in earlier ScummVM releases. This happens whether subtitles are enabled or not.

After a git bisect, commit eb851041edcef20d85f74c95d53e8c280fe8a0ef ("AGOS: SIMON2: Fix pirate dialogue freeze", PR ​https://github.com/scummvm/scummvm/pull/4303 and Trac #5856) appears to be related to this regression, for some reason.

[dwatteau]

Glitched Simon at MucSwamplings in ScummVM 2.7.0git

[BLooperZ]

Seems the following workaround resolves it, but I can't figure out why

diff --git a/engines/agos/vga.cpp b/engines/agos/vga.cpp
index 3c79f9d3b40..132456b5b8a 100644
--- a/engines/agos/vga.cpp
+++ b/engines/agos/vga.cpp
@@ -801,7 +801,7 @@ void AGOSEngine::vc15_sync() {
                }
        }

-       if (id != 200)
+       if (id != 200 || _lastVgaWaitFor == 1150)
                _lastVgaWaitFor = id;
        /* clear a wait event */
        if (id == _vgaWaitFor)

So far, the problem fixed in the linked bug #5856 happens because WAIT_SYNC 2417 called after SYNC 2417 was already set, but is overshadowed by immediately previous WAIT_SYNC 200, which is ignored in AGOSEngine::waitForSync so it seems safe, when disabling the guard in the change above completely, the glitch does not occur, but the pirate dialogue freezes again.

During my attempts I have tried using several data structures to keep track on previous values and restore them, but none of those attempts works.

[BLooperZ]

Might be useful as well (custom script dump of the glitch parts)

McSwampy (glitch between {He's gone commercial!} and {I do not believe it...}
==> (0x47) PROCESS 50
	(0x99) SET_BIT 28
	(0xa2) PRINT_STR 1 0 34353 5607 // {My God!}
	(0x2a) LET 101 6
	(0x47) PROCESS 85
	(0x77) WAIT_SYNC 200
	(0x9a) CLEAR_BIT 28
	(0xa2) PRINT_STR 1 0 34354 65535 // {He's gone commercial!}
	(0x77) WAIT_SYNC 200
	(0xbe) WAIT_FOR_MARK 15
	(0x2a) LET 101 6
	(0x47) PROCESS 90
	(0x77) WAIT_SYNC 1150
	(0xa2) PRINT_STR 1 0 34355 5608 // {I do not believe it...}
	(0x47) PROCESS 84
	(0x77) WAIT_SYNC 200
	(0x47) PROCESS 4123

Pirate (glitch between {At five past three, you shall all walk the plank into the dolphin infested waters and be lead to your deaths.} and {That OK with you is it?}
==> (0x47) PROCESS 1522
	(0xa2) PRINT_STR 1 1 10 1513 // {Good.}
	(0x62) ANIMATE 24 123 4 0 0 4
	(0x77) WAIT_SYNC 200
	(0x62) ANIMATE 24 127 4 0 0 4
	(0x77) WAIT_SYNC 2416
	(0x47) PROCESS 1522
	(0x2a) LET 50 1
	(0x99) SET_BIT 28
	(0xa2) PRINT_STR 1 1 33463 1514 // {Right, just to recap.}
	(0x62) ANIMATE 24 115 4 0 0 4
	(0x77) WAIT_SYNC 200
	(0x9a) CLEAR_BIT 28
	(0xa2) PRINT_STR 1 1 33464 65535 // {At five past three, you shall all walk the plank into the dolphin infested waters and be lead to your deaths.}
	(0x77) WAIT_SYNC 200
	(0x77) WAIT_SYNC 2417
==> (0x47) PROCESS 1522
	(0xa2) PRINT_STR 1 1 33465 1515 // {That OK with you is it?}
	(0x62) ANIMATE 24 123 4 0 0 4
	(0x77) WAIT_SYNC 200
	(0x47) PROCESS 1529
	(0xa2) PRINT_STR 1 2 194 1516 // {Yes, cap'n.}
	(0x62) ANIMATE 24 280 4 0 0 6
	(0x77) WAIT_SYNC 200
	(0x47) PROCESS 1522
	(0x99) SET_BIT 28
	(0xa2) PRINT_STR 1 1 33466 1517 // {No problem with that is there?}
	(0x62) ANIMATE 24 123 4 0 0 4
	(0x77) WAIT_SYNC 200
	(0x9a) CLEAR_BIT 28
	(0xa2) PRINT_STR 1 1 33467 65535 // {We're not breaking any regulations or anything?}
	(0x77) WAIT_SYNC 200

[JiFish]

This bug seems to be responsible for dozens of animation errors across Simon 2, where sprites are duplicated. In some cases both the sprites are animated. The easiest one to find is right at the start of the game when simon is exiting from the back of Calypso's Emporium. But also, mother bear in the 3 bears cottage when you are picking up the porridge; climbing down the sewer drain; talking to the king; and many, many more I have forgotten about.

It occurs so frequently that playing Simon 2 is honestly a pretty bad experience.

I have reverted to version 2.6.0, where I can confirm this does not occur.

Is there any chance we can get ​https://github.com/scummvm/scummvm/pull/4303 reverted in the short term. The introduced problem is much worse than the one it is fixing.

[dwatteau]

@JiFish: Thanks for doing a more thorough test of the various character animation regressions. This PR has been reverted for now for the next 2.8.0 daily builds.

This means that ticket #5856 is true again, so I'm going to reopen it.