From d819aafcf309dcce2caff2215a7c8b108f9abc27 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Fri, 7 Oct 2016 15:28:25 +0200 Subject: [PATCH] - changed handling of duplicate classes in DECORATE. Instead of replacing the original, the second class will get renamed now, using the originating file as an identifier. In the vast majority of cases this should do exactly what is needed: Create an unconflicting second class that can coexist with the original. Unless the class is used by name this should eliminate all problems with this, but so far I haven't seen anything that used them by name. This is choosing the lesser of two evils. While some mod out there may get broken, the old setup meant that the first class of a given name could not be written out to a savegame because it was not retrievable when loading it back. --- src/d_dehacked.cpp | 15 ++++++++--- src/dobjtype.cpp | 25 ++++++++++-------- src/thingdef/olddecorations.cpp | 3 ++- src/thingdef/thingdef.cpp | 45 +++++++++++++++++++++++++++++++-- 4 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/d_dehacked.cpp b/src/d_dehacked.cpp index 9e068b596..dd71fdf75 100644 --- a/src/d_dehacked.cpp +++ b/src/d_dehacked.cpp @@ -2985,9 +2985,11 @@ static bool LoadDehSupp () void FinishDehPatch () { unsigned int touchedIndex; + unsigned int nameindex = 0; for (touchedIndex = 0; touchedIndex < TouchedActors.Size(); ++touchedIndex) { + PClassActor *subclass; PClassActor *type = TouchedActors[touchedIndex]; AActor *defaults1 = GetDefaultByType (type); if (!(defaults1->flags & MF_SPECIAL)) @@ -2997,9 +2999,16 @@ void FinishDehPatch () // Create a new class that will serve as the actual pickup char typeNameBuilder[32]; - mysnprintf (typeNameBuilder, countof(typeNameBuilder), "DehackedPickup%d", touchedIndex); - PClassActor *subclass = static_cast(RUNTIME_CLASS(ADehackedPickup)-> - CreateDerivedClass(typeNameBuilder, sizeof(ADehackedPickup))); + // + do + { + // Retry until we find a free name. This is unlikely to happen but not impossible. + mysnprintf(typeNameBuilder, countof(typeNameBuilder), "DehackedPickup%d", nameindex++); + subclass = static_cast(RUNTIME_CLASS(ADehackedPickup)-> + CreateDerivedClass(typeNameBuilder, sizeof(ADehackedPickup))); + } + while (subclass == nullptr); + AActor *defaults2 = GetDefaultByType (subclass); memcpy ((void *)defaults2, (void *)defaults1, sizeof(AActor)); diff --git a/src/dobjtype.cpp b/src/dobjtype.cpp index c3d0702f3..37ff26fe3 100644 --- a/src/dobjtype.cpp +++ b/src/dobjtype.cpp @@ -2861,10 +2861,7 @@ void PClass::InsertIntoHash () found = TypeTable.FindType(RUNTIME_CLASS(PClass), (intptr_t)Outer, TypeName, &bucket); if (found != NULL) { // This type has already been inserted - // ... but there is no need whatsoever to make it a fatal error! - if (!strictdecorate) Printf (TEXTCOLOR_RED"Tried to register class '%s' more than once.\n", TypeName.GetChars()); - else I_Error("Tried to register class '%s' more than once.\n", TypeName.GetChars()); - TypeTable.ReplaceType(this, found, bucket); + I_Error("Tried to register class '%s' more than once.\n", TypeName.GetChars()); } else { @@ -3029,15 +3026,23 @@ PClass *PClass::CreateDerivedClass(FName name, unsigned int size) PClass *existclass = static_cast(TypeTable.FindType(RUNTIME_CLASS(PClass), /*FIXME:Outer*/0, name, &bucket)); // This is a placeholder so fill it in - if (existclass != NULL && (existclass->Size == TentativeClass)) + if (existclass != nullptr) { - type = const_cast(existclass); - if (!IsDescendantOf(type->ParentClass)) + if (existclass->Size == TentativeClass) { - I_Error("%s must inherit from %s but doesn't.", name.GetChars(), type->ParentClass->TypeName.GetChars()); + type = const_cast(existclass); + if (!IsDescendantOf(type->ParentClass)) + { + I_Error("%s must inherit from %s but doesn't.", name.GetChars(), type->ParentClass->TypeName.GetChars()); + } + DPrintf(DMSG_SPAMMY, "Defining placeholder class %s\n", name.GetChars()); + notnew = true; + } + else + { + // a different class with the same name already exists. Let the calling code deal with this. + return nullptr; } - DPrintf(DMSG_SPAMMY, "Defining placeholder class %s\n", name.GetChars()); - notnew = true; } else { diff --git a/src/thingdef/olddecorations.cpp b/src/thingdef/olddecorations.cpp index 3ea0c4788..9afbfc965 100644 --- a/src/thingdef/olddecorations.cpp +++ b/src/thingdef/olddecorations.cpp @@ -140,6 +140,7 @@ DEFINE_CLASS_PROPERTY(respawns, 0, FakeInventory) // Reads an old style decoration object // //========================================================================== +PClassActor *DecoDerivedClass(const FScriptPosition &sc, PClassActor *parent, FName typeName); void ParseOldDecoration(FScanner &sc, EDefinitionType def) { @@ -154,7 +155,7 @@ void ParseOldDecoration(FScanner &sc, EDefinitionType def) sc.MustGetString(); typeName = FName(sc.String); - type = static_cast(parent->CreateDerivedClass (typeName, parent->Size)); + type = DecoDerivedClass(FScriptPosition(sc), parent, typeName); ResetBaggage(&bag, parent); bag.Info = type; #ifdef _DEBUG diff --git a/src/thingdef/thingdef.cpp b/src/thingdef/thingdef.cpp index 32d4f1ce0..41e1e9542 100644 --- a/src/thingdef/thingdef.cpp +++ b/src/thingdef/thingdef.cpp @@ -68,13 +68,44 @@ TDeletingArray ActorDamageFuncs; + // EXTERNAL FUNCTION PROTOTYPES -------------------------------------------- void InitThingdef(); -void ParseDecorate (FScanner &sc); +void ParseDecorate(FScanner &ctx); // STATIC FUNCTION PROTOTYPES -------------------------------------------- PClassActor *QuestItemClasses[31]; +EXTERN_CVAR(Bool, strictdecorate); + +PClassActor *DecoDerivedClass(const FScriptPosition &sc, PClassActor *parent, FName typeName) +{ + PClassActor *type = static_cast(parent->CreateDerivedClass(typeName, parent->Size)); + if (type == nullptr) + { + FString newname = typeName.GetChars(); + FString sourcefile = sc.FileName; + + sourcefile.Substitute(":", "@"); + newname << '@' << sourcefile; + if (strictdecorate) + { + sc.Message(MSG_ERROR, "Tried to define class '%s' more than once.", typeName.GetChars()); + } + else + { + // Due to backwards compatibility issues this cannot be an unconditional error. + sc.Message(MSG_WARNING, "Tried to define class '%s' more than once. Renaming class to '%s'", typeName.GetChars(), newname.GetChars()); + } + type = static_cast(parent->CreateDerivedClass(newname, parent->Size)); + if (type == nullptr) + { + // This we cannot handle cleanly anymore. Let's just abort and forget about the odd mod out that was this careless. + sc.Message(MSG_FATAL, "Tried to define class '%s' more than twice in the same file.", typeName.GetChars()); + } + } + return type; +} //========================================================================== // // Starts a new actor definition @@ -141,7 +172,7 @@ PClassActor *CreateNewActor(const FScriptPosition &sc, FName typeName, FName par else { create: - ti = static_cast(parent->CreateDerivedClass (typeName, parent->Size)); + ti = DecoDerivedClass(sc, parent, typeName); } ti->Replacee = ti->Replacement = NULL; @@ -428,6 +459,16 @@ void LoadActors () while ((lump = Wads.FindLump ("DECORATE", &lastlump)) != -1) { FScanner sc(lump); + + if (Wads.GetLumpFile(lump) == 0) + { + // define namespace 'zdoom' + } + else + { + // use namespace 'zdoom' + } + ParseDecorate (sc); } FinishThingdef();