SCI32: Implement syncing planes+screen items from VM for restore

Fixes restoring saved games, when ScummVM dialogs are patched in
Removing clearing planes+screen items as well, because the scripts
actually did that, but did not clear everything.
Fixes "Invalid screen item" message when entering options menu,
after restoring in SQ6.

5 files changed, 96 insertions, 3 deletions

diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index fcf951e4f2..1b56e667bf 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -1066,10 +1066,12 @@ void gamestate_restore(EngineState *s, Common::SeekableReadStream *fh) {
 	if (g_sci->_gfxScreen)
 		g_sci->_gfxScreen->clearForRestoreGame();
 #ifdef ENABLE_SCI32
-	// Also clear any SCI32 planes/screen items currently showing so they
-	// don't show up after the load.
+	// Delete current planes/elements of actively loaded VM, only when our ScummVM dialogs are patched in
+	// We MUST NOT delete all planes/screen items. At least Space Quest 6 has a few in memory like for example
+	// the options plane, which are not re-added and are in memory all the time right from the start of the
+	// game. Sierra SCI32 did not clear planes, only scripts cleared the ones inside planes::elements.
 	if (getSciVersion() >= SCI_VERSION_2)
-		g_sci->_gfxFrameout->clear();
+		g_sci->_gfxFrameout->syncWithScripts(false);
 #endif
 
 	s->reset(true);
@@ -1094,6 +1096,12 @@ void gamestate_restore(EngineState *s, Common::SeekableReadStream *fh) {
 	if (g_sci->_gfxPorts)
 		g_sci->_gfxPorts->saveLoadWithSerializer(ser);
 
+#ifdef ENABLE_SCI32
+	// Add current planes/screen elements of freshly loaded VM, only when our ScummVM dialogs are patched in
+	if (getSciVersion() >= SCI_VERSION_2)
+		g_sci->_gfxFrameout->syncWithScripts(true);
+#endif
+
 	Vocabulary *voc = g_sci->getVocabulary();
 	if (ser.getVersion() >= 30 && voc)
 		voc->saveLoadWithSerializer(ser);
diff --git a/engines/sci/engine/selector.cpp b/engines/sci/engine/selector.cpp
index b86e0ffd68..66f3793918 100644
--- a/engines/sci/engine/selector.cpp
+++ b/engines/sci/engine/selector.cpp
@@ -193,6 +193,7 @@ void Kernel::mapSelectors() {
 	FIND_SELECTOR(textBottom);
 	FIND_SELECTOR(textRight);
 	FIND_SELECTOR(magnifier);
+	FIND_SELECTOR(casts);
 #endif
 }
 
diff --git a/engines/sci/engine/selector.h b/engines/sci/engine/selector.h
index 6b724ed271..12074ed7d2 100644
--- a/engines/sci/engine/selector.h
+++ b/engines/sci/engine/selector.h
@@ -157,6 +157,8 @@ struct SelectorCache {
 	Selector textTop, textLeft, textBottom, textRight;
 
 	Selector magnifier;
+
+	Selector casts; // needed for sync'ing screen items/planes with scripts, when our save/restore code is patched in (see GfxFrameout::syncWithScripts)
 #endif
 };
 
diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 71ea4c536b..9d8a806e56 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -21,6 +21,7 @@
  */
 
 #include "common/algorithm.h"
+#include "common/config-manager.h"
 #include "common/events.h"
 #include "common/keyboard.h"
 #include "common/list.h"
@@ -149,12 +150,92 @@ void GfxFrameout::run() {
 	_planes.add(initPlane);
 }
 
+// SCI32 actually did not clear anything at all it seems on restore. The scripts actually cleared up
+// planes + screen items right before restoring. And after restoring they sync'd its internal planes list
+// as well.
 void GfxFrameout::clear() {
 	_planes.clear();
 	_visiblePlanes.clear();
 	_showList.clear();
 }
 
+// This is what Game::restore does, only needed when our ScummVM dialogs are patched in
+// It actually does one pass before actual restore deleting screen items + planes
+// And after restore it does another pass adding screen items + planes.
+void GfxFrameout::syncWithScripts(bool addElements) {
+	EngineState *engineState = g_sci->getEngineState();
+	SegManager *segMan = engineState->_segMan;
+
+	// In case original save/restore dialogs are active, don't do anything
+	if (ConfMan.getBool("originalsaveload"))
+		return;
+
+	// Get planes list object
+	reg_t planesListObject = engineState->variables[VAR_GLOBAL][10];
+	reg_t planesListElements = readSelector(segMan, planesListObject, SELECTOR(elements));
+
+	List *planesList = segMan->lookupList(planesListElements);
+	reg_t planesNodeObject = planesList->first;
+
+	// Go through all elements of planes::elements
+	while (!planesNodeObject.isNull()) {
+		Node *planesNode = segMan->lookupNode(planesNodeObject);
+		reg_t planeObject = planesNode->value;
+
+		if (addElements) {
+			// Add this plane object
+			kernelAddPlane(planeObject);
+		}
+
+		reg_t planeCastsObject = readSelector(segMan, planeObject, SELECTOR(casts));
+		reg_t setListElements = readSelector(segMan, planeCastsObject, SELECTOR(elements));
+
+		// Now go through all elements of plane::casts::elements
+		List *planeCastsList = segMan->lookupList(setListElements);
+		reg_t planeCastsNodeObject = planeCastsList->first;
+
+		while (!planeCastsNodeObject.isNull()) {
+			Node *castsNode = segMan->lookupNode(planeCastsNodeObject);
+			reg_t castObject = castsNode->value;
+
+			reg_t castListElements = readSelector(segMan, castObject, SELECTOR(elements));
+
+			List *castList = segMan->lookupList(castListElements);
+			reg_t castNodeObject = castList->first;
+
+			while (!castNodeObject.isNull()) {
+				Node *castNode = segMan->lookupNode(castNodeObject);
+				reg_t castObject = castNode->value;
+
+				// read selector "-info-" of this object
+				// TODO: Seems to have been changed for SCI3
+				uint16 castInfoSelector = readSelectorValue(segMan, castObject, SELECTOR(_info_));
+
+				if (castInfoSelector & kInfoFlagViewInserted) {
+					if (addElements) {
+						// Flag set, so add this screen item
+						kernelAddScreenItem(castObject);
+					} else {
+						// Flag set, so delete this screen item
+						kernelDeleteScreenItem(castObject);
+					}
+				}
+
+				castNodeObject = castNode->succ;
+			}
+
+			planeCastsNodeObject = castsNode->succ;
+		}
+
+		if (!addElements) {
+			// Delete this plane object
+			kernelDeletePlane(planeObject);
+		}
+
+		planesNodeObject = planesNode->succ;
+	}
+}
+
 #pragma mark -
 #pragma mark Screen items
 
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index 08ff3c05d8..0da6866f7c 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -233,6 +233,7 @@ public:
 	~GfxFrameout();
 
 	void clear();
+	void syncWithScripts(bool addElements); // this is what Game::restore does, only needed when our ScummVM dialogs are patched in
 	void run();
 
 #pragma mark -