- removed the sequential processing of JSON objects because the benefit is too small.

After testing with a savegame on ZDCMP2 which is probably the largest map in existence, timing both methods resulted in a speed difference of less than 40 ms (70 vs 110 ms for reading all sectory, linedefs, sidedefs and objects).
This compares to an overall restoration time, including reloading the level, precaching all textures and setting everything up, of approx. 1.2 s, meaning an increase of 3% of the entire reloading time.
That's simply not worth all the negative side effects that may happen with a method that highly depends on proper code construction.
On the other hand, using random access means that a savegame version change is only needed now when the semantics of a field change, but not if some get added or deleted.

- do not I_Error out in the serializer unless caused by a programming error.

It is better to let the serializer finish, collect all the errors and I_Error out when the game is known to be in a stable enough state to allow unwinding.

src/d_netinfo.cpp

@@ -931,7 +931,7 @@ void ReadUserInfo(FSerializer &arc, userinfo_t &info, FString &skin)
 	{
 		while ((key = arc.GetKey()))
 		{
-			arc.StringPtr(key, str);
+			arc.StringPtr(nullptr, str);
 			name = key;
 			cvar = info.CheckKey(name);
 			if (cvar != NULL && *cvar != NULL)

src/dobjtype.cpp

@@ -2250,7 +2250,7 @@ bool PStruct::ReadFields(FSerializer &ar, void *addr) const
 		}
 		else
 		{
-			readsomething |= static_cast<const PField *>(sym)->Type->ReadValue(ar, label,
+			readsomething |= static_cast<const PField *>(sym)->Type->ReadValue(ar, nullptr,
 				(BYTE *)addr + static_cast<const PField *>(sym)->Offset);
 		}
 	}
@@ -2543,11 +2543,19 @@ bool PClass::ReadAllFields(FSerializer &ar, void *addr) const
 	bool readsomething = false;
 	bool foundsomething = false;
 	const char *key;
+	key = ar.GetKey();
+	if (strcmp(key, "classtype"))
+	{
+		// this does not represent a DObject
+		Printf(TEXTCOLOR_RED "trying to read user variables but got a non-object (first key is '%s')", key);
+		ar.mErrors++;
+		return false;
+	}
 	while ((key = ar.GetKey()))
 	{
 		if (strncmp(key, "class:", 6))
 		{
-			// This key does not represent any class fields anymore
+			// We have read all user variable blocks.
 			break;
 		}
 		foundsomething = true;
@@ -2565,7 +2573,11 @@ bool PClass::ReadAllFields(FSerializer &ar, void *addr) const
 			}
 			if (parent != nullptr)
 			{
-				readsomething |= type->ReadFields(ar, addr);
+				if (ar.BeginObject(nullptr))
+				{
+					readsomething |= type->ReadFields(ar, addr);
+					ar.EndObject();
+				}
 			}
 			else
 			{

src/dthinker.cpp

@@ -394,8 +394,12 @@ void DThinker::Tick ()
 
 size_t DThinker::PropagateMark()
 {
-	assert(NextThinker != NULL && !(NextThinker->ObjectFlags & OF_EuthanizeMe));
+	// Do not choke on partially initialized objects (as happens when loading a savegame fails)
-	assert(PrevThinker != NULL && !(PrevThinker->ObjectFlags & OF_EuthanizeMe));
+	if (NextThinker != nullptr || PrevThinker != nullptr)
+	{
+		assert(NextThinker != nullptr && !(NextThinker->ObjectFlags & OF_EuthanizeMe));
+		assert(PrevThinker != nullptr && !(PrevThinker->ObjectFlags & OF_EuthanizeMe));
+	}
 	GC::Mark(NextThinker);
 	GC::Mark(PrevThinker);
 	return Super::PropagateMark();

src/dthinker.h

@@ -105,6 +105,7 @@ private:
 	friend struct FThinkerList;
 	friend class FThinkerIterator;
 	friend class DObject;
+	friend class FSerializer;
 
 	DThinker *NextThinker, *PrevThinker;
 };

src/g_game.cpp

@@ -27,6 +27,7 @@
 #include <stdio.h>
 #include <stddef.h>
 #include <time.h>
+#include <memory>
 #ifdef __APPLE__
 #include <CoreServices/CoreServices.h>
 #endif
@@ -1844,154 +1845,162 @@ void G_DoLoadGame ()
 		Printf ("Could not read savegame '%s'\n", savename.GetChars());
 		return;
 	}
-
+	try
-	FResourceLump *info = resfile->FindLump("info.json");
-	if (info == nullptr)
 	{
-		delete resfile;
+		FResourceLump *info = resfile->FindLump("info.json");
-		Printf ("'%s' is not a valid savegame: Missing 'info.json'.\n", savename.GetChars());
+		if (info == nullptr)
-		return;
-	}
-
-	SaveVersion = 0;
-
-	void *data = info->CacheLump();
-	FSerializer arc;
-	if (!arc.OpenReader((const char *)data, info->LumpSize))
-	{
-		Printf("Failed to access savegame info\n");
-		delete resfile;
-		return;
-	}
-
-	// Check whether this savegame actually has been created by a compatible engine.
-	// Since there are ZDoom derivates using the exact same savegame format but
-	// with mutual incompatibilities this check simplifies things significantly.
-	FString savever, engine, map;
-	arc("Save Version", SaveVersion);
-	arc("Engine", engine);
-	arc("Current Map", map);
-
-	if (engine.CompareNoCase(GAMESIG) != 0)
-	{
-		// Make a special case for the message printed for old savegames that don't
-		// have this information.
-		if (engine.IsEmpty())
 		{
-			Printf ("Savegame is from an incompatible version\n");
+			delete resfile;
+			Printf("'%s' is not a valid savegame: Missing 'info.json'.\n", savename.GetChars());
+			return;
 		}
-		else
+
+		SaveVersion = 0;
+
+		void *data = info->CacheLump();
+		FSerializer arc;
+		if (!arc.OpenReader((const char *)data, info->LumpSize))
 		{
-			Printf ("Savegame is from another ZDoom-based engine: %s\n", engine);
+			Printf("Failed to access savegame info\n");
+			delete resfile;
+			return;
 		}
-		delete resfile;
-		return;
-	}
 
-	if (SaveVersion < MINSAVEVER || SaveVersion > SAVEVER)
+		// Check whether this savegame actually has been created by a compatible engine.
-	{
+		// Since there are ZDoom derivates using the exact same savegame format but
-		delete resfile;
+		// with mutual incompatibilities this check simplifies things significantly.
-		Printf ("Savegame is from an incompatible version");
+		FString savever, engine, map;
-		if (SaveVersion < MINSAVEVER)
+		arc("Save Version", SaveVersion);
+		arc("Engine", engine);
+		arc("Current Map", map);
+
+		if (engine.CompareNoCase(GAMESIG) != 0)
 		{
-			Printf(": %d (%d is the oldest supported)", SaveVersion, MINSAVEVER);
+			// Make a special case for the message printed for old savegames that don't
+			// have this information.
+			if (engine.IsEmpty())
+			{
+				Printf("Savegame is from an incompatible version\n");
+			}
+			else
+			{
+				Printf("Savegame is from another ZDoom-based engine: %s\n", engine);
+			}
+			delete resfile;
+			return;
 		}
-		else
+
+		if (SaveVersion < MINSAVEVER || SaveVersion > SAVEVER)
 		{
-			Printf(": %d (%d is the highest supported)", SaveVersion, SAVEVER);
+			delete resfile;
+			Printf("Savegame is from an incompatible version");
+			if (SaveVersion < MINSAVEVER)
+			{
+				Printf(": %d (%d is the oldest supported)", SaveVersion, MINSAVEVER);
+			}
+			else
+			{
+				Printf(": %d (%d is the highest supported)", SaveVersion, SAVEVER);
+			}
+			Printf("\n");
+			return;
 		}
-		Printf("\n");
+
+		if (!G_CheckSaveGameWads(arc, true))
+		{
+			delete resfile;
+			return;
+		}
+
+		if (map.IsEmpty())
+		{
+			Printf("Savegame is missing the current map\n");
+			delete resfile;
+			return;
+		}
+
+		// Now that it looks like we can load this save, hide the fullscreen console if it was up
+		// when the game was selected from the menu.
+		if (hidecon && gamestate == GS_FULLCONSOLE)
+		{
+			gamestate = GS_HIDECONSOLE;
+		}
+		// we are done with info.json.
+		arc.Close();
+
+		info = resfile->FindLump("globals.json");
+		if (info == nullptr)
+		{
+			delete resfile;
+			Printf("'%s' is not a valid savegame: Missing 'globals.json'.\n", savename.GetChars());
+			return;
+		}
+
+		data = info->CacheLump();
+		if (!arc.OpenReader((const char *)data, info->LumpSize))
+		{
+			Printf("Failed to access savegame info\n");
+			delete resfile;
+			return;
+		}
+
+
+		// Read intermission data for hubs
+		G_SerializeHub(arc);
+
+		bglobal.RemoveAllBots(true);
+
+		FString cvar;
+		arc("importantcvars", cvar);
+		if (!cvar.IsEmpty())
+		{
+			BYTE *vars_p = (BYTE *)cvar.GetChars();
+			C_ReadCVars(&vars_p);
+		}
+
+		DWORD time[2] = { 1,0 };
+
+		arc("ticrate", time[0])
+			("leveltime", time[1]);
+		// dearchive all the modifications
+		level.time = Scale(time[1], TICRATE, time[0]);
+
+		G_ReadSnapshots(resfile);
+		delete resfile;	// we no longer need the resource file below this point
+		resfile = nullptr;
+		G_ReadVisited(arc);
+
+		// load a base level
+		savegamerestore = true;		// Use the player actors in the savegame
+		bool demoplaybacksave = demoplayback;
+		G_InitNew(map, false);
+		demoplayback = demoplaybacksave;
+		savegamerestore = false;
+
+		STAT_Serialize(arc);
+		FRandom::StaticReadRNGState(arc);
+		P_ReadACSDefereds(arc);
+		P_ReadACSVars(arc);
+
+		NextSkill = -1;
+		arc("nextskill", NextSkill);
+
+		if (level.info != nullptr)
+			level.info->Snapshot.Clean();
+
+		BackupSaveName = savename;
+
+		// At this point, the GC threshold is likely a lot higher than the
+		// amount of memory in use, so bring it down now by starting a
+		// collection.
+		GC::StartCollection();
+	}
+	catch (...)
+	{
+		// delete the resource file if anything goes wrong in here.
+		if (resfile != nullptr) delete resfile;
 		return;
 	}
-
-	if (!G_CheckSaveGameWads (arc, true))
-	{
-		delete resfile;
-		return;
-	}
-
-	if (map.IsEmpty())
-	{
-		Printf ("Savegame is missing the current map\n");
-		delete resfile;
-		return;
-	}
-
-	// Now that it looks like we can load this save, hide the fullscreen console if it was up
-	// when the game was selected from the menu.
-	if (hidecon && gamestate == GS_FULLCONSOLE)
-	{
-		gamestate = GS_HIDECONSOLE;
-	}
-	// we are done with info.json.
-	arc.Close();
-
-	info = resfile->FindLump("globals.json");
-	if (info == nullptr)
-	{
-		delete resfile;
-		Printf("'%s' is not a valid savegame: Missing 'globals.json'.\n", savename.GetChars());
-		return;
-	}
-
-	data = info->CacheLump();
-	if (!arc.OpenReader((const char *)data, info->LumpSize))
-	{
-		Printf("Failed to access savegame info\n");
-		delete resfile;
-		return;
-	}
-
-
-	// Read intermission data for hubs
-	G_SerializeHub(arc);
-
-	bglobal.RemoveAllBots (true);
-
-	FString cvar;
-	arc("importantcvars",cvar);
-	if (!cvar.IsEmpty())
-	{
-		BYTE *vars_p = (BYTE *)cvar.GetChars();
-		C_ReadCVars (&vars_p);
-	}
-
-	DWORD time[2] = { 1,0 };
-
-	arc("ticrate", time[0])
-		("leveltime", time[1]);
-	// dearchive all the modifications
-	level.time = Scale (time[1], TICRATE, time[0]);
-
-	G_ReadSnapshots(resfile);
-	G_ReadVisited(arc);
-
-	// load a base level
-	savegamerestore = true;		// Use the player actors in the savegame
-	bool demoplaybacksave = demoplayback;
-	G_InitNew (map, false);
-	demoplayback = demoplaybacksave;
-	savegamerestore = false;
-
-	STAT_Serialize(arc);
-	FRandom::StaticReadRNGState(arc);
-	P_ReadACSDefereds(arc);
-	P_ReadACSVars(arc);
-
-	NextSkill = -1;
-	arc("nextskill", NextSkill);
-
-	if (level.info != nullptr)
-		level.info->Snapshot.Clean();
-
-	BackupSaveName = savename;
-
-	delete resfile;
-
-	// At this point, the GC threshold is likely a lot higher than the
-	// amount of memory in use, so bring it down now by starting a
-	// collection.
-	GC::StartCollection();
 }
 
 

