[Scummvm-git-logs] scummvm master -> c4678c993fe53365dc6a5f0bb02778023128ad31

athrxx athrxx at scummvm.org
Thu Aug 5 23:20:54 UTC 2021


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

Summary:
9bcfc91ce2 SCUMM: (v1/2) - fix walking steps calculations
5a94f51eab SCUMM: (v1/2) - fix actor move flags
c4678c993f SCUMM:  split Actor::calcMovementFactor() into separate functions for SCUMM1-3 and SCUMM4+.


Commit: 9bcfc91ce25e94705bfae43b7b90a7ecea3f895a
    https://github.com/scummvm/scummvm/commit/9bcfc91ce25e94705bfae43b7b90a7ecea3f895a
Author: athrxx (athrxx at scummvm.org)
Date: 2021-08-06T00:58:40+02:00

Commit Message:
SCUMM: (v1/2) - fix walking steps calculations

After my recent effort to do this for SCUMM3 I now try to achieve the same thing for v1/2. (Unsurprisingly) the step calculations actually have more in common with SCUMM3 than with the later versions upon which the code was based. However, I find the  v1/2 code somewhat more difficult to fix than v3, since it is quite heavily twisted and refactored to fit into our common code. So all testing and bug reporting is welcome...

Changed paths:
    engines/scumm/actor.cpp
    engines/scumm/actor.h
    engines/scumm/saveload.cpp


