SCUMM: Replace UB-triggering serialization code with Common::Serializer

Fixes Trac#10342.

3 files changed, 78 insertions, 77 deletions

diff --git a/engines/scumm/he/intern_he.h b/engines/scumm/he/intern_he.h
index bd3cdb2c57..25dd31f007 100644
--- a/engines/scumm/he/intern_he.h
+++ b/engines/scumm/he/intern_he.h
@@ -73,7 +73,7 @@ public:
 protected:
 	virtual void setupOpcodes();
 
-	virtual void saveOrLoad(Serializer *s);
+	virtual void saveLoadWithSerializer(Common::Serializer &s);
 
 	void localizeArray(int slot, byte scriptSlot);
 	void redimArray(int arrayId, int newX, int newY, int d);
@@ -163,7 +163,7 @@ protected:
 	virtual void setupScummVars();
 	virtual void resetScummVars();
 
-	virtual void saveOrLoad(Serializer *s);
+	virtual void saveLoadWithSerializer(Common::Serializer &s);
 
 	virtual void readRoomsOffsets();
 	virtual void readGlobalObjects();
@@ -222,7 +222,7 @@ public:
 protected:
 	virtual void setupOpcodes();
 
-	virtual void saveOrLoad(Serializer *s);
+	virtual void saveLoadWithSerializer(Common::Serializer &s);
 
 	virtual void redrawBGAreas();
 
@@ -490,7 +490,7 @@ protected:
 	virtual void processInput();
 	virtual void clearClickedStatus();
 
-	virtual void saveOrLoad(Serializer *s);
+	virtual void saveLoadWithSerializer(Common::Serializer &s);
 
 	virtual void readMAXS(int blockSize);
 	void setResourceOffHeap(int typeId, int resId, int val);
@@ -581,7 +581,7 @@ protected:
 
 	virtual void readMAXS(int blockSize);
 
-	virtual void saveOrLoad(Serializer *s);
+	virtual void saveLoadWithSerializer(Common::Serializer &s);
 
 	virtual void copyPalColor(int dst, int src);
 	virtual void darkenPalette(int redScale, int greenScale, int blueScale, int startColor, int endColor);
@@ -613,7 +613,7 @@ public:
 protected:
 	virtual void setupOpcodes();
 
-	virtual void saveOrLoad(Serializer *s);
+	virtual void saveLoadWithSerializer(Common::Serializer &s);
 
 	virtual void decodeParseString(int a, int b);
 
diff --git a/engines/scumm/he/sprite_he.cpp b/engines/scumm/he/sprite_he.cpp
index bef1c0b5eb..16c3b50365 100644
--- a/engines/scumm/he/sprite_he.cpp
+++ b/engines/scumm/he/sprite_he.cpp
@@ -1389,80 +1389,79 @@ void Sprite::processImages(bool arg) {
 	}
 }
 