src/g_level.cpp

@@ -1773,7 +1773,7 @@ void P_ReadACSDefereds (FSerializer &arc)
 			{
 				I_Error("Unknown map '%s' in savegame", key);
 			}
-			arc(key, i->deferred);
+			arc(nullptr, i->deferred);
 		}
 		arc.EndObject();
 	}

src/g_shared/a_sectoraction.cpp

@@ -69,6 +69,7 @@ void ASectorAction::Destroy ()
 		{
 			*prev.act = probe->tracer;
 		}
+		Sector = nullptr;
 	}
 
 	Super::Destroy ();

src/p_acs.cpp

@@ -1051,13 +1051,13 @@ static void ReadArrayVars (FSerializer &file, FWorldGlobalArray *vars, size_t co
 		while ((arraykey = file.GetKey()))
 		{
 			int i = (int)strtol(arraykey, nullptr, 10);
-			if (file.BeginObject(arraykey))
+			if (file.BeginObject(nullptr))
 			{
 				while ((arraykey = file.GetKey()))
 				{
 					int k = (int)strtol(arraykey, nullptr, 10);
 					int val;
-					file(arraykey, val);
+					file(nullptr, val);
 					vars[i].Insert(k, val);
 				}
 				file.EndObject();
@@ -1612,10 +1612,9 @@ void FBehavior::StaticSerializeModuleStates (FSerializer &arc)
 {
 	auto modnum = StaticModules.Size();
 
-	if (arc.BeginObject("acsmodules"))
+	if (arc.BeginArray("acsmodules"))
 	{
-		arc("count", modnum);
+		int modnum = arc.ArraySize();
-
 		if (modnum != StaticModules.Size())
 		{
 			I_Error("Level was saved with a different number of ACS modules. (Have %d, save has %d)", StaticModules.Size(), modnum);
@@ -1623,35 +1622,31 @@ void FBehavior::StaticSerializeModuleStates (FSerializer &arc)
 
 		for (modnum = 0; modnum < StaticModules.Size(); ++modnum)
 		{
-			if (arc.BeginArray("modules"))
+			FBehavior *module = StaticModules[modnum];
+			const char *modname = module->ModuleName;
+			int ModSize = module->GetDataSize();
+
+			if (arc.BeginObject(nullptr))
 			{
-				FBehavior *module = StaticModules[modnum];
+				arc.StringPtr("modname", modname)
-				const char *modname = module->ModuleName;
+					("modsize", ModSize);
-				int ModSize = module->GetDataSize();
 
-				if (arc.BeginObject(nullptr))
+				if (arc.isReading())
 				{
-					arc.StringPtr("modname", modname)
+					if (stricmp(modname, module->ModuleName) != 0)
-						("modsize", ModSize);
-
-					if (arc.isReading())
 					{
-						if (stricmp(modname, module->ModuleName) != 0)
+						I_Error("Level was saved with a different set or order of ACS modules. (Have %s, save has %s)", module->ModuleName, modname);
-						{
+					}
-							I_Error("Level was saved with a different set or order of ACS modules. (Have %s, save has %s)", module->ModuleName, modname);
+					else if (ModSize != module->GetDataSize())
-						}
+					{
-						else if (ModSize != module->GetDataSize())
+						I_Error("ACS module %s has changed from what was saved. (Have %d bytes, save has %d bytes)", module->ModuleName, module->GetDataSize(), ModSize);
-						{
-							I_Error("ACS module %s has changed from what was saved. (Have %d bytes, save has %d bytes)", module->ModuleName, module->GetDataSize(), ModSize);
-						}
 					}
-					module->SerializeVars(arc);
-					arc.EndObject();
 				}
-				arc.EndArray();
+				module->SerializeVars(arc);
+				arc.EndObject();
 			}
 		}
-		arc.EndObject();
+		arc.EndArray();
 	}
 }
 

src/p_saveg.cpp

@@ -904,6 +904,7 @@ void G_SerializeLevel(FSerializer &arc, bool hubload)
 	if (arc.isReading())
 	{
 		P_DestroyThinkers(hubload);
+		interpolator.ClearInterpolations();
 		arc.ReadObjects();
 	}
 
@@ -933,15 +934,14 @@ void G_SerializeLevel(FSerializer &arc, bool hubload)
 		sky1texture = level.skytexture1;
 		sky2texture = level.skytexture2;
 		R_InitSkyMap();
-		interpolator.ClearInterpolations();
+		G_AirControlChanged();
 	}
 
 
-	G_AirControlChanged();
 
 	// fixme: This needs to ensure it reads from the correct place. Should be one once there's enough of this code converted to JSON
 
-	FBehavior::StaticSerializeModuleStates(arc);
+//FBehavior::StaticSerializeModuleStates(arc);
 	// The order here is important: First world state, then portal state, then thinkers, and last polyobjects.
 	arc.Array("linedefs", lines, &loadlines[0], numlines);
 	arc.Array("sidedefs", sides, &loadsides[0], numsides);
@@ -950,7 +950,8 @@ void G_SerializeLevel(FSerializer &arc, bool hubload)
 	arc("lineportals", linePortals);
 	arc("sectorportals", sectorPortals);
 	if (arc.isReading()) P_CollectLinkedPortals();
-	DThinker::SerializeThinkers(arc, !hubload);
+
+//	DThinker::SerializeThinkers(arc, !hubload);
 	arc.Array("polyobjs", polyobjs, po_NumPolyobjs);
 	arc("subsectors", subsectors);
 	StatusBar->SerializeMessages(arc);
@@ -959,6 +960,7 @@ void G_SerializeLevel(FSerializer &arc, bool hubload)
 	FCanvasTextureInfo::Serialize(arc);
 	P_SerializePlayers(arc, hubload);
 	P_SerializeSounds(arc);
+
 	if (arc.isReading())
 	{
 		for (int i = 0; i < numsectors; i++)

src/serializer.cpp

@@ -28,6 +28,7 @@
 #include "v_font.h"
 #include "w_zip.h"
 #include "doomerrors.h"
+#include "v_text.h"
 
 char nulspace[1024 * 1024 * 4];
 
@@ -42,12 +43,10 @@ struct FJSONObject
 	rapidjson::Value *mObject;
 	rapidjson::Value::MemberIterator mIterator;
 	int mIndex;
-	bool mRandomAccess;
 
-	FJSONObject(rapidjson::Value *v, bool randomaccess = false)
+	FJSONObject(rapidjson::Value *v)
 	{
 		mObject = v;
-		mRandomAccess = randomaccess;
 		if (v->IsObject()) mIterator = v->MemberBegin();
 		else if (v->IsArray())
 		{
@@ -198,37 +197,34 @@ struct FReader
 	TArray<FJSONObject> mObjects;
 	rapidjson::Document mDoc;
 	TArray<DObject *> mDObjects;
+	rapidjson::Value *mKeyValue = nullptr;
 	int mPlayers[MAXPLAYERS];
-	bool mObjectsRead;
+	bool mObjectsRead = false;
 
-	FReader(const char *buffer, size_t length, bool randomaccess)
+	FReader(const char *buffer, size_t length)
 	{
 		rapidjson::Document doc;
 		mDoc.Parse(buffer, length);
-		mObjects.Push(FJSONObject(&mDoc, randomaccess));
+		mObjects.Push(FJSONObject(&mDoc));
 		memset(mPlayers, 0, sizeof(mPlayers));
-		mObjectsRead = false;
 	}
-	
+
 	rapidjson::Value *FindKey(const char *key)
 	{
 		FJSONObject &obj = mObjects.Last();
 		
 		if (obj.mObject->IsObject())
 		{
-			if (!obj.mRandomAccess)
+			if (key == nullptr)
 			{
-				if (obj.mIterator != obj.mObject->MemberEnd())
+				// we are performing an iteration of the object through GetKey.
-				{
+				auto p = mKeyValue;
-					if (!strcmp(key, obj.mIterator->name.GetString()))
+				mKeyValue = nullptr;
-					{
+				return p;
-						return &(obj.mIterator++)->value;
-					}
-				}
 			}
 			else
 			{
-				// for unordered searches. This is slower but will not rely on sequential order of items.
+				// Find the given key by name;
 				auto it = obj.mObject->FindMember(key);
 				if (it == obj.mObject->MemberEnd()) return nullptr;
 				return &it->value;
@@ -252,8 +248,9 @@ struct FReader
 bool FSerializer::OpenWriter(bool pretty)
 {
 	if (w != nullptr || r != nullptr) return false;
-	w = new FWriter(pretty);
 
+	mErrors = 0;
+	w = new FWriter(pretty);
 	BeginObject(nullptr);
 	return true;
 }
@@ -267,7 +264,9 @@ bool FSerializer::OpenWriter(bool pretty)
 bool FSerializer::OpenReader(const char *buffer, size_t length)
 {
 	if (w != nullptr || r != nullptr) return false;
-	r = new FReader(buffer, length, true);
+
+	mErrors = 0;
+	r = new FReader(buffer, length);
 	return true;
 }
 
@@ -282,15 +281,17 @@ bool FSerializer::OpenReader(FCompressedBuffer *input)
 	if (input->mSize <= 0 || input->mBuffer == nullptr) return false;
 	if (w != nullptr || r != nullptr) return false;
 
+	mErrors = 0;
 	if (input->mMethod == METHOD_STORED)
 	{
-		r = new FReader((char*)input->mBuffer, input->mSize, true);
+		r = new FReader((char*)input->mBuffer, input->mSize);
 	}
 	else
 	{
 		char *unpacked = new char[input->mSize];
 		input->Decompress(unpacked);
-		r = new FReader(unpacked, input->mSize, true);
+		r = new FReader(unpacked, input->mSize);
+		delete[] unpacked;
 	}
 	return true;
 }
@@ -310,9 +311,27 @@ void FSerializer::Close()
 	}
 	if (r != nullptr)
 	{
+		// we must explicitly delete all thinkers in the array which did not get linked into the thinker lists.
+		// Otherwise these objects may survive a level deletion and point to incorrect data.
+		for (auto &obj : r->mDObjects)
+		{
+			auto think = dyn_cast<DThinker>(obj);
+			if (think != nullptr)
+			{
+				if (think->NextThinker == nullptr || think->PrevThinker == nullptr)
+				{
+					think->Destroy();
+				}
+			}
+		}
+
 		delete r;
 		r = nullptr;
 	}
+	if (mErrors > 0)
+	{
+		I_Error("%d errors parsing JSON", mErrors);
+	}
 }
 
 //==========================================================================
@@ -369,7 +388,7 @@ void FSerializer::WriteKey(const char *key)
 //
 //==========================================================================
 
-bool FSerializer::BeginObject(const char *name, bool randomaccess)
+bool FSerializer::BeginObject(const char *name)
 {
 	if (isWriting())
 	{
@@ -382,13 +401,16 @@ bool FSerializer::BeginObject(const char *name, bool randomaccess)
 		auto val = r->FindKey(name);
 		if (val != nullptr)
 		{
+			assert(val->IsObject());
 			if (val->IsObject())
 			{
-				r->mObjects.Push(FJSONObject(val, randomaccess));
+				r->mObjects.Push(FJSONObject(val));
 			}
 			else
 			{
-				I_Error("Object expected for '%s'", name);
+				Printf(TEXTCOLOR_RED "Object expected for '%s'", name);
+				mErrors++;
+				return false;
 			}
 		}
 		else
@@ -405,7 +427,7 @@ bool FSerializer::BeginObject(const char *name, bool randomaccess)
 //
 //==========================================================================
 
-void FSerializer::EndObject(bool endwarning)
+void FSerializer::EndObject()
 {
 	if (isWriting())
 	{
@@ -422,14 +444,6 @@ void FSerializer::EndObject(bool endwarning)
 	}
 	else
 	{
-		if (endwarning && !r->mObjects.Last().mRandomAccess)
-		{
-			if (r->mObjects.Last().mIterator != r->mObjects.Last().mObject->MemberEnd())
-			{
-				assert(false && "Incomplete read of sequential object");
-				I_Error("Incomplete read of sequential object");
-			}
-		}
 		r->mObjects.Pop();
 	}
 }
@@ -453,13 +467,16 @@ bool FSerializer::BeginArray(const char *name)
 		auto val = r->FindKey(name);
 		if (val != nullptr)
 		{
+			assert(val->IsArray());
 			if (val->IsArray())
 			{
 				r->mObjects.Push(FJSONObject(val));
 			}
 			else
 			{
-				I_Error("Array expected for '%s'", name);
+				Printf(TEXTCOLOR_RED "Array expected for '%s'", name);
+				mErrors++;
+				return false;
 			}
 		}
 		else
@@ -487,6 +504,7 @@ void FSerializer::EndArray()
 		}
 		else
 		{
+			assert(false && "EndArray call not inside an array");
 			I_Error("EndArray call not inside an array");
 		}
 	}
@@ -496,22 +514,6 @@ void FSerializer::EndArray()
 	}
 }
 
-//==========================================================================
-//
-// Discards an entry (only needed for sequential access)
-//
-//==========================================================================
-
-FSerializer &FSerializer::Discard(const char *key)
-{
-	if (isReading())
-	{
-		// just get the key and advance the iterator, if present
-		if (!r->mObjects.Last().mRandomAccess) r->FindKey(key);
-	}
-	return *this;
-}
-
 //==========================================================================
 //
 // Special handler for args (because ACS specials' arg0 needs special treatment.)
@@ -563,13 +565,17 @@ FSerializer &FSerializer::Args(const char *key, int *args, int *defargs, int spe
 					}
 					else
 					{
-						I_Error("Integer expected for '%s[%d]'", key, i);
+						assert(false && "Integer expected");
+						Printf(TEXTCOLOR_RED "Integer expected for '%s[%d]'", key, i);
+						mErrors++;
 					}
 				}
 			}
 			else
 			{
-				I_Error("array expected for '%s'", key);
+				assert(false && "array expected");
+				Printf(TEXTCOLOR_RED "array expected for '%s'", key);
+				mErrors++;
 			}
 		}
 	}
