NANCY: Improve puzzle data storage and saving

Changed the way game-specific puzzle data is stored and
saved. Scene now holds a HashMap of lazily initialized
PuzzleData objects, each of which stores data for a specific
puzzle type. This helps avoid long switch statements in
the initialization and save/load code, and helps avoid
breaking savefiles every time a new puzzle type
is implemented.

engines/nancy/action/riddlepuzzle.cpp

@@ -25,6 +25,7 @@
 #include "engines/nancy/util.h"
 #include "engines/nancy/input.h"
 #include "engines/nancy/graphics.h"
+#include "engines/nancy/puzzledata.h"
 #include "engines/nancy/state/scene.h"
 
 #include "engines/nancy/action/riddlepuzzle.h"
@@ -45,7 +46,7 @@ void RiddlePuzzle::init() {
 }
 
 void RiddlePuzzle::readData(Common::SeekableReadStream &stream) {
-	_puzzleState = NancySceneState._riddlePuzzleState;
+	_puzzleState = (RiddlePuzzleData *)NancySceneState.getPuzzleData(RiddlePuzzleData::getTag());
 	assert(_puzzleState);
 
 	_viewportTextFontID = stream.readUint16LE();

engines/nancy/action/riddlepuzzle.h

@@ -25,6 +25,9 @@
 #include "engines/nancy/action/actionrecord.h"
 
 namespace Nancy {
+
+struct RiddlePuzzleData;
+
 namespace Action {
 
 class RiddlePuzzle : public RenderActionRecord {
@@ -73,7 +76,7 @@ protected:
 	bool _playerHasHitReturn = false;
 	Common::String _playerInput;
 	uint _riddleID = 0;
-	RiddlePuzzleState *_puzzleState = nullptr;
+	RiddlePuzzleData *_puzzleState = nullptr;
 };
 
 } // End of namespace Action

engines/nancy/action/rippedletterpuzzle.cpp

@@ -26,6 +26,7 @@
 #include "engines/nancy/sound.h"
 #include "engines/nancy/input.h"
 #include "engines/nancy/cursor.h"
+#include "engines/nancy/puzzledata.h"
 
 #include "engines/nancy/state/scene.h"
 #include "engines/nancy/action/rippedletterpuzzle.h"
@@ -55,7 +56,7 @@ void RippedLetterPuzzle::registerGraphics() {
 }
 
 void RippedLetterPuzzle::readData(Common::SeekableReadStream &stream) {
-	_puzzleState = NancySceneState._rippedLetterPuzzleState;
+	_puzzleState = (RippedLetterPuzzleData *)NancySceneState.getPuzzleData(RippedLetterPuzzleData::getTag());
 	assert(_puzzleState);
 
 	readFilename(stream, _imageName);

engines/nancy/action/rippedletterpuzzle.h

@@ -25,6 +25,9 @@
 #include "engines/nancy/action/actionrecord.h"
 
 namespace Nancy {
+
+struct RippedLetterPuzzleData;
+
 namespace Action {
 
 class RippedLetterPuzzle : public RenderActionRecord {
@@ -63,7 +66,7 @@ public:
 	
 	Graphics::ManagedSurface _image;
 	SolveState _solveState = kNotSolved;
-	RippedLetterPuzzleState *_puzzleState = nullptr;
+	RippedLetterPuzzleData *_puzzleState = nullptr;
 
 protected:
 	Common::String getRecordTypeName() const override { return "RippedLetterPuzzle"; }

engines/nancy/action/sliderpuzzle.cpp

@@ -25,6 +25,7 @@
 #include "engines/nancy/sound.h"
 #include "engines/nancy/input.h"
 #include "engines/nancy/util.h"
+#include "engines/nancy/puzzledata.h"
 
 #include "engines/nancy/action/sliderpuzzle.h"
 
@@ -46,7 +47,7 @@ void SliderPuzzle::readData(Common::SeekableReadStream &stream) {
 	_spuzData = g_nancy->_sliderPuzzleData;
 	assert(_spuzData);
 
-	_puzzleState = NancySceneState._sliderPuzzleState;
+	_puzzleState = (SliderPuzzleData *)NancySceneState.getPuzzleData(SliderPuzzleData::getTag());
 	assert(_puzzleState);
 
 	readFilename(stream, _imageName);

engines/nancy/action/sliderpuzzle.h

@@ -27,6 +27,7 @@
 namespace Nancy {
 
 struct SPUZ;
+struct SliderPuzzleData;
 
 namespace Action {
 
@@ -43,7 +44,7 @@ public:
 	void handleInput(NancyInput &input) override;
 
 	SPUZ *_spuzData = nullptr;
-	SliderPuzzleState *_puzzleState = nullptr;
+	SliderPuzzleData *_puzzleState = nullptr;
 
 	Common::String _imageName;
 	uint16 _width = 0;

engines/nancy/action/towerpuzzle.cpp

@@ -25,6 +25,7 @@
 #include "engines/nancy/resource.h"
 #include "engines/nancy/sound.h"
 #include "engines/nancy/input.h"
+#include "engines/nancy/puzzledata.h"
 #include "engines/nancy/state/scene.h"
 
 #include "engines/nancy/action/towerpuzzle.h"
@@ -50,7 +51,7 @@ void TowerPuzzle::registerGraphics() {
 }
 
 void TowerPuzzle::readData(Common::SeekableReadStream &stream) {
-	_puzzleState = NancySceneState._towerPuzzleState;
+	_puzzleState = (TowerPuzzleData *)NancySceneState.getPuzzleData(TowerPuzzleData::getTag());
 	assert(_puzzleState);
 
 	readFilename(stream, _imageName);

engines/nancy/action/towerpuzzle.h

@@ -25,6 +25,9 @@
 #include "engines/nancy/action/actionrecord.h"
 
 namespace Nancy {
+	
+struct TowerPuzzleData;
+
 namespace Action {
 
 class TowerPuzzle : public RenderActionRecord {
@@ -68,7 +71,7 @@ protected:
 	int8 _heldRingID = -1;
 	int8 _heldRingPoleID = -1;
 	SolveState _solveState = kNotSolved;
-	TowerPuzzleState *_puzzleState;
+	TowerPuzzleData *_puzzleState = nullptr;
 	uint _numRings = 0;
 };
 

engines/nancy/commontypes.h

@@ -249,28 +249,6 @@ struct StaticData {
 	void readData(Common::SeekableReadStream &stream, Common::Language language);
 };
 
-// Structs for game-specific puzzle data that needs to be saved/loaded
-struct SliderPuzzleState {
-	Common::Array<Common::Array<int16>> playerTileOrder;
-	bool playerHasTriedPuzzle;
-};
-
-struct RippedLetterPuzzleState {
-	Common::Array<int8> order;
-	Common::Array<byte> rotations;
-	bool playerHasTriedPuzzle;
-};
-
-struct TowerPuzzleState {
-	Common::Array<Common::Array<int8>> order;
-	bool playerHasTriedPuzzle;
-};
-
-struct RiddlePuzzleState {
-	Common::Array<byte> solvedRiddleIDs;
-	int8 incorrectRiddleID;
-};
-
 } // End of namespace Nancy
 
 #endif // NANCY_COMMONYPES_H

engines/nancy/module.mk

@@ -48,6 +48,7 @@ MODULE_OBJS = \
   input.o \
   metaengine.o \
   nancy.o \
+  puzzledata.o \
   renderobject.o \
   resource.o \
   sound.o \

engines/nancy/nancy.h

@@ -52,7 +52,7 @@ class Serializer;
  */
 namespace Nancy {
 
-static const int kSavegameVersion = 2;
+static const int kSavegameVersion = 3;
 
 struct NancyGameDescription;
 

engines/nancy/puzzledata.cpp

@@ -0,0 +1,114 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "engines/nancy/puzzledata.h"
+
+namespace Nancy {
+
+SliderPuzzleData::SliderPuzzleData() : playerHasTriedPuzzle(false) {}
+
+void SliderPuzzleData::synchronize(Common::Serializer &ser) {
+	ser.syncAsByte(playerHasTriedPuzzle);
+
+	byte x = 0, y = 0;
+
+	if (ser.isSaving()) {
+		y = playerTileOrder.size();
+		if (y) {
+			x = playerTileOrder.back().size();
+		} else {
+			x = 0;
+		}
+	}
+
+	ser.syncAsByte(x);
+	ser.syncAsByte(y);
+
+	playerTileOrder.resize(y);
+
+	for (int i = 0; i < y; ++i) {
+		playerTileOrder[i].resize(x);
+		ser.syncArray(playerTileOrder[i].data(), x, Common::Serializer::Sint16LE);
+	}
+}
+
+RippedLetterPuzzleData::RippedLetterPuzzleData() :
+	order(24, 0),
+	rotations(24, 0),
+	playerHasTriedPuzzle(false) {}
+
+void RippedLetterPuzzleData::synchronize(Common::Serializer &ser) {
+	if (ser.isLoading()) {
+		order.resize(24);
+		rotations.resize(24);
+	}
+
+	ser.syncArray(order.data(), 24, Common::Serializer::Byte);
+	ser.syncArray(rotations.data(), 24, Common::Serializer::Byte);
+}
+
+TowerPuzzleData::TowerPuzzleData() {
+	order.resize(3, Common::Array<int8>(6, -1));
+	playerHasTriedPuzzle = false;
+}
+
+void TowerPuzzleData::synchronize(Common::Serializer &ser) {
+	ser.syncAsByte(playerHasTriedPuzzle);
+
+	if (ser.isLoading()) {
+		order.resize(3, Common::Array<int8>(6, -1));
+	}
+
+	for (uint i = 0; i < 3; ++i) {
+		ser.syncArray(order[i].data(), 6, Common::Serializer::Byte);
+	}
+}
+
+RiddlePuzzleData::RiddlePuzzleData() :
+	incorrectRiddleID(-1) {}
+
+void RiddlePuzzleData::synchronize(Common::Serializer &ser) {
+	byte numRiddles = solvedRiddleIDs.size();
+	ser.syncAsByte(numRiddles);
+
+	if (ser.isLoading()) {
+		solvedRiddleIDs.resize(numRiddles);
+	}
+
+	ser.syncArray(solvedRiddleIDs.data(), numRiddles, Common::Serializer::Byte);
+}
+
+PuzzleData *makePuzzleData(const uint32 tag) {
+	switch(tag) {
+	case SliderPuzzleData::getTag():
+		return new SliderPuzzleData();
+	case RippedLetterPuzzleData::getTag():
+		return new RippedLetterPuzzleData();
+	case TowerPuzzleData::getTag():
+		return new TowerPuzzleData();
+	case RiddlePuzzleData::getTag():
+		return new RiddlePuzzleData();
+	default:
+		return nullptr;
+	}
+}
+
+} // End of namespace Nancy

engines/nancy/puzzledata.h

@@ -0,0 +1,85 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "common/serializer.h"
+#include "common/array.h"
+
+#ifndef NANCY_PUZZLEDATA_H
+#define NANCY_PUZZLEDATA_H
+
+namespace Nancy {
+
+// The following structs contain persistent data for specific
+// puzzle types, which is to be stored in savefiles
+
+struct PuzzleData {
+	PuzzleData() {}
+	virtual ~PuzzleData() {}
+
+	virtual void synchronize(Common::Serializer &ser) = 0;
+};
+
+struct SliderPuzzleData : public PuzzleData {
+	SliderPuzzleData();
+
+	static constexpr uint32 getTag() { return MKTAG('S', 'L', 'I', 'D'); }
+	virtual void synchronize(Common::Serializer &ser);
+
+	Common::Array<Common::Array<int16>> playerTileOrder;
+	bool playerHasTriedPuzzle;
+};
+
+struct RippedLetterPuzzleData : public PuzzleData {
+	RippedLetterPuzzleData();
+
+	static constexpr uint32 getTag() { return MKTAG('R', 'I', 'P', 'L'); }
+	virtual void synchronize(Common::Serializer &ser);
+
+	Common::Array<int8> order;
+	Common::Array<byte> rotations;
+	bool playerHasTriedPuzzle;
+};
+
+struct TowerPuzzleData : public PuzzleData {
+	TowerPuzzleData();
+
+	static constexpr uint32 getTag() { return MKTAG('T', 'O', 'W', 'R'); }
+	virtual void synchronize(Common::Serializer &ser);
+
+	Common::Array<Common::Array<int8>> order;
+	bool playerHasTriedPuzzle;
+};
+
+struct RiddlePuzzleData : public PuzzleData {
+	RiddlePuzzleData();
+
+	static constexpr uint32 getTag() { return MKTAG('R', 'I', 'D', 'L'); }
+	virtual void synchronize(Common::Serializer &ser);
+
+	Common::Array<byte> solvedRiddleIDs;
+	int8 incorrectRiddleID;
+};
+
+PuzzleData *makePuzzleData(const uint32 tag);
+
+} // End of namespace Nancy
+
+#endif // NANCY_PUZZLEDATA_H

engines/nancy/state/scene.cpp

@@ -115,11 +115,7 @@ Scene::Scene() :
 		_difficulty(0),
 		_activeConversation(nullptr),
 		_lightning(nullptr),
-		_specialEffect(nullptr),
-		_sliderPuzzleState(nullptr),
-		_rippedLetterPuzzleState(nullptr),
-		_towerPuzzleState(nullptr),
-		_riddlePuzzleState(nullptr) {}
+		_specialEffect(nullptr) {}
 
 Scene::~Scene()  {
 	delete _helpButton;
@@ -130,10 +126,8 @@ Scene::~Scene()  {
 	delete _clock;
 	delete _lightning;
 	delete _specialEffect;
-	delete _sliderPuzzleState;
-	delete _rippedLetterPuzzleState;
-	delete _towerPuzzleState;
-	delete _riddlePuzzleState;
+
+	clearPuzzleData();
 }
 
 void Scene::process() {
@@ -486,7 +480,7 @@ void Scene::synchronize(Common::Serializer &ser) {
 	ser.syncAsUint32LE((uint32 &)_timers.pushedPlayTime);
 	ser.syncAsUint32LE((uint32 &)_timers.timerTime);
 	ser.syncAsByte(_timers.timerIsActive);
-	ser.skip(1); // timeOfDay; To be removed on next savefile version bump
+	ser.skip(1, 0, 2);
 
 	g_nancy->setTotalPlayTime((uint32)_timers.lastTotalTime);
 
@@ -500,79 +494,38 @@ void Scene::synchronize(Common::Serializer &ser) {
 	ser.syncAsSint16LE(_lastHintCharacter);
 	ser.syncAsSint16LE(_lastHintID);
 
-	switch (g_nancy->getGameType()) {
-	case kGameTypeVampire:
-		// Fall through to avoid having to bump the savegame version
-		// fall through
-	case kGameTypeNancy1: {
-		// Synchronize SliderPuzzle static data
-		if (!_sliderPuzzleState) {
-			return;
+	// Sync game-specific puzzle data
+
+	// Support for older savefiles
+	if (ser.getVersion() < 3 && g_nancy->getGameType() <= kGameTypeNancy1) {
+		PuzzleData *pd = getPuzzleData(SliderPuzzleData::getTag());
+		if (pd) {
+			pd->synchronize(ser);
 		}
 
-		ser.syncAsByte(_sliderPuzzleState->playerHasTriedPuzzle);
+		return;
+	}
 
-		byte x = 0, y = 0;
+	byte numPuzzleData = _puzzleData.size();
+	ser.syncAsByte(numPuzzleData);
 
-		if (ser.isSaving()) {
-			y = _sliderPuzzleState->playerTileOrder.size();
-			if (y) {
-				x = _sliderPuzzleState->playerTileOrder.back().size();
-			} else {
-				x = 0;
+	if (ser.isSaving()) {
+		for (auto pd : _puzzleData) {
+			uint32 tag = pd._key;
+			ser.syncAsUint32LE(tag);
+			pd._value->synchronize(ser);
+		}
+	} else {
+		clearPuzzleData();
+
+		uint32 tag;
+		for (uint i = 0; i < numPuzzleData; ++i) {
+			ser.syncAsUint32LE(tag);
+			PuzzleData *pd = getPuzzleData(tag);
+			if (pd) {
+				pd->synchronize(ser);
 			}
 		}
-
-		ser.syncAsByte(x);
-		ser.syncAsByte(y);
-
-		_sliderPuzzleState->playerTileOrder.resize(y);
-
-		for (int i = 0; i < y; ++i) {
-			_sliderPuzzleState->playerTileOrder[i].resize(x);
-			ser.syncArray(_sliderPuzzleState->playerTileOrder[i].data(), x, Common::Serializer::Sint16LE);
-		}
-
-		break;
-	}
-	case kGameTypeNancy2 : {
-		if (!_rippedLetterPuzzleState || !_towerPuzzleState || !_riddlePuzzleState) {
-			break;
-		}
-
-		ser.syncAsByte(_rippedLetterPuzzleState->playerHasTriedPuzzle);
-
-		if (ser.isLoading()) {
-			_rippedLetterPuzzleState->order.resize(24);
-			_rippedLetterPuzzleState->rotations.resize(24);
-		}
-
-		ser.syncArray(_rippedLetterPuzzleState->order.data(), 24, Common::Serializer::Byte);
-		ser.syncArray(_rippedLetterPuzzleState->rotations.data(), 24, Common::Serializer::Byte);
-
-		ser.syncAsByte(_towerPuzzleState->playerHasTriedPuzzle);
-
-		if (ser.isLoading()) {
-			_towerPuzzleState->order.resize(3, Common::Array<int8>(6, -1));
-		}
-
-		for (uint i = 0; i < 3; ++i) {
-			ser.syncArray(_towerPuzzleState->order[i].data(), 6, Common::Serializer::Byte);
-		}
-
-		byte numRiddles = _riddlePuzzleState->solvedRiddleIDs.size();
-		ser.syncAsByte(numRiddles);
-
-		if (ser.isLoading()) {
-			_riddlePuzzleState->solvedRiddleIDs.resize(numRiddles);
-		}
-
-		ser.syncArray(_riddlePuzzleState->solvedRiddleIDs.data(), numRiddles, Common::Serializer::Byte);
-
-		break;
-	}
-	default:
-		break;
 	}
 }
 
@@ -604,38 +557,6 @@ void Scene::init() {
 		_lastHintCharacter = _lastHintID = -1;
 	}
 
-	// Initialize game-specific data
-	switch (g_nancy->getGameType()) {
-	case kGameTypeVampire:
-		// Fall through to avoid having to bump the savefile version
-		// fall through
-	case kGameTypeNancy1:
-		delete _sliderPuzzleState;
-		_sliderPuzzleState = new SliderPuzzleState();
-		_sliderPuzzleState->playerHasTriedPuzzle = false;
-		
-		break;
-	case kGameTypeNancy2:
-		delete _rippedLetterPuzzleState;
-		_rippedLetterPuzzleState = new RippedLetterPuzzleState();
-		_rippedLetterPuzzleState->playerHasTriedPuzzle = false;
-		_rippedLetterPuzzleState->order.resize(24, 0);
-		_rippedLetterPuzzleState->rotations.resize(24, 0);
-
-		delete _towerPuzzleState;
-		_towerPuzzleState = new TowerPuzzleState();
-		_towerPuzzleState->playerHasTriedPuzzle = false;
-		_towerPuzzleState->order.resize(3, Common::Array<int8>(6, -1));
-
-		delete _riddlePuzzleState;
-		_riddlePuzzleState = new RiddlePuzzleState();
-		_riddlePuzzleState->incorrectRiddleID = -1;
-		
-		break;
-	default:
-		break;
-	}
-
 	initStaticData();
 
 	if (ConfMan.hasKey("save_slot")) {
@@ -680,6 +601,22 @@ void Scene::specialEffect(byte type, uint16 fadeToBlackTime, uint16 frameTime) {
 	_specialEffect->init();
 }
 
+PuzzleData *Scene::getPuzzleData(const uint32 tag) {
+	// Lazy initialization ensures both init() and synchronize() will not need
+	// to care about which puzzles a specific game has
+
+	if (_puzzleData.contains(tag)) {
+		return _puzzleData[tag];
+	} else {
+		PuzzleData *newData = makePuzzleData(tag);
+		if (newData) {
+			_puzzleData.setVal(tag, newData);
+		}
+		
+		return newData;
+	}
+}
+
 void Scene::load() {
 	clearSceneData();
 
@@ -950,5 +887,11 @@ void Scene::clearSceneData() {
 	}
 }
 
+void Scene::clearPuzzleData() {
+	for (auto &pd : _puzzleData) {
+		delete pd._value;
+	}
+}
+
 } // End of namespace State
 } // End of namespace Nancy

engines/nancy/state/scene.h

@@ -25,6 +25,7 @@
 #include "common/singleton.h"
 
 #include "engines/nancy/commontypes.h"
+#include "engines/nancy/puzzledata.h"
 
 #include "engines/nancy/action/actionmanager.h"
 
@@ -190,11 +191,8 @@ public:
 	// Used from nancy2 onwards
 	void specialEffect(byte type, uint16 fadeToBlackTime, uint16 frameTime);
 
-	// Game-specific data that needs to be saved/loaded
-	SliderPuzzleState *_sliderPuzzleState;
-	RippedLetterPuzzleState *_rippedLetterPuzzleState;
-	TowerPuzzleState *_towerPuzzleState;
-	RiddlePuzzleState *_riddlePuzzleState;
+	// Get the persistent data for a given puzzle type
+	PuzzleData *getPuzzleData(const uint32 tag);
 
 private:
 	void init();
@@ -205,6 +203,7 @@ private:
 	void initStaticData();
 
 	void clearSceneData();
+	void clearPuzzleData();
 
 	enum State {
 		kInit,
@@ -262,6 +261,8 @@ private:
 	UI::InventoryBoxOrnaments *_inventoryBoxOrnaments;
 	UI::Clock *_clock;
 
+	Common::Rect _mapHotspot;
+
 	// General data
 	SceneState _sceneState;
 	PlayFlags _flags;
@@ -275,7 +276,7 @@ private:
 	Misc::Lightning *_lightning;
 	Misc::SpecialEffect *_specialEffect;
 
-	Common::Rect _mapHotspot;
+	Common::HashMap<uint32, PuzzleData *> _puzzleData;
 
 	Action::ActionManager _actionManager;
 	Action::ConversationSound *_activeConversation;