- 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

@@ -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<PClassActor *>(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<PClassActor *>(RUNTIME_CLASS(ADehackedPickup)->
+				CreateDerivedClass(typeNameBuilder, sizeof(ADehackedPickup)));
+		} 
+		while (subclass == nullptr);
+		
 		AActor *defaults2 = GetDefaultByType (subclass);
 		memcpy ((void *)defaults2, (void *)defaults1, sizeof(AActor));
 

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<PClass *>(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<PClass*>(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<PClass*>(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
 	{

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<PClassActor *>(parent->CreateDerivedClass (typeName, parent->Size));
+	type = DecoDerivedClass(FScriptPosition(sc), parent, typeName);
 	ResetBaggage(&bag, parent);
 	bag.Info = type;
 #ifdef _DEBUG

src/thingdef/thingdef.cpp

@@ -68,13 +68,44 @@
 
 TDeletingArray<class FxExpression *> 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<PClassActor *>(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<PClassActor *>(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<PClassActor *>(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();