AGI: Add detection+workaround for endofloop+motion at the same time

Detects when end.of.loop + motions are used on the same screen
object at the same time, which would have resulted in flag
corruption in the original interpreter. We detect this
situation now, show a warning and disable the cycler in case
cycler was activated first.
This solves a new issue in kq1, when grabbing the eagle in room
22, that was previously hidden just like in the original AGI.
Fixes bug #7046

3 files changed, 65 insertions, 0 deletions

diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index c5db1dfc1d..6540eea0f5 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -926,6 +926,8 @@ private:
 	void checkMotion(ScreenObjEntry *screenObj);
 
 public:
+	void motionActivated(ScreenObjEntry *screenObj);
+	void cyclerActivated(ScreenObjEntry *screenObj);
 	void checkAllMotions();
 	void moveObj(ScreenObjEntry *screenObj);
 	void inDestination(ScreenObjEntry *screenObj);
diff --git a/engines/agi/motion.cpp b/engines/agi/motion.cpp
index 7f49028701..8aae0e5924 100644
--- a/engines/agi/motion.cpp
+++ b/engines/agi/motion.cpp
@@ -62,6 +62,57 @@ void AgiEngine::changePos(ScreenObjEntry *screenObj) {
 	}
 }
 
+// WORKAROUND:
+// A motion was just activated, check if "end.of.loop"/"reverse.loop" is currently active for the same screen object
+// If this is the case, it would result in some random flag getting overwritten in original AGI after the loop was
+// completed, because in original AGI loop_flag + move_flag shared the same memory location.
+// This is basically an implementation error in the original interpreter.
+// Happens in at least KQ1, when grabbing the eagle (room 22).
+void AgiEngine::motionActivated(ScreenObjEntry *screenObj) {
+	if (screenObj->flags & fCycling) {
+		// Cycling active too
+		switch (screenObj->cycle) {
+		case kCycleEndOfLoop: // "end.of.loop"
+		case kCycleRevLoop: // "reverse.loop"
+			// Disable it
+			screenObj->flags &= ~fCycling;
+			screenObj->cycle = kCycleNormal;
+
+			warning("Motion activated for screen object %d, but cycler also active", screenObj->objectNr);
+			warning("This would have resulted in flag corruption in original AGI. Cycler disabled.");
+			break;
+		default:
+			break;
+		}
+	}
+}
+
+// WORKAROUND:
+// See comment for motionActivated()
+// This way no flag would have been overwritten, but certain other variables of the motions.
+void AgiEngine::cyclerActivated(ScreenObjEntry *screenObj) {
+	switch (screenObj->motionType) {
+	case kMotionWander:
+		// this would have resulted in wander_count to get corrupted
+		// We don't stop it.
+		break;
+	case kMotionFollowEgo:
+		// this would have resulted in follow_stepSize to get corrupted
+		// do not stop motion atm - screenObj->direction = 0;
+		// do not stop motion atm - screenObj->motionType = kMotionNormal;
+		break;
+	case kMotionMoveObj:
+		// this would have resulted in move_x to get corrupted
+		// do not stop motion atm - motionMoveObjStop(screenObj);
+		break;
+	default:
+		return;
+		break;
+	}
+	warning("Cycler activated for screen object %d, but motion also active", screenObj->objectNr);
+	warning("This would have resulted in corruption in original AGI. Motion disabled.");
+}
+
 void AgiEngine::motionWander(ScreenObjEntry *screenObj) {
 	uint8 originalWanderCount = screenObj->wander_count;
 
diff --git a/engines/agi/op_cmd.cpp b/engines/agi/op_cmd.cpp
index 8ee9958ecd..0e0f49b542 100644
--- a/engines/agi/op_cmd.cpp
+++ b/engines/agi/op_cmd.cpp
@@ -1495,6 +1495,8 @@ void cmdReverseLoop(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	screenObj->flags |= (fDontupdate | fUpdate | fCycling);
 	screenObj->loop_flag = loopFlag;
 	state->_vm->setFlag(screenObj->loop_flag, false);
+
+	vm->cyclerActivated(screenObj);
 }
 
 void cmdReverseLoopV1(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
@@ -1520,6 +1522,8 @@ void cmdEndOfLoop(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	screenObj->flags |= (fDontupdate | fUpdate | fCycling);
 	screenObj->loop_flag = loopFlag;
 	vm->setFlag(screenObj->loop_flag, false);
+
+	vm->cyclerActivated(screenObj);
 }
 
 void cmdEndOfLoopV1(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
@@ -1632,6 +1636,8 @@ void cmdFollowEgo(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 		vm->setFlag(screenObj->follow_flag, false);
 		screenObj->flags |= fUpdate;
 	}
+
+	vm->motionActivated(screenObj);
 }
 
 void cmdMoveObj(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
@@ -1660,6 +1666,8 @@ void cmdMoveObj(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 		screenObj->flags |= fUpdate;
 	}
 
+	vm->motionActivated(screenObj);
+
 	if (objectNr == 0)
 		state->playerControl = false;
 
@@ -1688,6 +1696,8 @@ void cmdMoveObjF(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	vm->setFlag(screenObj->move_flag, false);
 	screenObj->flags |= fUpdate;
 
+	vm->motionActivated(screenObj);
+
 	if (objectNr == 0)
 		state->playerControl = false;
 
@@ -1709,6 +1719,8 @@ void cmdWander(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	} else {
 		screenObj->flags |= fUpdate;
 	}
+
+	vm->motionActivated(screenObj);
 }
 
 void cmdSetGameID(AgiGame *state, AgiEngine *vm, uint8 *parameter) {