-void Sprite::saveOrLoadSpriteData(Serializer *s) {
-	static const SaveLoadEntry spriteEntries[] = {
-		MKLINE(SpriteInfo, id, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, zorder, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, flags, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, image, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, imageState, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, group, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, palette, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, priority, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, bbox.left, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, bbox.top, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, bbox.right, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, bbox.bottom, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, dx, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, dy, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, pos.x, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, pos.y, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, tx, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, ty, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, userValue, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, curImageState, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, curImage, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, imglistNum, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, shadow, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, imageStateCount, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, angle, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, scale, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, animProgress, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, curAngle, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, curScale, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, curImgFlags, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, animIndex, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, animSpeed, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, sourceImage, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, maskImage, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, zbufferImage, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, classFlags, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, imgFlags, sleInt32, VER(48)),
-		MKLINE(SpriteInfo, conditionBits, sleInt32, VER(48)),
-		MKEND()
-	};
-
-	static const SaveLoadEntry spriteGroupEntries[] = {
-		MKLINE(SpriteGroup, bbox.left, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, bbox.top, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, bbox.right, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, bbox.bottom, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, priority, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, flags, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, tx, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, ty, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, image, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, scaling, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, scale_x_ratio_mul, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, scale_x_ratio_div, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, scale_y_ratio_mul, sleInt32, VER(48)),
-		MKLINE(SpriteGroup, scale_y_ratio_div, sleInt32, VER(48)),
-		MKEND()
-	};
-
-	if (s->getVersion() >= VER(64)) {
-		s->saveLoadArrayOf(_spriteTable, _varNumSprites + 1, sizeof(_spriteTable[0]), spriteEntries);
-		s->saveLoadArrayOf(_spriteGroups, _varNumSpriteGroups + 1, sizeof(_spriteGroups[0]), spriteGroupEntries);
+static void syncWithSerializer(Common::Serializer &s, SpriteInfo &si) {
+	s.syncAsSint32LE(si.id, VER(48));
+	s.syncAsSint32LE(si.zorder, VER(48));
+	s.syncAsSint32LE(si.flags, VER(48));
+	s.syncAsSint32LE(si.image, VER(48));
+	s.syncAsSint32LE(si.imageState, VER(48));
+	s.syncAsSint32LE(si.group, VER(48));
+	s.syncAsSint32LE(si.palette, VER(48));
+	s.syncAsSint32LE(si.priority, VER(48));
+	s.syncAsSint32LE(si.bbox.left, VER(48));
+	s.syncAsSint32LE(si.bbox.top, VER(48));
+	s.syncAsSint32LE(si.bbox.right, VER(48));
+	s.syncAsSint32LE(si.bbox.bottom, VER(48));
+	s.syncAsSint32LE(si.dx, VER(48));
+	s.syncAsSint32LE(si.dy, VER(48));
+	s.syncAsSint32LE(si.pos.x, VER(48));
+	s.syncAsSint32LE(si.pos.y, VER(48));
+	s.syncAsSint32LE(si.tx, VER(48));
+	s.syncAsSint32LE(si.ty, VER(48));
+	s.syncAsSint32LE(si.userValue, VER(48));
+	s.syncAsSint32LE(si.curImageState, VER(48));
+	s.syncAsSint32LE(si.curImage, VER(48));
+	s.syncAsSint32LE(si.imglistNum, VER(48));
+	s.syncAsSint32LE(si.shadow, VER(48));
+	s.syncAsSint32LE(si.imageStateCount, VER(48));
+	s.syncAsSint32LE(si.angle, VER(48));
+	s.syncAsSint32LE(si.scale, VER(48));
+	s.syncAsSint32LE(si.animProgress, VER(48));
+	s.syncAsSint32LE(si.curAngle, VER(48));
+	s.syncAsSint32LE(si.curScale, VER(48));
+	s.syncAsSint32LE(si.curImgFlags, VER(48));
+	s.syncAsSint32LE(si.animIndex, VER(48));
+	s.syncAsSint32LE(si.animSpeed, VER(48));
+	s.syncAsSint32LE(si.sourceImage, VER(48));
+	s.syncAsSint32LE(si.maskImage, VER(48));
+	s.syncAsSint32LE(si.zbufferImage, VER(48));
+	s.syncAsSint32LE(si.classFlags, VER(48));
+	s.syncAsSint32LE(si.imgFlags, VER(48));
+	s.syncAsSint32LE(si.conditionBits, VER(48));
+}
+
+static void syncWithSerializer(Common::Serializer &s, SpriteGroup &sg) {
+	s.syncAsSint32LE(sg.bbox.left, VER(48));
+	s.syncAsSint32LE(sg.bbox.top, VER(48));
+	s.syncAsSint32LE(sg.bbox.right, VER(48));
+	s.syncAsSint32LE(sg.bbox.bottom, VER(48));
+	s.syncAsSint32LE(sg.priority, VER(48));
+	s.syncAsSint32LE(sg.flags, VER(48));
+	s.syncAsSint32LE(sg.tx, VER(48));
+	s.syncAsSint32LE(sg.ty, VER(48));
+	s.syncAsSint32LE(sg.image, VER(48));
+	s.syncAsSint32LE(sg.scaling, VER(48));
+	s.syncAsSint32LE(sg.scale_x_ratio_mul, VER(48));
+	s.syncAsSint32LE(sg.scale_x_ratio_div, VER(48));
+	s.syncAsSint32LE(sg.scale_y_ratio_mul, VER(48));
+	s.syncAsSint32LE(sg.scale_y_ratio_div, VER(48));
+}
+
+void Sprite::saveLoadWithSerializer(Common::Serializer &s) {
+	if (s.getVersion() >= VER(64)) {
+		s.syncArray(_spriteTable, _varNumSprites + 1, syncWithSerializer);
+		s.syncArray(_spriteGroups, _varNumSpriteGroups + 1, syncWithSerializer);
 	} else {
-		s->saveLoadArrayOf(_activeSpritesTable, _varNumSprites, sizeof(_activeSpritesTable[0]), spriteEntries);
-		s->saveLoadArrayOf(_spriteTable, _varNumSprites, sizeof(_spriteTable[0]), spriteEntries);
-		s->saveLoadArrayOf(_spriteGroups, _varNumSpriteGroups, sizeof(_spriteGroups[0]), spriteGroupEntries);
+		// TODO: This had been bogus, what is it really supposed to do?
+//		s->saveLoadArrayOf(_activeSpritesTable, _varNumSprites, sizeof(_activeSpritesTable[0]), spriteEntries);
+		s.syncArray(*_activeSpritesTable, _varNumSprites, syncWithSerializer);
+		s.syncArray(_spriteTable, _varNumSprites, syncWithSerializer);
+		s.syncArray(_spriteGroups, _varNumSpriteGroups, syncWithSerializer);
 	}
 
 	// Reset active sprite table
-	if (s->isLoading())
+	if (s.isLoading())
 		_numSpritesToProcess = 0;
-
 }
 
 } // End of namespace Scumm
diff --git a/engines/scumm/he/sprite_he.h b/engines/scumm/he/sprite_he.h
index 3ea6bb9f84..77e527b3ce 100644
--- a/engines/scumm/he/sprite_he.h
+++ b/engines/scumm/he/sprite_he.h
@@ -23,6 +23,8 @@
 #if !defined(SCUMM_HE_SPRITE_HE_H) && defined(ENABLE_HE)
 #define SCUMM_HE_SPRITE_HE_H
 
+#include "common/serializer.h"
+
 namespace Scumm {
 
 enum SpriteFlags {
@@ -98,7 +100,7 @@ struct SpriteGroup {
 
 class ScummEngine_v90he;
 
-class Sprite {
+class Sprite : public Common::Serializable {
 public:
 	Sprite(ScummEngine_v90he *vm);
 	virtual ~Sprite();
@@ -112,7 +114,7 @@ public:
 	int32 _varNumSprites;
 	int32 _varMaxSprites;
 
-	void saveOrLoadSpriteData(Serializer *s);
+	void saveLoadWithSerializer(Common::Serializer &s);
 	void resetBackground();
 	void setRedrawFlags(bool checkZOrder);
 	void sortActiveSprites();