[Scummvm-git-logs] scummvm master -> 3a0ecdb2f7ae24c30ef2fd1a22e7729ef3603769

[bluegr]

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

Summary:
8459c6f277 DGDS: Fix reading per-scene globals
3a0ecdb2f7 DGDS: Add safeguards against undefined behavior in scene globals


Commit: 8459c6f2770fb36b9c8d2664bd9d92b2e6bc545a
    https://github.com/scummvm/scummvm/commit/8459c6f2770fb36b9c8d2664bd9d92b2e6bc545a
Author: Filippos Karapetis (bluegr at gmail.com)
Date: 2024-07-06T17:40:05+03:00

Commit Message:
DGDS: Fix reading per-scene globals

The functions that were passed to the constructor would not be called
in the order that they appear, causing the globals list to be initialized
incorrectly

Changed paths:
    engines/dgds/scene.cpp


diff --git a/engines/dgds/scene.cpp b/engines/dgds/scene.cpp
index f0d6d08ad15..50b20a2c5cb 100644
--- a/engines/dgds/scene.cpp
+++ b/engines/dgds/scene.cpp
@@ -1647,8 +1647,11 @@ void GDSScene::initIconSizes() {
 
 bool GDSScene::readPerSceneGlobals(Common::SeekableReadStream *s) {
 	uint16 numGlobals = s->readUint16LE();
+	uint16 num, scene;
 	for (uint16 i = 0; i < numGlobals; i++) {
-		_perSceneGlobals.push_back(PerSceneGlobal(s->readUint16LE(), s->readUint16LE()));
+		num = s->readUint16LE();
+		scene = s->readUint16LE();
+		_perSceneGlobals.push_back(PerSceneGlobal(num, scene));
 		_perSceneGlobals.back()._val = s->readSint16LE();
 	}
 	return !s->err();


Commit: 3a0ecdb2f7ae24c30ef2fd1a22e7729ef3603769
    https://github.com/scummvm/scummvm/commit/3a0ecdb2f7ae24c30ef2fd1a22e7729ef3603769
Author: Filippos Karapetis (bluegr at gmail.com)
Date: 2024-07-06T17:40:05+03:00

Commit Message:
DGDS: Add safeguards against undefined behavior in scene globals

Guard against potential cases where a scene global isn't correctly
set, and we fall back to game globals. This essentially resulted in
undefined behavior, so now we properly try to handle such cases, or
error out to detect potential game-breaking bugs

Changed paths:
    engines/dgds/globals.cpp
    engines/dgds/scene.cpp
    engines/dgds/scene.h


diff --git a/engines/dgds/globals.cpp b/engines/dgds/globals.cpp
index 884b37022aa..4c0e204a362 100644
--- a/engines/dgds/globals.cpp
+++ b/engines/dgds/globals.cpp
@@ -93,7 +93,8 @@ int16 Globals::getGlobal(uint16 num) {
 			return global->get();
 		}
 	}
-	return 0;
+
+	error("getGlobal: requested non-existing global %d", num);
 }
 
 int16 Globals::setGlobal(uint16 num, int16 val) {
@@ -102,7 +103,8 @@ int16 Globals::setGlobal(uint16 num, int16 val) {
 			return global->set(val);
 		}
 	}
-	return 0;
+
+	error("setGlobal: requested non-existing global %d", num);
 }
 
 Common::Error Globals::syncState(Common::Serializer &s) {
diff --git a/engines/dgds/scene.cpp b/engines/dgds/scene.cpp
index 50b20a2c5cb..0234a4b59e4 100644
--- a/engines/dgds/scene.cpp
+++ b/engines/dgds/scene.cpp
@@ -1757,6 +1757,11 @@ int16 GDSScene::getGlobal(uint16 num) {
 	for (const auto &global : _perSceneGlobals) {
 		if (global.matches(num, curSceneNum))
 			return global._val;
+		else if (!global.matches(num, curSceneNum) && global.numMatches(num)) {
+			// This looks like a script bug, get it anyway
+			warning("getGlobal: scene global %d is not in scene %d", num, curSceneNum);
+			return global._val;
+		}
 	}
 	Globals *gameGlobals = engine->getGameGlobals();
 	return gameGlobals->getGlobal(num);
@@ -1769,6 +1774,11 @@ int16 GDSScene::setGlobal(uint16 num, int16 val) {
 		if (global.matches(num, curSceneNum)) {
 			global._val = val;
 			return val;
+		} else if (!global.matches(num, curSceneNum) && global.numMatches(num)) {
+			// This looks like a script bug, set it anyway
+			warning("setGlobal: scene global %d is not in scene %d", num, curSceneNum);
+			global._val = val;
+			return val;			
 		}
 	}
 	Globals *gameGlobals = engine->getGameGlobals();
diff --git a/engines/dgds/scene.h b/engines/dgds/scene.h
index 59c19066f41..2a0f8fa1fd3 100644
--- a/engines/dgds/scene.h
+++ b/engines/dgds/scene.h
@@ -206,6 +206,7 @@ public:
 
 	Common::String dump(const Common::String &indent) const;
 	bool matches(uint16 num, uint16 scene) const { return num == _num && (_sceneNo == 0 || _sceneNo == scene); }
+	bool numMatches(uint16 num) const { return num == _num; }
 
 	int16 _val;