[Scummvm-git-logs] scummvm master -> 21c261245601499ecbee41e996f577d49117a380
mduggan
noreply at scummvm.org
Mon Oct 21 12:14:34 UTC 2024
This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
21c2612456 DGDS: Fix scene ops on scene change
Commit: 21c261245601499ecbee41e996f577d49117a380
https://github.com/scummvm/scummvm/commit/21c261245601499ecbee41e996f577d49117a380
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2024-10-21T23:13:38+11:00
Commit Message:
DGDS: Fix scene ops on scene change
In d26ee6, I changed the scene ops to always be executed to the end even if the
scene changed - this was not correct and it breaks scene 60 (police cutscene)
in Rise of the Dragon, which stores the current day into a variable - but if
run twice will leave the day as 999 after leaving.
The correct behavior is not to abort ops *only* in the case of scene change to
inventory. All other scene changes should still abort running scene ops.
Changed paths:
engines/dgds/clock.cpp
engines/dgds/clock.h
engines/dgds/dgds.cpp
engines/dgds/scene.cpp
engines/dgds/scene.h
diff --git a/engines/dgds/clock.cpp b/engines/dgds/clock.cpp
index 714543b4ce3..ec361ab1176 100644
--- a/engines/dgds/clock.cpp
+++ b/engines/dgds/clock.cpp
@@ -162,6 +162,10 @@ void Clock::update(bool gameRunning) {
addGameTime(mins_to_add);
}
+Common::String Clock::dump() {
+ return Common::String::format("days %d hours %d mins %d", _days, _mins, _hours);
+}
+
Common::Error Clock::syncState(Common::Serializer &s) {
s.syncAsUint32LE(_lastPlayTime);
s.syncAsUint32LE(_millis);
diff --git a/engines/dgds/clock.h b/engines/dgds/clock.h
index 17671532765..1fcb1e8d600 100644
--- a/engines/dgds/clock.h
+++ b/engines/dgds/clock.h
@@ -64,6 +64,8 @@ public:
Common::Error syncState(Common::Serializer &s);
+ Common::String dump();
+
private:
uint32 _lastPlayTime;
uint32 _millis;
diff --git a/engines/dgds/dgds.cpp b/engines/dgds/dgds.cpp
index 01519ed0f4d..8c55679a317 100644
--- a/engines/dgds/dgds.cpp
+++ b/engines/dgds/dgds.cpp
@@ -158,6 +158,8 @@ void DgdsEngine::loadIcons() {
bool DgdsEngine::changeScene(int sceneNum) {
assert(_scene && _adsInterp);
+ debug("CHANGE SCENE %d -> %d (clock %s)", _scene->getNum(), sceneNum, _clock.dump().c_str());
+
if (sceneNum == _scene->getNum()) {
warning("Tried to change from scene %d to itself, doing nothing.", sceneNum);
return false;
@@ -223,8 +225,8 @@ bool DgdsEngine::changeScene(int sceneNum) {
else
_adsInterp->unload();
- _scene->runEnterSceneOps();
debug("%s", _scene->dump("").c_str());
+ _scene->runEnterSceneOps();
_justChangedScene1 = true;
_justChangedScene2 = true;
@@ -801,7 +803,6 @@ Common::Error DgdsEngine::syncGame(Common::Serializer &s) {
_storedAreaBuffer.fillRect(Common::Rect(SCREEN_WIDTH, SCREEN_HEIGHT), 0);
}
- debug("%s", _scene->dump("").c_str());
_scene->runEnterSceneOps();
return Common::kNoError;
diff --git a/engines/dgds/scene.cpp b/engines/dgds/scene.cpp
index f44ad98f19d..f7c475338e0 100644
--- a/engines/dgds/scene.cpp
+++ b/engines/dgds/scene.cpp
@@ -621,7 +621,7 @@ static void _drawDragonCountdown(FontManager::FontType fontType, int16 x, int16
}
-bool Scene::runSceneOp(const SceneOp &op, bool sceneChanged) {
+bool Scene::runSceneOp(const SceneOp &op) {
DgdsEngine *engine = DgdsEngine::getInstance();
switch (op._opCode) {
case kSceneOpChangeScene:
@@ -646,7 +646,7 @@ bool Scene::runSceneOp(const SceneOp &op, bool sceneChanged) {
case kSceneOpOpenInventory:
engine->getInventory()->open();
// This implicitly changes scene num
- return true;
+ break;
case kSceneOpShowDlg:
if (op._args.size() == 1)
engine->getScene()->showDialog(0, op._args[0]);
@@ -663,7 +663,7 @@ bool Scene::runSceneOp(const SceneOp &op, bool sceneChanged) {
engine->getScene()->enableTrigger(op._args[0]);
break;
case kSceneOpChangeSceneToStored: {
- uint16 sceneNo = engine->getGameGlobals()->getGlobal(0x61);
+ int16 sceneNo = engine->getGameGlobals()->getGlobal(0x61);
if (engine->changeScene(sceneNo))
return true;
break;
@@ -732,7 +732,7 @@ bool Scene::runSceneOp(const SceneOp &op, bool sceneChanged) {
}
/*static*/
-bool Scene::runDragonOp(const SceneOp &op, bool sceneChanged) {
+bool Scene::runDragonOp(const SceneOp &op) {
DgdsEngine *engine = DgdsEngine::getInstance();
switch (op._opCode) {
case kSceneOpPasscode:
@@ -776,7 +776,7 @@ bool Scene::runDragonOp(const SceneOp &op, bool sceneChanged) {
}
/*static*/
-bool Scene::runChinaOp(const SceneOp &op, bool sceneChanged) {
+bool Scene::runChinaOp(const SceneOp &op) {
DgdsEngine *engine = DgdsEngine::getInstance();
switch (op._opCode) {
case kSceneOpOpenChinaOpenGameOverMenu:
@@ -806,7 +806,7 @@ bool Scene::runChinaOp(const SceneOp &op, bool sceneChanged) {
return false;
}
-bool Scene::runBeamishOp(const SceneOp &op, bool sceneChanged) {
+bool Scene::runBeamishOp(const SceneOp &op) {
DgdsEngine *engine = DgdsEngine::getInstance();
if (op._opCode & 0x8000) {
@@ -837,15 +837,18 @@ bool Scene::runBeamishOp(const SceneOp &op, bool sceneChanged) {
//
// Note: ops list here is not a reference on purpose, it must be copied.
// The underlying list might be freed during execution if the scene changes, but
-// we have to finish executing the whole list.
+// we have to finish executing the list if the scene changed into inventory -
+// which could have invalidated the op list pointer. We *don't* finish executing
+// if any other scene change happens.
//
-// Scene change can also invalidate the `this` pointer, which is why
-// this is static and the runOp functions fetch the scene through the engine.
+// Because scene change can also invalidate the `this` pointer, this is static
+// and the runOp functions fetch the scene through the engine.
//
/*static*/
bool Scene::runOps(const Common::Array<SceneOp> ops, int16 addMinuites /* = 0 */) {
DgdsEngine *engine = DgdsEngine::getInstance();
bool sceneChanged = false;
+ int16 startSceneNum = engine->getScene()->getNum();
for (const SceneOp &op : ops) {
if (!checkConditions(op._conditionList))
continue;
@@ -855,26 +858,36 @@ bool Scene::runOps(const Common::Array<SceneOp> ops, int16 addMinuites /* = 0 */
addMinuites = 0;
}
if (op._opCode < 100) {
- sceneChanged |= runSceneOp(op, sceneChanged);
- // NOTE: After executing op, `this` may no longer be valid.
+ sceneChanged = runSceneOp(op);
} else {
// Game-specific opcode
switch (engine->getGameId()) {
case GID_DRAGON:
- sceneChanged |= runDragonOp(op, sceneChanged);
+ sceneChanged = runDragonOp(op);
break;
case GID_HOC:
- sceneChanged |= runChinaOp(op, sceneChanged);
+ sceneChanged = runChinaOp(op);
break;
case GID_WILLY:
- sceneChanged |= runBeamishOp(op, sceneChanged);
+ sceneChanged = runBeamishOp(op);
break;
default:
error("TODO: Implement game-specific scene op for this game");
}
}
+
+ if (sceneChanged)
+ break;
}
- return !sceneChanged;
+
+ //
+ // The definition of "scene changed" returned by this function is slightly different -
+ // for the purpose of continuing to run ops above, we ignore changes to scene 2 (the
+ // inventory), but for the purpose of telling the caller, any change means they
+ // need to stop as pointers are no longer valid.
+ //
+ int16 endSceneNum = engine->getScene()->getNum();
+ return startSceneNum == endSceneNum;
}
/*static*/
diff --git a/engines/dgds/scene.h b/engines/dgds/scene.h
index 3736256cd4f..daeffe7ce92 100644
--- a/engines/dgds/scene.h
+++ b/engines/dgds/scene.h
@@ -334,10 +334,10 @@ protected:
// These are all static as they are potentially run over scene changes.
static bool checkConditions(const Common::Array<SceneConditions> &cond);
- static bool runSceneOp(const SceneOp &op, bool sceneChanged);
- static bool runDragonOp(const SceneOp &op, bool sceneChanged);
- static bool runChinaOp(const SceneOp &op, bool sceneChanged);
- static bool runBeamishOp(const SceneOp &op, bool sceneChanged);
+ static bool runSceneOp(const SceneOp &op);
+ static bool runDragonOp(const SceneOp &op);
+ static bool runChinaOp(const SceneOp &op);
+ static bool runBeamishOp(const SceneOp &op);
uint32 _magic;
Common::String _version;
More information about the Scummvm-git-logs
mailing list