@@ -611,7 +617,9 @@ FSerializer &FSerializer::ScriptNum(const char *key, int &num)
 			}
 			else
 			{
-				I_Error("Integer expected for '%s'", key);
+				assert(false && "Integer expected");
+				Printf(TEXTCOLOR_RED "Integer expected for '%s'", key);
+				mErrors++;
 			}
 		}
 	}
@@ -739,7 +747,8 @@ unsigned FSerializer::GetSize(const char *group)
 
 //==========================================================================
 //
-//
+// gets the key pointed to by the iterator, caches its value
+// and returns the key string.
 //
 //==========================================================================
 
@@ -749,7 +758,8 @@ const char *FSerializer::GetKey()
 	if (!r->mObjects.Last().mObject->IsObject()) return nullptr;	// non-objects do not have keys.
 	auto &it = r->mObjects.Last().mIterator;
 	if (it == r->mObjects.Last().mObject->MemberEnd()) return nullptr;
-	return it->name.GetString();
+	r->mKeyValue = &it->value;
+	return (it++)->name.GetString();
 }
 
 //==========================================================================
@@ -773,6 +783,9 @@ void FSerializer::WriteObjects()
 			w->Key("classtype");
 			w->String(obj->GetClass()->TypeName.GetChars());
 
+			obj->SerializeUserVars(*this);
+			obj->Serialize(*this);
+			obj->CheckIfSerialized();
 			if (obj->IsKindOf(RUNTIME_CLASS(AActor)) &&
 				(player = static_cast<AActor *>(obj)->player) &&
 				player->mo == obj)
