[Scummvm-git-logs] scummvm master -> 8485fdca7e5155307de58091e958a4b7c51437a1

[athrxx]

This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
8485fdca7e KYRA: (MR) - really fix animation glitch (bug #11312)


Commit: 8485fdca7e5155307de58091e958a4b7c51437a1
    https://github.com/scummvm/scummvm/commit/8485fdca7e5155307de58091e958a4b7c51437a1
Author: athrxx (athrxx at scummvm.org)
Date: 2020-01-15T17:07:25+01:00

Commit Message:
KYRA: (MR) - really fix animation glitch (bug #11312)

The invalid table access takes place in several more locations. I missed the location that is actually responsible for the reported bug in my last fixing attempt (due to the fact that this bug is based on undefined behavior that results from the invalid mem access: in MSVC the bug is nonexistent, with GCC builds it seems to depend on whether they're optimized or not). The location that actually requires the fix is in KyraEngine_MR::enterNewSceneUnk2(). I have now fixed all locations where the table can be incorrectly accessed. The other locations have been marked with comments.

Changed paths:
    engines/kyra/engine/kyra_mr.cpp
    engines/kyra/engine/scene_mr.cpp
    engines/kyra/script/script_mr.cpp
    engines/kyra/sequence/sequences_mr.cpp
    engines/kyra/text/text_mr.cpp


diff --git a/engines/kyra/engine/kyra_mr.cpp b/engines/kyra/engine/kyra_mr.cpp
index b489b0f..51b7d68 100644
--- a/engines/kyra/engine/kyra_mr.cpp
+++ b/engines/kyra/engine/kyra_mr.cpp
@@ -1202,6 +1202,7 @@ void KyraEngine_MR::makeCharFacingMouse() {
 		_mainCharacter.facing = 5;
 	else
 		_mainCharacter.facing = 3;
+	// _mainCharacter.facing can not be 0xFF here, so this is save.
 	_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
 	updateCharacterAnim(0);
 	refreshAnimObjectsIfNeed();
diff --git a/engines/kyra/engine/scene_mr.cpp b/engines/kyra/engine/scene_mr.cpp
index 882acd5..7b59a9a 100644
--- a/engines/kyra/engine/scene_mr.cpp
+++ b/engines/kyra/engine/scene_mr.cpp
@@ -266,7 +266,9 @@ void KyraEngine_MR::enterNewSceneUnk1(int facing, int unk1, int unk2) {
 void KyraEngine_MR::enterNewSceneUnk2(int unk1) {
 	_savedMouseState = -1;
 	if (_mainCharX == -1 && _mainCharY == -1 && !unk1) {
-		_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
+		if (_mainCharacter.facing == 0xFF)
+			debugC(3, kDebugLevelSprites, "KyraEngine_MR::enterNewSceneUnk2(): Triggered WORKAROUND for invalid character facing");
+		_mainCharacter.animFrame = (_mainCharacter.facing == 0xFF) ? 0 : _characterFrameTable[_mainCharacter.facing];
 		updateCharacterAnim(0);
 		refreshAnimObjectsIfNeed();
 	}
@@ -445,13 +447,9 @@ void KyraEngine_MR::initSceneAnims(int unk1) {
 	AnimObj *obj = &_animObjects[0];
 
 	if (_mainCharacter.animFrame != 87 && !unk1) {
-		if (_mainCharacter.facing == 0xFF) {
-			// Fix for bug #11312 (café scene)
+		if (_mainCharacter.facing == 0xFF)
 			debugC(3, kDebugLevelSprites, "KyraEngine_MR::initSceneAnims(): Triggered WORKAROUND for invalid character facing");
-			_mainCharacter.animFrame = 0;
-		} else {
-			_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
-		}
+		_mainCharacter.animFrame = (_mainCharacter.facing == 0xFF) ? 0 : _characterFrameTable[_mainCharacter.facing];
 	}
 
 	obj->enabled = true;
@@ -649,7 +647,7 @@ int KyraEngine_MR::trySceneChange(int *moveTable, int unk1, int updateChar) {
 	}
 
 	if (updateChar)
-		_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
+		_mainCharacter.animFrame = (_mainCharacter.facing == 0xFF) ? 0 : _characterFrameTable[_mainCharacter.facing];
 
 	updateCharacterAnim(0);
 	refreshAnimObjectsIfNeed();
diff --git a/engines/kyra/script/script_mr.cpp b/engines/kyra/script/script_mr.cpp
index 1051fcc..f06b263 100644
--- a/engines/kyra/script/script_mr.cpp
+++ b/engines/kyra/script/script_mr.cpp
@@ -73,7 +73,7 @@ int KyraEngine_MR::o3_refreshCharacter(EMCState *script) {
 		_mainCharacter.facing = facing;
 
 	if (frame >= 0 && frame != 87)
-		_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
+		_mainCharacter.animFrame = (_mainCharacter.facing == 0xFF) ? 0 : _characterFrameTable[_mainCharacter.facing];
 	else
 		_mainCharacter.animFrame = 87;
 
@@ -91,7 +91,7 @@ int KyraEngine_MR::o3_getMalcolmsMood(EMCState *script) {
 
 int KyraEngine_MR::o3_getCharacterFrameFromFacing(EMCState *script) {
 	debugC(3, kDebugLevelScriptFuncs, "KyraEngine_MR::o3_getCharacterFrameFromFacing(%p) ()", (const void *)script);
-	return _characterFrameTable[_mainCharacter.facing];
+	return (_mainCharacter.facing == 0xFF) ? 0 : _characterFrameTable[_mainCharacter.facing];
 }
 
 int KyraEngine_MR::o3_setCharacterFacing(EMCState *script) {
@@ -109,7 +109,7 @@ int KyraEngine_MR::o3_showSceneFileMessage(EMCState *script) {
 int KyraEngine_MR::o3_setCharacterAnimFrameFromFacing(EMCState *script) {
 	debugC(3, kDebugLevelScriptFuncs, "KyraEngine_MR::o3_setCharacterAnimFrameFromFacing(%p) ()", (const void *)script);
 	updateCharPal(0);
-	_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
+	_mainCharacter.animFrame = (_mainCharacter.facing == 0xFF) ? 0 : _characterFrameTable[_mainCharacter.facing];
 	updateCharacterAnim(0);
 	refreshAnimObjectsIfNeed();
 	return 0;
@@ -673,7 +673,7 @@ int KyraEngine_MR::o3_enterNewScene(EMCState *script) {
 	_unk5 = 1;
 
 	if (_mainCharX == -1 || _mainCharY == -1) {
-		_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
+		_mainCharacter.animFrame = (_mainCharacter.facing == 0xFF) ? 0 : _characterFrameTable[_mainCharacter.facing];
 		updateCharacterAnim(0);
 	}
 	_screen->showMouse();
@@ -700,7 +700,7 @@ int KyraEngine_MR::o3_setMalcolmPos(EMCState *script) {
 	if (_mainCharX == -1 && _mainCharY == -1)
 		_mainCharacter.animFrame = 87;
 	else
-		_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
+		_mainCharacter.animFrame = (_mainCharacter.facing == 0xFF) ? 0 : _characterFrameTable[_mainCharacter.facing];
 
 	return 0;
 }
diff --git a/engines/kyra/sequence/sequences_mr.cpp b/engines/kyra/sequence/sequences_mr.cpp
index abfd0d8..a028da5 100644
--- a/engines/kyra/sequence/sequences_mr.cpp
+++ b/engines/kyra/sequence/sequences_mr.cpp
@@ -80,6 +80,7 @@ void KyraEngine_MR::showBadConscience() {
 			_mainCharacter.facing = 3;
 		else
 			_mainCharacter.facing = 5;
+		// _mainCharacter.facing can not be 0xFF here, so this is save.
 		_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
 	}
 
diff --git a/engines/kyra/text/text_mr.cpp b/engines/kyra/text/text_mr.cpp
index 8400700..17648db 100644
--- a/engines/kyra/text/text_mr.cpp
+++ b/engines/kyra/text/text_mr.cpp
@@ -225,6 +225,7 @@ void KyraEngine_MR::objectChat(const char *str, int object, int vocHigh, int voc
 	int chat = talkScriptTable[chatType + _mainCharacter.facing * 4];
 	objectChatProcess(talkFilenameTable[chat]);
 	_text->restoreScreen();
+	// _mainCharacter.facing can not be 0xFF here, so this is save.
 	_mainCharacter.animFrame = _characterFrameTable[_mainCharacter.facing];
 	updateCharacterAnim(0);
 	_chatText = 0;