From 86e9282193fce0df638a34e5f9fcf81b4ea0ad71 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Fri, 23 Sep 2016 14:04:05 +0200 Subject: [PATCH] - 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 | 2 +- src/dobjtype.cpp | 18 +- src/dthinker.cpp | 8 +- src/dthinker.h | 1 + src/g_game.cpp | 281 ++++++++++++++++---------------- src/g_level.cpp | 2 +- src/g_shared/a_sectoraction.cpp | 1 + src/p_acs.cpp | 47 +++--- src/p_saveg.cpp | 10 +- src/serializer.cpp | 259 ++++++++++++++++++----------- src/serializer.h | 7 +- 11 files changed, 364 insertions(+), 272 deletions(-) diff --git a/src/d_netinfo.cpp b/src/d_netinfo.cpp index 0bd3ded1ff..3827df491f 100644 --- a/src/d_netinfo.cpp +++ b/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) diff --git a/src/dobjtype.cpp b/src/dobjtype.cpp index 5629fa4aa1..9693da1375 100644 --- a/src/dobjtype.cpp +++ b/src/dobjtype.cpp @@ -2250,7 +2250,7 @@ bool PStruct::ReadFields(FSerializer &ar, void *addr) const } else { - readsomething |= static_cast(sym)->Type->ReadValue(ar, label, + readsomething |= static_cast(sym)->Type->ReadValue(ar, nullptr, (BYTE *)addr + static_cast(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 { diff --git a/src/dthinker.cpp b/src/dthinker.cpp index 3cb40cb8f7..0cd2c6a685 100644 --- a/src/dthinker.cpp +++ b/src/dthinker.cpp @@ -394,8 +394,12 @@ void DThinker::Tick () size_t DThinker::PropagateMark() { - assert(NextThinker != NULL && !(NextThinker->ObjectFlags & OF_EuthanizeMe)); - assert(PrevThinker != NULL && !(PrevThinker->ObjectFlags & OF_EuthanizeMe)); + // Do not choke on partially initialized objects (as happens when loading a savegame fails) + 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(); diff --git a/src/dthinker.h b/src/dthinker.h index 3f3a37b783..9ce8943b41 100644 --- a/src/dthinker.h +++ b/src/dthinker.h @@ -105,6 +105,7 @@ private: friend struct FThinkerList; friend class FThinkerIterator; friend class DObject; + friend class FSerializer; DThinker *NextThinker, *PrevThinker; }; diff --git a/src/g_game.cpp b/src/g_game.cpp index 3147f694dd..5d1f83b828 100644 --- a/src/g_game.cpp +++ b/src/g_game.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #ifdef __APPLE__ #include #endif @@ -1844,154 +1845,162 @@ void G_DoLoadGame () Printf ("Could not read savegame '%s'\n", savename.GetChars()); return; } - - FResourceLump *info = resfile->FindLump("info.json"); - if (info == nullptr) + try { - delete resfile; - Printf ("'%s' is not a valid savegame: Missing 'info.json'.\n", savename.GetChars()); - 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()) + FResourceLump *info = resfile->FindLump("info.json"); + if (info == nullptr) { - 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) - { - delete resfile; - Printf ("Savegame is from an incompatible version"); - if (SaveVersion < MINSAVEVER) + // 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) { - 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(); } diff --git a/src/g_level.cpp b/src/g_level.cpp index de27248497..dcf97ba4fe 100644 --- a/src/g_level.cpp +++ b/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(); } diff --git a/src/g_shared/a_sectoraction.cpp b/src/g_shared/a_sectoraction.cpp index da3830d74b..38435e16be 100644 --- a/src/g_shared/a_sectoraction.cpp +++ b/src/g_shared/a_sectoraction.cpp @@ -69,6 +69,7 @@ void ASectorAction::Destroy () { *prev.act = probe->tracer; } + Sector = nullptr; } Super::Destroy (); diff --git a/src/p_acs.cpp b/src/p_acs.cpp index e06109678f..3bd7218b3b 100644 --- a/src/p_acs.cpp +++ b/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]; - const char *modname = module->ModuleName; - int ModSize = module->GetDataSize(); + arc.StringPtr("modname", modname) + ("modsize", ModSize); - if (arc.BeginObject(nullptr)) + if (arc.isReading()) { - arc.StringPtr("modname", modname) - ("modsize", ModSize); - - if (arc.isReading()) + if (stricmp(modname, module->ModuleName) != 0) { - 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); - } - 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("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()) + { + 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(); } } diff --git a/src/p_saveg.cpp b/src/p_saveg.cpp index 72717f2d6d..e6c521b1bd 100644 --- a/src/p_saveg.cpp +++ b/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++) diff --git a/src/serializer.cpp b/src/serializer.cpp index bd09d90e9e..4d9da256b1 100644 --- a/src/serializer.cpp +++ b/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 mObjects; rapidjson::Document mDoc; TArray 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()) - { - if (!strcmp(key, obj.mIterator->name.GetString())) - { - return &(obj.mIterator++)->value; - } - } + // we are performing an iteration of the object through GetKey. + auto p = mKeyValue; + mKeyValue = nullptr; + return p; } 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(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(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 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()); - } - else - { - I_Error("object does not represent a colormap for '%s'", key); + return arc; } } - else - { - I_Error("object does not represent a colormap for '%s'", key); - } + assert(false && "not a colormap"); + Printf(TEXTCOLOR_RED "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; diff --git a/src/serializer.h b/src/serializer.h index 7b35294d23..4730136a2b 100644 --- a/src/serializer.h +++ b/src/serializer.h @@ -73,15 +73,14 @@ public: bool OpenReader(FCompressedBuffer *input); void Close(); void ReadObjects(); - bool BeginObject(const char *name, bool randomaccess = false); - void EndObject(bool endwarning = false); + bool BeginObject(const char *name); + 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);