@@ -781,9 +794,6 @@ void FSerializer::WriteObjects()
 				w->Int(int(player - players));
 			}
 
-			obj->SerializeUserVars(*this);
-			obj->Serialize(*this);
-			obj->CheckIfSerialized();
 			EndObject();
 		}
 		EndArray();
@@ -799,8 +809,7 @@ void FSerializer::WriteObjects()
 void FSerializer::ReadObjects()
 {
 	bool founderrors = false;
-	unsigned i;
+	
-
 	if (isReading() && BeginArray("objects"))
 	{
 		// Do not link any thinker that's being created here. This will be done by deserializing the thinker list later.
@@ -845,7 +854,6 @@ void FSerializer::ReadObjects()
 						if (BeginObject(nullptr))
 						{
 							int pindex = -1;
-							Discard("classtype");
 							Serialize(*this, "playerindex", pindex, nullptr);
 							if (obj != nullptr)
 							{
@@ -857,20 +865,23 @@ void FSerializer::ReadObjects()
 								obj->SerializeUserVars(*this);
 								obj->Serialize(*this);
 							}
-							EndObject(true);
+							EndObject();
 						}
 					}
 					catch (CRecoverableError &err)
 					{
-						I_Error("%s\n while restoring %s", err.GetMessage(),obj? obj->GetClass()->TypeName.GetChars() : "invalid object");
+						Printf(TEXTCOLOR_RED "'%s'\n while restoring %s", err.GetMessage(),obj? obj->GetClass()->TypeName.GetChars() : "invalid object");
+						mErrors++;
 					}
 				}
 			}
 			EndArray();
 			DThinker::bSerialOverride = false;
