AGI: remove timer hack, implement in game timer

in game timer is now updated, when scripts read in game timer
VM variables and during main loop. ScummVM total play time feature
is used for it. Game cycle syncing is done at the same time.

12 files changed, 154 insertions, 89 deletions

diff --git a/engines/agi/agi.cpp b/engines/agi/agi.cpp
index 8dd663ccbb..c8490d4a89 100644
--- a/engines/agi/agi.cpp
+++ b/engines/agi/agi.cpp
@@ -390,7 +390,6 @@ AgiEngine::AgiEngine(OSystem *syst, const AGIGameDescription *gameDesc) : AgiBas
 
 	setupOpcodes();
 	_game._curLogic = NULL;
-	_timerHack = 0;
 
 	_lastSaveTime = 0;
 	_lastTick = 0;
@@ -460,8 +459,6 @@ void AgiEngine::initialize() {
 	_font->init();
 	_text->init(_systemUI);
 
-	_gfx->initMachine();
-
 	_game.gameFlags = 0;
 
 	_text->charAttrib_Set(15, 0);
@@ -530,7 +527,6 @@ AgiEngine::~AgiEngine() {
 	delete _text;
 	delete _sprites;
 	delete _picture;
-	_gfx->deinitMachine();
 	delete _gfx;
 	delete _font;
 	delete _words;
@@ -549,7 +545,7 @@ Common::Error AgiEngine::go() {
 	if (_game.mouseEnabled) {
 		CursorMan.showMouse(true);
 	}
-	setTotalPlayTime(0);
+	inGameTimerReset();
 
 	if (_game.state < STATE_LOADED) {
 		do {
diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 4c7580dadb..7e1505f8f7 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -653,7 +653,6 @@ public:
 	GfxMgr  *_gfx;
 
 	Common::RenderMode _renderMode;
-	volatile uint32 _clockCount;
 	AgiDebug _debug;
 	AgiGame _game;
 	Common::RandomSource *_rnd;
@@ -824,12 +823,13 @@ public:
 
 	byte getVar(int16 varNr);
 	void setVar(int16 varNr, byte newValue);
+
+public:
 	void decrypt(uint8 *mem, int len);
 	void releaseSprites();
 	int mainCycle(bool onlyCheckForEvents = false);
 	int viewPictures();
 	int runGame();
-	void updateTimer();
 	int getAppDir(char *appDir, unsigned int size);
 
 	int setupV2Game(int ver);
@@ -950,14 +950,23 @@ public:
 public:
 	void redrawScreen();
 
+	void inGameTimerReset(uint32 newPlayTime = 0);
+	void inGameTimerPause();
+	void inGameTimerResume();
+	uint32 inGameTimerGet();
+
+	void inGameTimerUpdate();
+
+private:
+	uint32 _lastUsedPlayTimeInCycles; // 20 per second
+	uint32 _lastUsedPlayTimeInSeconds; // actual seconds
+	uint32 _passedPlayTimeCycles; // increased by 1 every time we passed a cycle
+
 private:
 	AgiCommand _agiCommands[183];
 	AgiCommand _agiCondCommands[256];
 
 	void setupOpcodes();
-
-public:
-	int _timerHack;			// Workaround for timer loop in MH1 logic 153
 };
 
 } // End of namespace Agi
diff --git a/engines/agi/cycle.cpp b/engines/agi/cycle.cpp
index 85a465cb02..6506ee29eb 100644
--- a/engines/agi/cycle.cpp
+++ b/engines/agi/cycle.cpp
@@ -173,37 +173,6 @@ void AgiEngine::interpretCycle() {
 	//_gfx->doUpdate();
 }
 
-/**
- * Update AGI interpreter timer.
- */
-void AgiEngine::updateTimer() {
-	_clockCount++;
-	if (_clockCount <= TICK_SECONDS)
-		return;
-
-	_clockCount -= TICK_SECONDS;
-
-	if (!_game.clockEnabled)
-		return;
-
-	setVar(VM_VAR_SECONDS, getVar(VM_VAR_SECONDS) + 1);
-	if (getVar(VM_VAR_SECONDS) < 60)
-		return;
-
-	setVar(VM_VAR_SECONDS, 0);
-	setVar(VM_VAR_MINUTES, getVar(VM_VAR_MINUTES) + 1);
-	if (getVar(VM_VAR_MINUTES) < 60)
-		return;
-
-	setVar(VM_VAR_MINUTES, 0);
-	setVar(VM_VAR_HOURS, getVar(VM_VAR_HOURS) + 1);
-	if (getVar(VM_VAR_HOURS) < 24)
-		return;
-
-	setVar(VM_VAR_HOURS, 0);
-	setVar(VM_VAR_DAYS, getVar(VM_VAR_DAYS) + 1);
-}
-
 void AgiEngine::newInputMode(InputMode mode) {
 	//if (mode == INPUTMODE_MENU && !getflag(VM_FLAG_MENUS_WORK) && !(getFeatures() & GF_MENUS))
 	//	return;
@@ -224,7 +193,6 @@ int AgiEngine::mainCycle(bool onlyCheckForEvents) {
 
 	if (!onlyCheckForEvents) {
 		pollTimer();
-		updateTimer();
 	}
 
 	if (_menu->delayedExecuteActive()) {
@@ -402,7 +370,11 @@ int AgiEngine::playGame() {
 		if (!mainCycle())
 			continue;
 
-		if (getVar(VM_VAR_TIME_DELAY) == 0 || (1 + _clockCount) % getVar(VM_VAR_TIME_DELAY) == 0) {
+		inGameTimerUpdate();
+
+		if (_passedPlayTimeCycles >= getVar(VM_VAR_TIME_DELAY)) {
+			_passedPlayTimeCycles = 0;
+
 			if (!_game.hasPrompt && _game.inputMode == INPUTMODE_NORMAL) {
 				_text->promptRedraw();
 				_game.hasPrompt = 1;
@@ -468,6 +440,10 @@ int AgiEngine::runGame() {
 		if (_restartGame) {
 			setFlag(VM_FLAG_RESTART_GAME, true);
 			setVar(VM_VAR_TIME_DELAY, 2);	// "normal" speed
+
+			// Reset in-game timer
+			inGameTimerReset();
+
 			_restartGame = false;
 		}
 
diff --git a/engines/agi/global.cpp b/engines/agi/global.cpp
index 9f3fb451c2..3cc8ffc6f8 100644
--- a/engines/agi/global.cpp
+++ b/engines/agi/global.cpp
@@ -58,9 +58,89 @@ void AgiEngine::setVar(int16 varNr, byte newValue) {
 }
 
 byte AgiEngine::getVar(int16 varNr) {
+	switch (varNr) {
+	case VM_VAR_SECONDS:
+	case VM_VAR_MINUTES:
+	case VM_VAR_HOURS:
+	case VM_VAR_DAYS:
+		// Timer Update is necessary in here, because of at least Manhunter 1 script 153
+		// Sierra AGI updated the timer via a timer procedure
+		inGameTimerUpdate();
+		break;
+	default:
+		break;
+	}
 	return _game.vars[varNr];
 }
 
+// In-Game timer, used for timer VM Variables
+void AgiEngine::inGameTimerReset(uint32 newPlayTime) {
+	_passedPlayTimeCycles = 0;
+	_lastUsedPlayTimeInCycles = newPlayTime / 50;
+	_lastUsedPlayTimeInSeconds = newPlayTime / 1000;
+	setTotalPlayTime(newPlayTime);
+}
+void AgiEngine::inGameTimerPause() {
+	pauseEngine(true);
+}
+void AgiEngine::inGameTimerResume() {
+	pauseEngine(false);
+}
+uint32 AgiEngine::inGameTimerGet() {
+	return getTotalPlayTime();
+}
+
+// This is called, when one of the timer variables is read
+// We calculate the latest variables, according to current official playtime
+// This is also called in the main loop, because the game needs to be sync'd to 20 cycles per second
+void AgiEngine::inGameTimerUpdate() {
+	uint32 curPlayTimeMilliseconds = inGameTimerGet();
+	uint32 curPlayTimeCycles = curPlayTimeMilliseconds / 50;
+
+	if (curPlayTimeCycles == _lastUsedPlayTimeInCycles) {
+		// No difference, skip updating
+		return;
+	}
+
+	// Increase passed cycles accordingly
+	int32 playTimeCycleDelta = curPlayTimeCycles - _lastUsedPlayTimeInCycles;
+	if (playTimeCycleDelta > 0) {
+		_passedPlayTimeCycles += playTimeCycleDelta;
+	}
+	_lastUsedPlayTimeInCycles = curPlayTimeCycles;
+
+	// Now calculate current play time in seconds
+	uint32 curPlayTimeSeconds = curPlayTimeMilliseconds / 1000;
+
+	if (curPlayTimeSeconds == _lastUsedPlayTimeInSeconds) {
+		// No difference, skip updating
+		return;
+	}
+
+	uint32 secondsLeft = 0;
+	byte   curDays = 0;
+	byte   curHours = 0;
+	byte   curMinutes = 0;
+	byte   curSeconds = 0;
+
+	curDays = curPlayTimeSeconds / 86400;
+	secondsLeft = curPlayTimeSeconds % 86400;
+
+	curHours = secondsLeft / 3600;
+	secondsLeft = secondsLeft % 3600;
+
+	curMinutes = secondsLeft / 60;
+	curSeconds = secondsLeft % 60;
+
+	// directly set them, otherwise we would go into an endless loop
+	_game.vars[VM_VAR_SECONDS] = curSeconds;
+	_game.vars[VM_VAR_MINUTES] = curMinutes;
+	_game.vars[VM_VAR_HOURS] = curHours;
+	_game.vars[VM_VAR_DAYS] = curDays;
+
+	_lastUsedPlayTimeInSeconds = curPlayTimeSeconds;
+}
+
 void AgiEngine::decrypt(uint8 *mem, int len) {
 	const uint8 *key;
 	int i;
diff --git a/engines/agi/graphics.cpp b/engines/agi/graphics.cpp
index b19e729409..f1407715cc 100644
--- a/engines/agi/graphics.cpp
+++ b/engines/agi/graphics.cpp
@@ -156,16 +156,6 @@ int GfxMgr::deinitVideo() {
 	return errOK;
 }
 
-int GfxMgr::initMachine() {
-	_vm->_clockCount = 0;
-
-	return errOK;
-}
-
-int GfxMgr::deinitMachine() {
-	return errOK;
-}
-
 void GfxMgr::setRenderStartOffset(uint16 offsetY) {
 	if (offsetY >= (DISPLAY_HEIGHT - SCRIPT_HEIGHT))
 		error("invalid render start offset");
diff --git a/engines/agi/graphics.h b/engines/agi/graphics.h
index 7fffe320c9..f321e9e66b 100644
--- a/engines/agi/graphics.h
+++ b/engines/agi/graphics.h
@@ -76,9 +76,6 @@ public:
 	void initMouseCursor(MouseCursorData *mouseCursor, const byte *bitmapData, uint16 width, uint16 height, int hotspotX, int hotspotY);
 	void setMouseCursor(bool busy = false);
 
-	int initMachine();
-	int deinitMachine();
-
 	void setRenderStartOffset(uint16 offsetY);
 	uint16 getRenderStartOffsetY();
 
diff --git a/engines/agi/keyboard.cpp b/engines/agi/keyboard.cpp
index 81de225e71..2a5f801a25 100644
--- a/engines/agi/keyboard.cpp
+++ b/engines/agi/keyboard.cpp
@@ -547,7 +547,6 @@ int AgiEngine::waitKey() {
 			break;
 
 		pollTimer();
-		updateTimer();
 
 		g_system->updateScreen();
 	}
diff --git a/engines/agi/menu.cpp b/engines/agi/menu.cpp
index 411cd002fd..ea24bd5b46 100644
--- a/engines/agi/menu.cpp
+++ b/engines/agi/menu.cpp
@@ -292,11 +292,17 @@ void GfxMenu::execute() {
 	}
 	drawActiveMenu();
 
+	// original AGI did not do this, at least when the menu was called by scripts
+	_vm->inGameTimerPause();
+
 	_vm->cycleInnerLoopActive(CYCLE_INNERLOOP_MENU);
 	do {
 		_vm->mainCycle();
 	} while (_vm->cycleInnerLoopIsActive() && !(_vm->shouldQuit() || _vm->_restartGame));
 
+	// original AGI did not do this, at least when the menu was called by scripts
+	_vm->inGameTimerResume();
+
 	removeActiveMenu();
 
 	_text->charAttrib_Pop();
diff --git a/engines/agi/op_cmd.cpp b/engines/agi/op_cmd.cpp
index 57a6d6833d..4b82d2464a 100644
--- a/engines/agi/op_cmd.cpp
+++ b/engines/agi/op_cmd.cpp
@@ -811,25 +811,41 @@ void cmdResetScanStart(AgiGame *state, uint8 *parameter) {
 }
 
 void cmdSaveGame(AgiGame *state, uint8 *parameter) {
+	AgiEngine *vm = state->_vm;
+
+	vm->inGameTimerPause();
+
 	if (state->automaticSave) {
-		if (state->_vm->saveGameAutomatic()) {
+		if (vm->saveGameAutomatic()) {
 			// automatic save succeded
+			vm->inGameTimerResume();
 			return;
 		}
 		// fall back to regular dialog otherwise
 	}
-	state->_vm->saveGameDialog();
+
+	vm->saveGameDialog();
+
+	vm->inGameTimerResume();
 }
 
 void cmdLoadGame(AgiGame *state, uint8 *parameter) {
+	AgiEngine *vm = state->_vm;
+
+	vm->inGameTimerPause();
+
 	if (state->automaticSave) {
-		if (state->_vm->loadGameAutomatic()) {
+		if (vm->loadGameAutomatic()) {
 			// automatic restore succeded
+			vm->inGameTimerResume();
 			return;
 		}
 		// fall back to regular dialog otherwise
 	}
-	state->_vm->loadGameDialog();
+
+	vm->loadGameDialog();
+
+	vm->inGameTimerResume();
 }
 
 void cmdInitDisk(AgiGame *state, uint8 *parameter) {				// do nothing
@@ -1740,11 +1756,8 @@ void cmdSetGameID(AgiGame *state, uint8 *parameter) {
 
 void cmdPause(AgiGame *state, uint8 *parameter) {
 	AgiEngine *vm = state->_vm;
-	int originalClockState = state->clockEnabled;
 	bool skipPause = false;
 
-	state->clockEnabled = false;
-
 	// We check in here, if a special key was specified to trigger menus.
 	// If that's the case, normally triggering the menu should be handled inside handleController()
 	// For the rare cases, where this approach doesn't work because the trigger is not mapped to a controller,
@@ -1773,10 +1786,16 @@ void cmdPause(AgiGame *state, uint8 *parameter) {
 
 	if (!skipPause) {
 		// Show pause message box
+		int originalClockState = state->clockEnabled;
+
+		vm->inGameTimerPause();
+		state->clockEnabled = false;
+
 		state->_vm->_systemUI->pauseDialog();
-	}
 
-	state->clockEnabled = originalClockState;
+		vm->inGameTimerPause();
+		state->clockEnabled = originalClockState;
+	}
 }
 
 void cmdSetMenu(AgiGame *state, uint8 *parameter) {
@@ -2370,7 +2389,6 @@ int AgiEngine::runLogic(int n) {
 
 	_game._curLogic->cIP = _game._curLogic->sIP;
 
-	_timerHack = 0;
 	while (state->_curLogic->cIP < _game.logics[n].size && !(shouldQuit() || _restartGame)) {
 		// TODO: old code, needs to be adjusted
 #if 0
@@ -2405,14 +2423,6 @@ int AgiEngine::runLogic(int n) {
 		case 0xfe:	// goto
 			// +2 covers goto size
 			state->_curLogic->cIP += 2 + ((int16)READ_LE_UINT16(state->_curLogic->data + state->_curLogic->cIP));
-
-			// timer must keep running even in goto loops,
-			// but AGI engine can't do that :(
-			if (_timerHack > 20) {
-				pollTimer();
-				updateTimer();
-				_timerHack = 0;
-			}
 			break;
 		case 0x00:	// return
 			debugC(2, kDebugLevelScripts, "%sreturn() // Logic %d", st, n);
diff --git a/engines/agi/op_test.cpp b/engines/agi/op_test.cpp
index c461dfc4c4..5836e9aba2 100644
--- a/engines/agi/op_test.cpp
+++ b/engines/agi/op_test.cpp
@@ -42,38 +42,26 @@ namespace Agi {
 #define testObjInRoom(obj, v)	(state->_vm->objectGetLocation(obj) == getVar(v))
 
 void condEqual(AgiGame *state, uint8 *p) {
-	if (p[0] == VM_VAR_SECONDS)
-		state->_vm->_timerHack++;
 	state->testResult = testEqual(p[0], p[1]);
 }
 
 void condEqualV(AgiGame *state, uint8 *p) {
-	if (p[0] == VM_VAR_SECONDS || p[1] == VM_VAR_SECONDS)
-		state->_vm->_timerHack++;
 	state->testResult = testEqual(p[0], getVar(p[1]));
 }
 
 void condLess(AgiGame *state, uint8 *p) {
-	if (p[0] == VM_VAR_SECONDS)
-		state->_vm->_timerHack++;
 	state->testResult = testLess(p[0], p[1]);
 }
 
 void condLessV(AgiGame *state, uint8 *p) {
-	if (p[0] == VM_VAR_SECONDS || p[1] == VM_VAR_SECONDS)
-		state->_vm->_timerHack++;
 	state->testResult = testLess(p[0], getVar(p[1]));
 }
 
 void condGreater(AgiGame *state, uint8 *p) {
-	if (p[0] == VM_VAR_SECONDS)
-		state->_vm->_timerHack++;
 	state->testResult = testGreater(p[0], p[1]);
 }
 
 void condGreaterV(AgiGame *state, uint8 *p) {
-	if (p[0] == VM_VAR_SECONDS || p[1] == VM_VAR_SECONDS)
-		state->_vm->_timerHack++;
 	state->testResult = testGreater(p[0], getVar(p[1]));
 }
 
diff --git a/engines/agi/preagi.cpp b/engines/agi/preagi.cpp
index 833b2d3064..088d23aa68 100644
--- a/engines/agi/preagi.cpp
+++ b/engines/agi/preagi.cpp
@@ -62,7 +62,6 @@ void PreAgiEngine::initialize() {
 	_picture = new PictureMgr(this, _gfx);
 
 	_font->init();
-	_gfx->initMachine();
 
 	_game.gameFlags = 0;
 
diff --git a/engines/agi/saveload.cpp b/engines/agi/saveload.cpp
index b6974ee651..b47a0cf43f 100644
--- a/engines/agi/saveload.cpp
+++ b/engines/agi/saveload.cpp
@@ -331,6 +331,7 @@ int AgiEngine::loadGame(const Common::String &fileName, bool checkId) {
 	uint8 t;
 	int16 parm[7];
 	Common::InSaveFile *in;
+	bool totalPlayTimeWasSet = false;
 
 	debugC(3, kDebugLevelMain | kDebugLevelSavegame, "AgiEngine::loadGame(%s)", fileName.c_str());
 
@@ -380,7 +381,8 @@ int AgiEngine::loadGame(const Common::String &fileName, bool checkId) {
 		in->readUint16BE(); // save time
 		if (saveVersion >= 6) {
 			uint32 playTime = in->readUint32BE();
-			g_engine->setTotalPlayTime(playTime * 1000);
+			inGameTimerReset(playTime * 1000);
+			totalPlayTimeWasSet = true;
 		}
 	}
 
@@ -439,6 +441,19 @@ int AgiEngine::loadGame(const Common::String &fileName, bool checkId) {
 	for (i = 0; i < MAX_VARS; i++)
 		_game.vars[i] = in->readByte();
 
+	if (!totalPlayTimeWasSet) {
+		// If we haven't gotten total play time by now, try to calculate it by using VM Variables
+		// This will happen for at least saves before version 6
+		// Direct access because otherwise we would trigger an update to these variables according to ScummVM total play time
+		byte playTimeSeconds = _game.vars[VM_VAR_SECONDS];
+		byte playTimeMinutes = _game.vars[VM_VAR_MINUTES];
+		byte playTimeHours   = _game.vars[VM_VAR_HOURS];
+		byte playTimeDays    = _game.vars[VM_VAR_DAYS];
+		uint32 playTime = (playTimeSeconds + (playTimeMinutes * 60) + (playTimeHours * 3600) + (playTimeDays * 86400)) * 1000;
+
+		inGameTimerReset(playTime);
+	}
+
 	setVar(VM_VAR_FREE_PAGES, 180); // Set amount of free memory to realistic value (Overwriting the just loaded value)
 
 	_game.horizon = in->readSint16BE();