diff --git a/engines/scumm/actor.cpp b/engines/scumm/actor.cpp
index 16242938fb..d9367b77de 100644
--- a/engines/scumm/actor.cpp
+++ b/engines/scumm/actor.cpp
@@ -151,8 +151,8 @@ void Actor::initActor(int mode) {
 		_flip = false;
 		_speedx = 8;
 		_speedy = 2;
-		_v3stepX = 0;
-		_v3stepThreshold = 0;
+		_stepX = 0;
+		_stepThreshold = 0;
 		_frame = 0;
 		_walkbox = 0;
 		_animProgress = 0;
@@ -486,22 +486,28 @@ int Actor::calcMovementFactor(const Common::Point& next) {
 	diffX = next.x - _pos.x;
 	diffY = next.y - _pos.y;
 
-	if (_vm->_game.version == 3) {
-		// These two lines fix bug #1052 (INDY3: Hitler facing wrong directions in the Berlin scene).
-		// I can't see anything like this in the original SCUMM1/2 code, so I limit this to SCUMM3.
-		if (!(_moving & MF_LAST_LEG) && (int)_speedx > ABS(diffX) && (int)_speedy > ABS(diffY))
-			return 0;
+	if (_vm->_game.version <= 3) {
+		if (_vm->_game.version <= 2) {
+			_stepThreshold = MAX(ABS(diffX), ABS(diffY));
+			deltaXFactor = deltaYFactor = 1;
+		} else {
+			// These two lines fix bug #1052 (INDY3: Hitler facing wrong directions in the Berlin scene).
+			// I can't see anything like this in the original SCUMM1/2 code, so I limit this to SCUMM3.
+			if (!(_moving & MF_LAST_LEG) && (int)_speedx > ABS(diffX) && (int)_speedy > ABS(diffY))
+				return 0;
 
-		_v3stepX = ((ABS(diffY) / (int)_speedy) >> 1) > (ABS(diffX) / (int)_speedx) ? _speedy + 1 : _speedx;
-		_v3stepThreshold = MAX(ABS(diffY) / _speedy, ABS(diffX) / _v3stepX);
-		deltaXFactor = (int32)_v3stepX;
+			_stepX = ((ABS(diffY) / (int)_speedy) >> 1) > (ABS(diffX) / (int)_speedx) ? _speedy + 1 : _speedx;
+			_stepThreshold = MAX(ABS(diffY) / _speedy, ABS(diffX) / _stepX);
+			deltaXFactor = (int32)_stepX;
+			deltaYFactor = (int32)_speedy;
+		}
 		if (diffX < 0)
 			deltaXFactor = -deltaXFactor;
-		deltaYFactor = (int32)_speedy;
 		if (diffY < 0)
 			deltaYFactor = -deltaYFactor;
-		_walkdata.xfrac = _walkdata.v3XAdd = diffX / deltaXFactor;
-		_walkdata.yfrac = _walkdata.v3YAdd = diffY / deltaYFactor;
+
+		_walkdata.xfrac = _walkdata.xAdd = diffX / deltaXFactor;
+		_walkdata.yfrac = _walkdata.yAdd = diffY / deltaYFactor;
 	} else {
 		deltaYFactor = _speedy << 16;
 		if (diffY < 0)
@@ -554,17 +560,22 @@ int Actor::actorWalkStep() {
 
 	nextFacing = updateActorDirection(true);
 	if (!(_moving & MF_IN_LEG) || _facing != nextFacing) {
-		if (_walkFrame != _frame || _facing != nextFacing) {
+		int facing = _facing;
+		if (_walkFrame != _frame || _facing != nextFacing)
 			startWalkAnim(1, nextFacing);
-		}
+
 		_moving |= MF_IN_LEG;
-		// The next two lines fix bug #12278 for ZAK FM-TOWNS. Limited to SCUMM3.
-		if (_vm->_game.version == 3)
+		// The next two lines fix bug #12278 for ZAK FM-TOWNS (SCUMM3). They are alse required for SCUMM 1/2 to prevent movement while
+		// turning, but only if the character has to make a turn. The correct behavior for v1/2 can be tested by letting Zak (only v1/2
+		// versions) walk in the starting room from the torn wallpaper to the desk drawer: Zak should first turn around clockwise by
+		// 180°, then walk one step to the left, then turn clockwise 90°. For ZAK FM-TOWNS (SCUMM3) this part will look quite different
+		// (and a bit weird), but I have confirmed the correctness with the FM-Towns emulator, too.
+		if (_vm->_game.version == 3 || (_vm->_game.version <= 2 && facing != nextFacing))
 			return 1;
 	}
 
 	if (_vm->_game.version == 3) {
-		if (_walkdata.next.x - (int)_v3stepX <= _pos.x && _walkdata.next.x + (int)_v3stepX >= _pos.x)
+		if (_walkdata.next.x - (int)_stepX <= _pos.x && _walkdata.next.x + (int)_stepX >= _pos.x)
 			_pos.x = _walkdata.next.x;
 		if (_walkdata.next.y - (int)_speedy <= _pos.y && _walkdata.next.y + (int)_speedy >= _pos.y)
 			_pos.y = _walkdata.next.y;
@@ -581,27 +592,14 @@ int Actor::actorWalkStep() {
 		return 0;
 	}
 
-	if (_vm->_game.version <= 2) {
-		if (_walkdata.deltaXFactor != 0) {
-			if (_walkdata.deltaXFactor > 0)
-				_pos.x += 1;
-			else
-				_pos.x -= 1;
-		}
-		if (_walkdata.deltaYFactor != 0) {
-			if (_walkdata.deltaYFactor > 0)
-				_pos.y += 1;
-			else
-				_pos.y -= 1;
-		}
-	} else if (_vm->_game.version == 3) {
-		if ((_walkdata.xfrac += _walkdata.v3XAdd) >= _v3stepThreshold) {
+	if (_vm->_game.version <= 3) {
+		if ((_walkdata.xfrac += _walkdata.xAdd) >= _stepThreshold) {
 			_pos.x += _walkdata.deltaXFactor;
-			_walkdata.xfrac -= _v3stepThreshold;
+			_walkdata.xfrac -= _stepThreshold;
 		}
-		if ((_walkdata.yfrac += _walkdata.v3YAdd) >= _v3stepThreshold) {
+		if ((_walkdata.yfrac += _walkdata.yAdd) >= _stepThreshold) {
 			_pos.y += _walkdata.deltaYFactor;
-			_walkdata.yfrac -= _v3stepThreshold;
+			_walkdata.yfrac -= _stepThreshold;
 		}
 	} else {
 		tmpX = (_pos.x << 16) + _walkdata.xfrac + (_walkdata.deltaXFactor >> 8) * _scalex;
@@ -3745,25 +3743,34 @@ void Actor::saveLoadWithSerializer(Common::Serializer &s) {
 		setDirection(_facing);
 	}
 
-	if (_vm->_game.version == 3) {
-		if (s.isLoading() && s.getVersion() < VER(101)) {
+	if (_vm->_game.version <= 3) {
+		int rev = (_vm->_game.version == 3 ? 101 : 102);
+		if (s.isLoading() && s.getVersion() < VER(rev)) {
 			int diffX = _walkdata.next.x - _pos.x;
 			int diffY = _walkdata.next.y - _pos.y;
-			_v3stepX = ((ABS(diffY) / (int)_speedy) >> 1) >(ABS(diffX) / (int)_speedx) ? _speedy + 1 : _speedx;
-			_v3stepThreshold = MAX(ABS(diffY) / _speedy, ABS(diffX) / _v3stepX);
-			_walkdata.deltaXFactor = (int32)_v3stepX;
+
+			if (_vm->_game.version < 3) {
+				_stepThreshold = MAX(ABS(diffX), ABS(diffY));
+				_walkdata.deltaXFactor = _walkdata.deltaYFactor = 1;
+			} else {
+				_stepX = ((ABS(diffY) / (int)_speedy) >> 1) >(ABS(diffX) / (int)_speedx) ? _speedy + 1 : _speedx;
+				_stepThreshold = MAX(ABS(diffY) / _speedy, ABS(diffX) / _stepX);
+				_walkdata.deltaXFactor = (int32)_stepX;
+				_walkdata.deltaYFactor = (int32)_speedy;
+			}
+
 			if (diffX < 0)
 				_walkdata.deltaXFactor = -_walkdata.deltaXFactor;
-			_walkdata.deltaYFactor = (int32)_speedy;
 			if (diffY < 0)
 				_walkdata.deltaYFactor = -_walkdata.deltaYFactor;
-			_walkdata.xfrac = _walkdata.v3XAdd = diffX / _walkdata.deltaXFactor;
-			_walkdata.yfrac = _walkdata.v3YAdd = diffY / _walkdata.deltaYFactor;
+			_walkdata.xfrac = _walkdata.xAdd = diffX / _walkdata.deltaXFactor;
+			_walkdata.yfrac = _walkdata.yAdd = diffY / _walkdata.deltaYFactor;
+
 		} else {
-			s.syncAsUint16LE(_walkdata.v3XAdd, VER(101));
-			s.syncAsUint16LE(_walkdata.v3YAdd, VER(101));
-			s.syncAsUint16LE(_v3stepX, VER(101));
-			s.syncAsUint16LE(_v3stepThreshold, VER(101));
+			s.syncAsUint16LE(_walkdata.xAdd, VER(rev));
+			s.syncAsUint16LE(_walkdata.yAdd, VER(rev));
+			s.syncAsUint16LE(_stepX, VER(rev));
+			s.syncAsUint16LE(_stepThreshold, VER(rev));
 		}
 	}
 }
diff --git a/engines/scumm/actor.h b/engines/scumm/actor.h
index 3a9fb96ff2..267d1e624b 100644
--- a/engines/scumm/actor.h
+++ b/engines/scumm/actor.h
@@ -151,7 +151,7 @@ protected:
 		Common::Point point3;
 		int32 deltaXFactor, deltaYFactor;
 		uint16 xfrac, yfrac;
-		uint16 v3XAdd, v3YAdd;
+		uint16 xAdd, yAdd;
 
 		void reset() {
 			dest.x = dest.y = 0;
@@ -165,8 +165,8 @@ protected:
 			deltaYFactor = 0;
 			xfrac = 0;
 			yfrac = 0;
-			v3XAdd = 0;
-			v3YAdd = 0;
+			xAdd = 0;
+			yAdd = 0;
 		}
 	};
 
@@ -176,7 +176,7 @@ protected:
 	uint16 _facing;
 	uint16 _targetFacing;
 	uint _speedx, _speedy;
-	uint _v3stepX, _v3stepThreshold;
+	uint _stepX, _stepThreshold;
 	byte _animProgress, _animSpeed;
 	bool _costumeNeedsInit;
 	ActorWalkData _walkdata;
diff --git a/engines/scumm/saveload.cpp b/engines/scumm/saveload.cpp
index 4dd13977d8..4cc80bf6d6 100644
--- a/engines/scumm/saveload.cpp
+++ b/engines/scumm/saveload.cpp
@@ -68,7 +68,7 @@ struct SaveInfoSection {
 
 #define SaveInfoSectionSize (4+4+4 + 4+4 + 4+2)
 
-#define CURRENT_VER 101
+#define CURRENT_VER 102
 #define INFOSECTION_VERSION 2
 
 #pragma mark -


Commit: 5a94f51eabb153715d28510ff26b315b65654436
    https://github.com/scummvm/scummvm/commit/5a94f51eabb153715d28510ff26b315b65654436
Author: athrxx (athrxx at scummvm.org)
Date: 2021-08-06T01:00:15+02:00

Commit Message:
SCUMM: (v1/2) - fix actor move flags

(final fix for ticket no. 3215 ("SCUMM: Zak McKracken - intro behavior + movement")

The ticket has been closed a couple weeks ago, since a user claimed that the intro was fully fixed. And it mostly was. Except one little thing about the movement of the floating hat which, after chasing Zak, would return to the left of the screen one step to early.

Turns out that o2_waitForActor() got triggered one step to early, because we didn't set the final MF_TURN flag...

Changed paths:
    engines/scumm/actor.cpp


diff --git a/engines/scumm/actor.cpp b/engines/scumm/actor.cpp
index d9367b77de..21713e3463 100644
--- a/engines/scumm/actor.cpp
+++ b/engines/scumm/actor.cpp
@@ -548,6 +548,9 @@ int Actor::calcMovementFactor(const Common::Point& next) {
 	else
 		_targetFacing = getAngleFromPos(deltaXFactor, deltaYFactor, (_vm->_game.id == GID_DIG || _vm->_game.id == GID_CMI));
 
+	if (_vm->_game.version <= 2 && _facing != updateActorDirection(true))
+		_moving |= MF_TURN;
+
 	return actorWalkStep();
 }
 
@@ -560,7 +563,6 @@ int Actor::actorWalkStep() {
 
 	nextFacing = updateActorDirection(true);
 	if (!(_moving & MF_IN_LEG) || _facing != nextFacing) {
-		int facing = _facing;
 		if (_walkFrame != _frame || _facing != nextFacing)
 			startWalkAnim(1, nextFacing);
 
@@ -570,7 +572,7 @@ int Actor::actorWalkStep() {
 		// versions) walk in the starting room from the torn wallpaper to the desk drawer: Zak should first turn around clockwise by
 		// 180°, then walk one step to the left, then turn clockwise 90°. For ZAK FM-TOWNS (SCUMM3) this part will look quite different
 		// (and a bit weird), but I have confirmed the correctness with the FM-Towns emulator, too.
-		if (_vm->_game.version == 3 || (_vm->_game.version <= 2 && facing != nextFacing))
+		if (_vm->_game.version == 3 || (_vm->_game.version <= 2 && (_moving & MF_TURN)))
 			return 1;
 	}
 
@@ -1168,7 +1170,7 @@ void Actor_v2::walkActor() {
 		actorWalkStep();
 	} else {
 		if (_moving & MF_LAST_LEG) {
-			_moving = 0;
+			_moving = MF_TURN;
 			startAnimActor(_standFrame);
 			if (_targetFacing != _walkdata.destdir)
 				turnToDirection(_walkdata.destdir);


Commit: c4678c993fe53365dc6a5f0bb02778023128ad31
    https://github.com/scummvm/scummvm/commit/c4678c993fe53365dc6a5f0bb02778023128ad31
Author: athrxx (athrxx at scummvm.org)
Date: 2021-08-06T01:00:57+02:00

Commit Message:
SCUMM:  split Actor::calcMovementFactor() into separate functions for SCUMM1-3 and SCUMM4+.

(The code has diverged so much, it makes sense to make use of the already existing sub class here)

Changed paths:
    engines/scumm/actor.cpp
    engines/scumm/actor.h


diff --git a/engines/scumm/actor.cpp b/engines/scumm/actor.cpp
index 21713e3463..97d4487e93 100644
--- a/engines/scumm/actor.cpp
+++ b/engines/scumm/actor.cpp
@@ -486,67 +486,81 @@ int Actor::calcMovementFactor(const Common::Point& next) {
 	diffX = next.x - _pos.x;
 	diffY = next.y - _pos.y;
 
-	if (_vm->_game.version <= 3) {
-		if (_vm->_game.version <= 2) {
-			_stepThreshold = MAX(ABS(diffX), ABS(diffY));
-			deltaXFactor = deltaYFactor = 1;
-		} else {
-			// These two lines fix bug #1052 (INDY3: Hitler facing wrong directions in the Berlin scene).
-			// I can't see anything like this in the original SCUMM1/2 code, so I limit this to SCUMM3.
-			if (!(_moving & MF_LAST_LEG) && (int)_speedx > ABS(diffX) && (int)_speedy > ABS(diffY))
-				return 0;
+	deltaYFactor = _speedy << 16;
+	if (diffY < 0)
+		deltaYFactor = -deltaYFactor;
 
-			_stepX = ((ABS(diffY) / (int)_speedy) >> 1) > (ABS(diffX) / (int)_speedx) ? _speedy + 1 : _speedx;
-			_stepThreshold = MAX(ABS(diffY) / _speedy, ABS(diffX) / _stepX);
-			deltaXFactor = (int32)_stepX;
-			deltaYFactor = (int32)_speedy;
-		}
+	deltaXFactor = deltaYFactor * diffX;
+	if (diffY != 0) {
+		deltaXFactor /= diffY;
+	} else {
+		deltaYFactor = 0;
+	}
+
+	if ((uint)ABS(deltaXFactor >> 16) > _speedx) {
+		deltaXFactor = _speedx << 16;
 		if (diffX < 0)
 			deltaXFactor = -deltaXFactor;
-		if (diffY < 0)
-			deltaYFactor = -deltaYFactor;
-
-		_walkdata.xfrac = _walkdata.xAdd = diffX / deltaXFactor;
-		_walkdata.yfrac = _walkdata.yAdd = diffY / deltaYFactor;
-	} else {
-		deltaYFactor = _speedy << 16;
-		if (diffY < 0)
-			deltaYFactor = -deltaYFactor;
 
-		deltaXFactor = deltaYFactor * diffX;
-		if (diffY != 0) {
-			deltaXFactor /= diffY;
+		deltaYFactor = deltaXFactor * diffY;
+		if (diffX != 0) {
+			deltaYFactor /= diffX;
 		} else {
-			deltaYFactor = 0;
+			deltaXFactor = 0;
 		}
+	}
 
-		if ((uint)ABS(deltaXFactor >> 16) > _speedx) {
-			deltaXFactor = _speedx << 16;
-			if (diffX < 0)
-				deltaXFactor = -deltaXFactor;
+	_walkdata.xfrac = 0;
+	_walkdata.yfrac = 0;
+	_walkdata.cur = _pos;
+	_walkdata.next = next;
+	_walkdata.deltaXFactor = deltaXFactor;
+	_walkdata.deltaYFactor = deltaYFactor;
 
-			deltaYFactor = deltaXFactor * diffY;
-			if (diffX != 0) {
-				deltaYFactor /= diffX;
-			} else {
-				deltaXFactor = 0;
-			}
-		}
+	_targetFacing = getAngleFromPos(deltaXFactor, deltaYFactor, (_vm->_game.id == GID_DIG || _vm->_game.id == GID_CMI));
 
-		_walkdata.xfrac = 0;
-		_walkdata.yfrac = 0;
+	return actorWalkStep();
+}
+
+int Actor_v3::calcMovementFactor(const Common::Point& next) {
+	int diffX, diffY;
+	int32 deltaXFactor, deltaYFactor;
+
+	if (_pos == next)
+		return 0;
+
+	diffX = next.x - _pos.x;
+	diffY = next.y - _pos.y;
+
+	if (_vm->_game.version <= 2) {
+		_stepThreshold = MAX(ABS(diffX), ABS(diffY));
+		deltaXFactor = deltaYFactor = 1;
+	} else {
+		// These two lines fix bug #1052 (INDY3: Hitler facing wrong directions in the Berlin scene).
+		// I can't see anything like this in the original SCUMM1/2 code, so I limit this to SCUMM3.
+		if (!(_moving & MF_LAST_LEG) && (int)_speedx > ABS(diffX) && (int)_speedy > ABS(diffY))
+			return 0;
+
+		_stepX = ((ABS(diffY) / (int)_speedy) >> 1) > (ABS(diffX) / (int)_speedx) ? _speedy + 1 : _speedx;
+		_stepThreshold = MAX(ABS(diffY) / _speedy, ABS(diffX) / _stepX);
+		deltaXFactor = (int32)_stepX;
+		deltaYFactor = (int32)_speedy;
 	}
 
+	if (diffX < 0)
+		deltaXFactor = -deltaXFactor;
+	if (diffY < 0)
+		deltaYFactor = -deltaYFactor;
+
+	_walkdata.xfrac = _walkdata.xAdd = diffX / deltaXFactor;
+	_walkdata.yfrac = _walkdata.yAdd = diffY / deltaYFactor;
 	_walkdata.cur = _pos;
 	_walkdata.next = next;
 	_walkdata.deltaXFactor = deltaXFactor;
 	_walkdata.deltaYFactor = deltaYFactor;
 
-	if (_vm->_game.version <= 3)
-		// The x/y distance ratio which determines whether to face up/down instead of left/right is different for SCUMM1/2 and SCUMM3.
-		_targetFacing = oldDirToNewDir(((ABS(diffY) * (_vm->_game.version == 3 ? 3 : 1)) > ABS(diffX)) ? 3 - (diffY >= 0 ? 1 : 0) : (diffX >= 0 ? 1 : 0));
-	else
-		_targetFacing = getAngleFromPos(deltaXFactor, deltaYFactor, (_vm->_game.id == GID_DIG || _vm->_game.id == GID_CMI));
+	// The x/y distance ratio which determines whether to face up/down instead of left/right is different for SCUMM1/2 and SCUMM3.
+	_targetFacing = oldDirToNewDir(((ABS(diffY) * (_vm->_game.version == 3 ? 3 : 1)) > ABS(diffX)) ? 3 - (diffY >= 0 ? 1 : 0) : (diffX >= 0 ? 1 : 0));
 
 	if (_vm->_game.version <= 2 && _facing != updateActorDirection(true))
 		_moving |= MF_TURN;
diff --git a/engines/scumm/actor.h b/engines/scumm/actor.h
index 267d1e624b..8c131ec2b8 100644
--- a/engines/scumm/actor.h
+++ b/engines/scumm/actor.h
@@ -208,7 +208,7 @@ public:
 	void putActor(int x, int y, int room);
 	void setActorWalkSpeed(uint newSpeedX, uint newSpeedY);
 protected:
-	int calcMovementFactor(const Common::Point& next);
+	virtual int calcMovementFactor(const Common::Point& next);
 	int actorWalkStep();
 	int remapDirection(int dir, bool is_walking);
 	virtual void setupActorScale();
@@ -333,6 +333,7 @@ public:
 	Actor_v3(ScummEngine *scumm, int id) : Actor(scumm, id) {}
 
 	void walkActor() override;
+	int calcMovementFactor(const Common::Point& next) override;
 
 protected:
 	void setupActorScale() override;




More information about the Scummvm-git-logs mailing list