+			assert(!founderrors);
 			if (founderrors)
 			{
-				I_Error("Failed to restore all objects in savegame");
+				Printf(TEXTCOLOR_RED "Failed to restore all objects in savegame");
+				mErrors++;
 			}
 		}
 		catch(...)
@@ -989,13 +1000,15 @@ FSerializer &Serialize(FSerializer &arc, const char *key, bool &value, bool *def
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsBool());
 			if (val->IsBool())
 			{
 				value = val->GetBool();
 			}
 			else
 			{
-				I_Error("boolean type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "boolean type expected for '%s'", key);
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1023,13 +1036,15 @@ FSerializer &Serialize(FSerializer &arc, const char *key, int64_t &value, int64_
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsInt64());
 			if (val->IsInt64())
 			{
 				value = val->GetInt64();
 			}
 			else
 			{
-				I_Error("integer type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "integer type expected for '%s'", key);
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1057,13 +1072,15 @@ FSerializer &Serialize(FSerializer &arc, const char *key, uint64_t &value, uint6
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsUint64());
 			if (val->IsUint64())
 			{
 				value = val->GetUint64();
 			}
 			else
 			{
-				I_Error("integer type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "integer type expected for '%s'", key);
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1092,13 +1109,15 @@ FSerializer &Serialize(FSerializer &arc, const char *key, int32_t &value, int32_
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsInt());
 			if (val->IsInt())
 			{
 				value = val->GetInt();
 			}
 			else
 			{
-				I_Error("integer type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "integer type expected for '%s'", key);
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1126,13 +1145,15 @@ FSerializer &Serialize(FSerializer &arc, const char *key, uint32_t &value, uint3
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsUint());
 			if (val->IsUint())
 			{
 				value = val->GetUint();
 			}
 			else
 			{
-				I_Error("integer type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "integer type expected for '%s'", key);
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1202,13 +1223,15 @@ FSerializer &Serialize(FSerializer &arc, const char *key, double &value, double
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsDouble());
 			if (val->IsDouble())
 			{
 				value = val->GetDouble();
 			}
 			else
 			{
-				I_Error("float type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "float type expected for '%s'", key);
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1239,6 +1262,7 @@ FSerializer &Serialize(FSerializer &arc, const char *key, float &value, float *d
 template<class T>
 FSerializer &SerializePointer(FSerializer &arc, const char *key, T *&value, T **defval, T *base)
 {
+	assert(base != nullptr);
 	if (arc.isReading() || !arc.w->inObject() || defval == nullptr || value != *defval)
 	{
 		ptrdiff_t vv = value == nullptr ? -1 : value - base;
@@ -1342,13 +1366,16 @@ FSerializer &Serialize(FSerializer &arc, const char *key, FTextureID &value, FTe
 			{
 				const rapidjson::Value &nameval = (*val)[0];
 				const rapidjson::Value &typeval = (*val)[1];
+				assert(nameval.IsString() && typeval.IsInt());
 				if (nameval.IsString() && typeval.IsInt())
 				{
 					value = TexMan.GetTexture(nameval.GetString(), typeval.GetInt());
 				}
 				else
 				{
-					I_Error("object does not represent a texture for '%s'", key);
+					Printf(TEXTCOLOR_RED "object does not represent a texture for '%s'", key);
+					value.SetNull();
+					arc.mErrors++;
 				}
 			}
 			else if (val->IsNull())
@@ -1361,7 +1388,10 @@ FSerializer &Serialize(FSerializer &arc, const char *key, FTextureID &value, FTe
 			}
 			else
 			{
-				I_Error("object does not represent a texture for '%s'", key);
+				assert(false && "not a texture");
+				Printf(TEXTCOLOR_RED "object does not represent a texture for '%s'", key);
+				value.SetNull();
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1432,7 +1462,10 @@ FSerializer &Serialize(FSerializer &arc, const char *key, DObject *&value, DObje
 				}
 				else
 				{
-					I_Error("Invalid object reference for '%s'", key);
+					assert(false && "invalid object reference");
+					Printf(TEXTCOLOR_RED "Invalid object reference for '%s'", key);
+					value = nullptr;
+					arc.mErrors++;
 				}
 			}
 		}
@@ -1469,13 +1502,16 @@ FSerializer &Serialize(FSerializer &arc, const char *key, FName &value, FName *d
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsString());
 			if (val->IsString())
 			{
 				value = val->GetString();
 			}
 			else
 			{
-				I_Error("String expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "String expected for '%s'", key);
+				arc.mErrors++;
+				value = NAME_None;
 			}
 		}
 	}
@@ -1509,7 +1545,7 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, FDynamicCol
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
-			if (val->IsObject())
+			if (val->IsArray())
 			{
 				const rapidjson::Value &colorval = (*val)[0];
 				const rapidjson::Value &fadeval = (*val)[1];
@@ -1517,16 +1553,12 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, FDynamicCol
 				if (colorval.IsUint() && fadeval.IsUint() && desatval.IsUint())
 				{
 					cm = GetSpecialLights(colorval.GetUint(), fadeval.GetUint(), desatval.GetUint());
-				}
+					return arc;
-				else
-				{
-					I_Error("object does not represent a colormap for '%s'", key);
 				}
 			}
-			else
+			assert(false && "not a colormap");
-			{
+			Printf(TEXTCOLOR_RED "object does not represent a colormap for '%s'", key);
-				I_Error("object does not represent a colormap for '%s'", key);
+			cm = &NormalLight;
-			}
 		}
 	}
 	return arc;
@@ -1555,6 +1587,7 @@ FSerializer &Serialize(FSerializer &arc, const char *key, FSoundID &sid, FSoundI
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsString() || val->IsNull());
 			if (val->IsString())
 			{
 				sid = val->GetString();
@@ -1565,7 +1598,9 @@ FSerializer &Serialize(FSerializer &arc, const char *key, FSoundID &sid, FSoundI
 			}
 			else
 			{
-				I_Error("string type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "string type expected for '%s'", key);
+				sid = 0;
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1601,6 +1636,7 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, PClassActor
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsString() || val->IsNull());
 			if (val->IsString())
 			{
 				clst = PClass::FindActor(val->GetString());
@@ -1611,7 +1647,9 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, PClassActor
 			}
 			else
 			{
-				I_Error("string type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "string type expected for '%s'", key);
+				clst = nullptr;
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1657,7 +1695,9 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, PClass *&cl
 			}
 			else
 			{
-				I_Error("string type expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "string type expected for '%s'", key);
+				clst = nullptr;
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1719,18 +1759,33 @@ FSerializer &Serialize(FSerializer &arc, const char *key, FState *&state, FState
 				const rapidjson::Value &ndx = (*val)[1];
 
 				state = nullptr;
-				if (cls.IsString() && ndx.IsInt())
+				assert(cls.IsString() && ndx.IsUint());
+				if (cls.IsString() && ndx.IsUint())
 				{
 					PClassActor *clas = PClass::FindActor(cls.GetString());
-					if (clas)
+					if (clas && ndx.GetUint() < (unsigned)clas->NumOwnedStates)
 					{
-						state = clas->OwnedStates + ndx.GetInt();
+						state = clas->OwnedStates + ndx.GetUint();
 					}
+					else
+					{
+						// this can actually happen by changing the DECORATE so treat it as a warning, not an error.
+						state = nullptr;
+						Printf(TEXTCOLOR_ORANGE "Invalid state '%s+%d' for '%s'", cls.GetString(), ndx.GetInt(), key);
+					}
+				}
+				else
+				{
+					assert(false && "not a state");
+					Printf(TEXTCOLOR_RED "data does not represent a state for '%s'", key);
+					arc.mErrors++;
 				}
 			}
 			else if (!retcode)
 			{
-				I_Error("array type expected for '%s'", key);
+				assert(false && "not an array");
+				Printf(TEXTCOLOR_RED "array type expected for '%s'", key);
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1766,6 +1821,7 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, FStrifeDial
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsUint() || val->IsNull());
 			if (val->IsNull())
 			{
 				node = nullptr;
@@ -1774,7 +1830,7 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, FStrifeDial
 			{
 				if (val->GetUint() >= StrifeDialogues.Size())
 				{
-					node = NULL;
+					node = nullptr;
 				}
 				else
 				{
@@ -1783,7 +1839,9 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, FStrifeDial
 			}
 			else
 			{
-				I_Error("integer expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "integer expected for '%s'", key);
+				arc.mErrors++;
+				node = nullptr;
 			}
 		}
 	}
@@ -1819,6 +1877,7 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, FString *&p
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsNull() || val->IsString());
 			if (val->IsNull())
 			{
 				pstr = nullptr;
@@ -1829,7 +1888,9 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, FString *&p
 			}
 			else
 			{
-				I_Error("string expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "string expected for '%s'", key);
+				pstr = nullptr;
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1858,6 +1919,7 @@ FSerializer &Serialize(FSerializer &arc, const char *key, FString &pstr, FString
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsNull() || val->IsString());
 			if (val->IsNull())
 			{
 				pstr = "";
@@ -1868,7 +1930,9 @@ FSerializer &Serialize(FSerializer &arc, const char *key, FString &pstr, FString
 			}
 			else
 			{
-				I_Error("string expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "string expected for '%s'", key);
+				pstr = "";
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1904,6 +1968,7 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, char *&pstr
 		auto val = arc.r->FindKey(key);
 		if (val != nullptr)
 		{
+			assert(val->IsNull() || val->IsString());
 			if (val->IsNull())
 			{
 				pstr = nullptr;
@@ -1914,7 +1979,9 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, char *&pstr
 			}
 			else
 			{
-				I_Error("string expected for '%s'", key);
+				Printf(TEXTCOLOR_RED "string expected for '%s'", key);
+				pstr = nullptr;
+				arc.mErrors++;
 			}
 		}
 	}
@@ -1941,7 +2008,7 @@ template<> FSerializer &Serialize(FSerializer &arc, const char *key, FFont *&fon
 		font = V_GetFont(n);
 		if (font == nullptr)
 		{
-			Printf("Could not load font %s\n", n);
+			Printf(TEXTCOLOR_ORANGE "Could not load font %s\n", n);
 			font = SmallFont;
 		}
 		return arc;

src/serializer.h

@@ -73,15 +73,14 @@ public:
 	bool OpenReader(FCompressedBuffer *input);
 	void Close();
 	void ReadObjects();
-	bool BeginObject(const char *name, bool randomaccess = false);
+	bool BeginObject(const char *name);
-	void EndObject(bool endwarning = false);
+	void EndObject();
 	bool BeginArray(const char *name);
 	void EndArray();
 	unsigned GetSize(const char *group);
 	const char *GetKey();
 	const char *GetOutput(unsigned *len = nullptr);
 	FCompressedBuffer GetCompressedOutput();
-	FSerializer &Discard(const char *key);
 	FSerializer &Args(const char *key, int *args, int *defargs, int special);
 	FSerializer &Terrain(const char *key, int &terrain, int *def = nullptr);
 	FSerializer &Sprite(const char *key, int32_t &spritenum, int32_t *def);
@@ -167,6 +166,8 @@ public:
 		obj = (T)val;
 		return *this;
 	}
+
+	int mErrors = 0;
 };
 
 FSerializer &Serialize(FSerializer &arc, const char *key, bool &value, bool *defval);