MADS: Cleanup of Scene::checkStartWalk to become Player::newWalk

7 files changed, 94 insertions, 65 deletions

diff --git a/engines/mads/action.cpp b/engines/mads/action.cpp
index e997261938..fe90305c9d 100644
--- a/engines/mads/action.cpp
+++ b/engines/mads/action.cpp
@@ -31,7 +31,6 @@ namespace MADS {
 
 MADSAction::MADSAction(MADSEngine *vm) : _vm(vm) {
 	clear();
-	_startWalkFlag = false;
 	_statusTextIndex = -1;
 	_selectedAction = 0;
 	_inProgress = false;
@@ -63,7 +62,6 @@ void MADSAction::clear() {
 	_action._objectNameId = -1;
 	_action._indirectObjectId = -1;
 	_textChanged = true;
-	_walkFlag = false;
 }
 
 void MADSAction::appendVocab(int vocabId, bool capitalise) {
@@ -77,10 +75,11 @@ void MADSAction::appendVocab(int vocabId, bool capitalise) {
 
 void MADSAction::checkCustomDest(int v) {
 	Scene &scene = _vm->_game->_scene;
+	Player &player = _vm->_game->_player;
 
 	if (_v86F4A && (v == -3 || _savedFields._selectedRow < 0)) {
-		_startWalkFlag = true;
-		scene._destPos = scene._customDest;
+		_vm->_game->_player._needToWalk = true;
+		player._prepareWalkPos = scene._customDest;
 	}
 }
 
@@ -265,11 +264,12 @@ void MADSAction::refresh() {
 
 void MADSAction::startAction() {
 	Game &game = *_vm->_game;
+	Player &player = game._player;
 	Scene &scene = _vm->_game->_scene;
 	DynamicHotspots &dynHotspots = scene._dynamicHotspots;
 	Hotspots &hotspots = scene._hotspots;
 
-	game._player.cancelCommand();
+	player.cancelCommand();
 
 	_inProgress = true;
 	_v8453A = ABORTMODE_0;
@@ -290,7 +290,7 @@ void MADSAction::startAction() {
 	if ((_actionMode2 == ACTIONMODE2_4) && (_v86F42 == 0))
 		_v8453A = -1;
 
-	_startWalkFlag = false;
+	player._needToWalk = false;
 	int hotspotId = -1;
 
 	if (!_savedFields._lookFlag && (_vm->_game->_screenObjects._v832EC != 1)) {
@@ -304,13 +304,13 @@ void MADSAction::startAction() {
 			if ((hs._feetPos.x == -1) || (hs._feetPos.x == -3)) {
 				checkCustomDest(hs._feetPos.x);
 			} else if (hs._feetPos.x == 0) {
-				scene._targetFacing = hs._facing;
+				player._prepareWalkFacing = hs._facing;
 			} else if (_savedFields._actionMode == ACTIONMODE_NONE || hs._cursor >= CURSOR_WAIT) {
-				_startWalkFlag = true;
-				scene._destPos = hs._feetPos;
+				player._needToWalk = true;
+				player._prepareWalkPos = hs._feetPos;
 			}
 
-			scene._targetFacing = hs._facing;
+			player._prepareWalkFacing = hs._facing;
 			hotspotId = -1;
 		}
 	}
@@ -322,20 +322,20 @@ void MADSAction::startAction() {
 			checkCustomDest(hs._feetPos.x);
 		} else if (hs._feetPos.x >= 0) {
 			if (_savedFields._actionMode == ACTIONMODE_NONE || hs._cursor < CURSOR_WAIT) {
-				_startWalkFlag = true;
-				scene._destPos = hs._feetPos;
+				player._needToWalk = true;
+				player._prepareWalkPos = hs._feetPos;
 			}
 		}
 
-		scene._targetFacing = hs._facing;
+		player._prepareWalkFacing = hs._facing;
 	}
 
-	_walkFlag = _startWalkFlag;
+	player._readyToWalk = player._needToWalk;
 }
 
 void MADSAction::checkAction() {
 	if (isAction(VERB_LOOK) || isAction(VERB_THROW))
-		_startWalkFlag = 0;
+		_vm->_game->_player._needToWalk = false;
 }
 
 bool MADSAction::isAction(int verbId, int objectNameId, int indirectObjectId) {
diff --git a/engines/mads/action.h b/engines/mads/action.h
index 74de167a4e..a517a65d0e 100644
--- a/engines/mads/action.h
+++ b/engines/mads/action.h
@@ -94,10 +94,8 @@ public:
 	int _selectedRow;
 	bool _textChanged;
 	int _selectedAction;
-	bool _startWalkFlag;
 	int _statusTextIndex;
 	int _hotspotId;
-	bool _walkFlag;
 	ActionSavedFields _savedFields;
 
 	// Unknown fields
diff --git a/engines/mads/nebular/nebular_scenes2.cpp b/engines/mads/nebular/nebular_scenes2.cpp
index 96347ea226..d90aa93b20 100644
--- a/engines/mads/nebular/nebular_scenes2.cpp
+++ b/engines/mads/nebular/nebular_scenes2.cpp
@@ -192,7 +192,7 @@ void Scene201::enter() {
 		_scene->_sequences.addSubEntry(_globals._spriteIndexes[21], SM_FRAME_INDEX, 12, 70);
 		_scene->_sequences.setDepth(_globals._spriteIndexes[21], 1);
 		_pterodactylFlag = false;
-		_game._player.startWalking(Common::Point(157, 143), FACING_NORTH);
+		_game._player.walk(Common::Point(157, 143), FACING_NORTH);
 		_vm->_palette->setEntry(252, 45, 63, 45);
 		_vm->_palette->setEntry(253, 20, 45, 20);
 		_scene->_kernelMessages.add(Common::Point(0, 0), 0x1110, 2, 0, 120, _game.getQuote(90));
@@ -707,14 +707,16 @@ int Scene202::subStep4(int randVal) {
 }
 
 void Scene202::preActions() {
-	MADSAction *action = _game._player._action;
-	if (action->_walkFlag)
+	Player &player = _vm->_game->_player;
+	MADSAction *action = player._action;
+
+	if (player._readyToWalk)
 		_scene->_kernelMessages.reset();
 
-	if (!_ladderTopFl && (action->isAction(0x4E, 0xC7) || !action->_walkFlag)) {
+	if (!_ladderTopFl && (action->isAction(0x4E, 0xC7) || !player._readyToWalk)) {
 		if (_game._trigger == 0) {
 			_vm->_sound->command(29);
-			action->_walkFlag = false;
+			player._readyToWalk = false;
 			_game._player._stepEnabled = false;
 			_scene->_sequences.remove(_globals._spriteIndexes[24]);
 			_globals._spriteIndexes[23] = _scene->_sequences.addReverseSpriteCycle(_globals._spriteIndexes[8], false, 6, 1, 0, 0);
@@ -724,20 +726,20 @@ void Scene202::preActions() {
 			_scene->_sequences.updateTimeout(-1, _globals._spriteIndexes[23]);
 			_scene->_dynamicHotspots.remove(_ladderHotspotId);
 			_game._player._visible = true;
-			action->_walkFlag = true;
+			player._readyToWalk = true;
 			_game._player._stepEnabled = true;
 			_ladderTopFl = false;
 		}		
 	}
 
 	if (action->isAction(0x3, 0x27) && action->_activeAction._indirectObjectId > 0) {
-		if (!action->_walkFlag || _ladderTopFl)
-			action->_startWalkFlag = false;
+		if (!player._readyToWalk || _ladderTopFl)
+			_game._player._needToWalk = false;
 		else
-			action->_startWalkFlag = true;
+			_game._player._needToWalk = true;
 
 		if (!_ladderTopFl)
-			_game._player.startWalking(Common::Point(171, 122), FACING_NORTH);
+			_game._player.walk(Common::Point(171, 122), FACING_NORTH);
 	}
 }
 
@@ -988,7 +990,7 @@ void Scene203::enter() {
 
 	if ((_globals[kRhotundaStatus] == 0) && (_scene->_roomChanged == 0)) {
 		_globals._v0 = -1;
-		_game._player.startWalking(Common::Point(158, 135), FACING_SOUTH);
+		_game._player.walk(Common::Point(158, 135), FACING_SOUTH);
 		int idx = _scene->_dynamicHotspots.add(131, 396, 0, Common::Rect(0, 0, 320, 156));
 		_scene->_dynamicHotspots.setPosition(idx, 155, 152, FACING_SOUTH);
 		_scene->_dynamicHotspots.setCursor(idx, CURSOR_GO_DOWN);
@@ -1041,7 +1043,7 @@ void Scene203::step() {
 
 void Scene203::preActions() {
 	if (_globals._v0 && !_action.isAction(0x18C, 0x83)) {
-		_game._player.startWalking(Common::Point(158, 136), FACING_SOUTH);
+		_game._player.walk(Common::Point(158, 136), FACING_SOUTH);
 		_action._inProgress = false;
 		return;
 	}
@@ -1476,14 +1478,16 @@ void Scene208::step() {
 }
 
 void Scene208::preActions() {
-	if (_action.isAction(3) && _action._walkFlag)
-		_action._startWalkFlag = true;
+	Player &player = _vm->_game->_player;
+
+	if (_action.isAction(3) && player._readyToWalk)
+		player._needToWalk = true;
 
 	if (_action.isAction(0x18C, 0x9B))
-		_game._player._walkOffScreenSceneId = 209;
+		player._walkOffScreenSceneId = 209;
 
 	if (_action.isAction(0x18C, 0xF6))
-		_game._player._walkOffScreenSceneId = 207;
+		player._walkOffScreenSceneId = 207;
 }
 
 void Scene208::actions() {
@@ -1510,7 +1514,7 @@ void Scene208::actions() {
 		if (_game._player._stepEnabled) {
 			_game._player._stepEnabled = false;
 			_globals._v0 = true;
-			_game._player.startWalking(Common::Point(20, 148), FACING_EAST);
+			_game._player.walk(Common::Point(20, 148), FACING_EAST);
 		}
 	} else if (_action.isAction(0x7, 0x35, 0x1A9)) {
 		warning("TODO: sub3B282(4);");
diff --git a/engines/mads/player.cpp b/engines/mads/player.cpp
index fe92f876d3..57726e7bef 100644
--- a/engines/mads/player.cpp
+++ b/engines/mads/player.cpp
@@ -37,6 +37,7 @@ Player::Player(MADSEngine *vm): _vm(vm) {
 	_facing = FACING_NORTH;
 	_turnToFacing = FACING_NORTH;
 	_targetFacing = FACING_NORTH;
+	_prepareWalkFacing = FACING_NONE;
 	_mirror = false;
 	_spritesLoaded = false;
 	_spritesStart = 0;
@@ -45,6 +46,8 @@ Player::Player(MADSEngine *vm): _vm(vm) {
 	_stepEnabled = false;
 	_visible = false;
 	_priorVisible = false;
+	_needToWalk = false;
+	_readyToWalk = false;
 	_visible3 = false;
 	_loadsFirst = false;
 	_loadedFirst = false;
@@ -85,8 +88,8 @@ void Player::cancelWalk() {
 	_routeCount = 0;
 	_walkAnywhere = false;
 
-	_action->_startWalkFlag = false;
-	_action->_walkFlag = false;
+	_needToWalk = false;
+	_readyToWalk = false;
 }
 
 bool Player::loadSprites(const Common::String &prefix) {
@@ -341,7 +344,7 @@ void Player::clearStopList() {
 	_trigger = 0;
 }
 
-void Player::setDest(const Common::Point &pt, Facing facing) {
+void Player::startWalking(const Common::Point &pt, Facing facing) {
 	Scene &scene = _vm->_game->_scene;
 
 	clearStopList();
@@ -370,14 +373,14 @@ void Player::setDest(const Common::Point &pt, Facing facing) {
 	}
 }
 
-void Player::startWalking(const Common::Point &pos, Facing direction) {
+void Player::walk(const Common::Point &pos, Facing facing) {
 	Scene &scene = _vm->_game->_scene;
 
 	cancelWalk();
-	scene._action._startWalkFlag = true;
-	scene._action._walkFlag = true;
-	scene._destPos = pos;
-	scene._targetFacing = direction;
+	_needToWalk = true;
+	_readyToWalk = true;
+	_prepareWalkPos = pos;
+	_prepareWalkFacing = facing;
 }
 
 void Player::nextFrame() {
@@ -392,7 +395,7 @@ void Player::nextFrame() {
 			idle();
 		}
 
-		postUpdate();
+		setFrame();
 		update();
 	}
 }
@@ -514,10 +517,10 @@ void Player::idle() {
 	int frameIndex = ABS(_frameListIndex);
 	int direction = (_frameListIndex < 0) ? -1 : 1;
 
-	if (frameIndex >= spriteSet._charInfo->_numEntries)
+	if (frameIndex >= spriteSet._charInfo->_numEntries) {
 		// Reset back to the start of the list
 		_frameListIndex = 0;
-	else {
+	}  else {
 		_frameNumber += direction;
 		_forceRefresh = true;
 
@@ -532,7 +535,7 @@ void Player::idle() {
 	}
 }
 
-void Player::postUpdate() {
+void Player::setFrame() {
 	if (_moving) {
 		if (++_frameNumber > _frameCount)
 			_frameNumber = 1;
@@ -757,6 +760,13 @@ void Player::startMovement() {
 	_distAccum = -_deltaDistance;
 }
 
+void Player::newWalk() {
+	if (_needToWalk && _readyToWalk) {
+		startWalking(_prepareWalkPos, _prepareWalkFacing);
+		_needToWalk = false;
+	}
+}
+
 void Player::step() {
 	warning("TODO: Player::step");
 }
diff --git a/engines/mads/player.h b/engines/mads/player.h
index 6500806f1f..2a9274840c 100644
--- a/engines/mads/player.h
+++ b/engines/mads/player.h
@@ -70,7 +70,10 @@ private:
 	 */
 	void move();
 
-	void postUpdate();
+	/**
+	 * Update the player's frame number
+	 */
+	void setFrame();
 
 	/**
 	 * Get the sprite slot index for the player
@@ -82,6 +85,9 @@ private:
 	 */
 	int getScale(int yp);
 
+	/**
+	 * Sets the frame rate for the current sprite set
+	 */
 	void setBaseFrameRate();
 
 	void setupRoute();
@@ -107,6 +113,7 @@ public:
 
 	Facing _facing;
 	Facing _turnToFacing;
+	Facing _prepareWalkFacing;
 	int _xDirection, _yDirection;
 	Facing _targetFacing;
 	bool _spritesLoaded;
@@ -126,6 +133,7 @@ public:
 	Common::Point _targetPos;
 	Common::Point _posChange;
 	Common::Point _posDiff;
+	Common::Point _prepareWalkPos;
 	bool _moving;
 	int _walkOffScreen, _walkOffScreenSceneId;
 	int _next;
@@ -137,6 +145,8 @@ public:
 	int _trigger;
 	bool _scalingVelocity;
 	bool _forceRefresh;
+	bool _needToWalk;
+	bool _readyToWalk;
 	int _centerOfGravity;
 	int _currentDepth;
 	int _currentScale;
@@ -181,11 +191,30 @@ public:
 
 	void update();
 
+	/**
+	 * Handler method for when the player is not moving
+	 */
 	void idle();
 
-	void setDest(const Common::Point &pt, Facing facing);
+	/**
+	 * Starts the player walking towards a given point and direction facing
+	 * @param pos		Destination location
+	 * @param facing	Direction to face once the destination is reached
+	 */
+	void startWalking(const Common::Point &pt, Facing facing);
+
+	/**
+	 * Used by the game scripst to make the player walk to a given destination.
+	 * The difference from startWalking is that this contains several extra
+	 * layers of checking that startWalking bypasses.
+	 */
+	void walk(const Common::Point &pos, Facing facing);
 
-	void startWalking(const Common::Point &pos, Facing direction);
+	/**
+	 * If a new walk sequence is pending, and has been okayed by the preparser,
+	 * start the actual walking
+	 */
+	void newWalk();
 
 	void nextFrame();
 
diff --git a/engines/mads/scene.cpp b/engines/mads/scene.cpp
index 62702518fc..2e788115db 100644
--- a/engines/mads/scene.cpp
+++ b/engines/mads/scene.cpp
@@ -42,7 +42,6 @@ Scene::Scene(MADSEngine *vm): _vm(vm), _action(_vm), _depthSurface(vm),
 	_depthStyle = 0;
 	_roomChanged = false;
 	_reloadSceneFlag = false;
-	_targetFacing = FACING_NONE;
 	_freeAnimationFlag = false;
 	_animationData = nullptr;
 	_activeAnimation = nullptr;
@@ -296,11 +295,11 @@ void Scene::doFrame() {
 			_dynamicHotspots.refresh();
 
 		// Check all on-screen visual objects
-		_vm->_game->_screenObjects.check(player._stepEnabled && !_action._startWalkFlag &&
+		_vm->_game->_screenObjects.check(player._stepEnabled && !player._needToWalk &&
 				!_vm->_game->_fx);
 	}
 
-	if (_action._selectedAction && player._stepEnabled && !_action._startWalkFlag &&
+	if (_action._selectedAction && player._stepEnabled && !player._needToWalk &&
 			!_vm->_game->_trigger && !player._trigger) {
 		_action.startAction();
 		if (_action._activeAction._verbId == Nebular::NOUN_LOOK_AT) {
@@ -315,11 +314,11 @@ void Scene::doFrame() {
 		doPreactions();
 	}
 
-	checkStartWalk();
+	player.newWalk();
 	if (!_vm->_game->_fx)
 		_frameStartTime = _vm->_events->getFrameCounter();
 
-	if ((_action._inProgress && !player._moving && !_action._startWalkFlag &&
+	if ((_action._inProgress && !player._moving && !player._needToWalk &&
 			player._turnToFacing == player._facing) ||
 			(_vm->_game->_trigger && _vm->_game->_abortTimersMode == ABORTMODE_0)) {
 		doAction();
@@ -499,13 +498,6 @@ void Scene::doAction() {
 		_vm->_game->_trigger = 0;
 }
 
-void Scene::checkStartWalk() {
-	if (_action._startWalkFlag && _action._walkFlag) {
-		_vm->_game->_player.setDest(_destPos, _targetFacing);
-		_action._startWalkFlag = false;
-	}
-}
-
 void Scene::doSceneStep() {
 	_vm->_game->_abortTimersMode2 = ABORTMODE_1;
 	_sceneLogic->step();
diff --git a/engines/mads/scene.h b/engines/mads/scene.h
index ee1a0c7853..0af5fa6575 100644
--- a/engines/mads/scene.h
+++ b/engines/mads/scene.h
@@ -65,8 +65,6 @@ private:
 
 	void doAction();
 
-	void checkStartWalk();
-
 	void doSceneStep();
 
 	void checkKeyboard();
@@ -118,8 +116,6 @@ public:
 	MADSAction _action;
 	bool _roomChanged;
 	bool _reloadSceneFlag;
-	Common::Point _destPos;
-	Facing _targetFacing;
 	Common::Point _posAdjust;
 	uint32 _frameStartTime;
 	Layer _layer;