Merge pull request #235 from lilligreen/namespaces

Fix for namespace linking and unlinking issues

engine/source/console/consoleNamespace.cc

@@ -148,12 +148,14 @@ bool Namespace::canTabComplete(const char *prevText, const char *bestMatch, cons
    }
 }
 
-bool Namespace::unlinkClass(Namespace *parent)
+bool Namespace::unlinkClass(Namespace* parent)
 {
-   Namespace *walk = this;
+   Namespace* walk = this;
+
    while(walk->mParent && walk->mParent->mName == mName)
       walk = walk->mParent;
 
+   // Make sure "parent" is the direct parent namespace.
    if(walk->mParent && walk->mParent != parent)
    {
       Con::errorf(ConsoleLogEntry::General, "Namespace::unlinkClass - cannot unlink namespace parent linkage for %s for %s.",
@@ -161,30 +163,37 @@ bool Namespace::unlinkClass(Namespace *parent)
       return false;
    }
 
-   AssertFatal(mRefCountToParent > 0, "Namespace::unlinkClass - reference count to parent is less than 0");
+   // Decrease the reference count. Note that we do this on the bottom-most namespace.
+   AssertWarn(mRefCountToParent > 0, "Namespace::unlinkClass - reference count to parent is already at 0");
    mRefCountToParent--;
 
+   // Unlink if the count dropped to zero.
    if(mRefCountToParent == 0)
+   {
       walk->mParent = NULL;
-
-   trashCache();
+      trashCache();
+   }
 
    return true;
 }
 
 
-bool Namespace::classLinkTo(Namespace *parent)
+bool Namespace::classLinkTo(Namespace* parent)
 {
-   Namespace *walk = this;
+   Namespace* walk = this;
+
    while(walk->mParent && walk->mParent->mName == mName)
       walk = walk->mParent;
 
+   // Make sure there is no existing parent namespace.
    if(walk->mParent && walk->mParent != parent)
    {
-      Con::errorf(ConsoleLogEntry::General, "Error: cannot change namespace parent linkage of %s from %s to %s.",
+      Con::errorf(ConsoleLogEntry::General, "Namespace::classLinkTo - cannot change namespace parent linkage of %s from %s to %s.",
          walk->mName, walk->mParent->mName, parent->mName);
       return false;
    }
+
+   // Increase the reference count and add the parent namespace.
    mRefCountToParent++;
    walk->mParent = parent;
 

engine/source/sim/simObject.cc

@@ -46,7 +46,7 @@ namespace Sim
 
 //-----------------------------------------------------------------------------
 
-SimObject::SimObject( const U8 namespaceLinkMask ) : mNSLinkMask( namespaceLinkMask )
+SimObject::SimObject()
 {
     mFlags.set( ModStaticFields | ModDynamicFields );
     objectName               = NULL;
@@ -62,7 +62,7 @@ SimObject::SimObject( const U8 namespaceLinkMask ) : mNSLinkMask( namespaceLinkM
     mTypeMask                = 0;
     mScriptCallbackGuard     = 0;
     mFieldDictionary         = NULL;
-    mCanSaveFieldDictionary	 = true;
+    mCanSaveFieldDictionary  = true;
     mClassName               = NULL;
     mSuperClassName          = NULL;
     mProgenitorFile          = CodeBlock::getCurrentCodeBlockFullPath();
@@ -103,7 +103,7 @@ bool SimObject::registerObject()
    AssertFatal(Sim::gIdDictionary && Sim::gNameDictionary, 
       "SimObject::registerObject - tried to register an object before Sim::init()!");
 
-   Sim::gIdDictionary->insert(this);	
+   Sim::gIdDictionary->insert(this);  
 
    Sim::gNameDictionary->insert(this);
 
@@ -200,40 +200,46 @@ void SimObject::setId(SimObjectId newId)
 
 void SimObject::assignName(const char *name)
 {
-   // Added this assert 3/30/2007 because it is dumb to try to name
-   // a SimObject the same thing as it's class name -patw
-   //AssertFatal( dStricmp( getClassName(), name ), "Attempted to assign a name to a SimObject which matches it's type name." );
-   if( dStricmp( getClassName(), name ) == 0 )
-      Con::errorf( "SimObject::assignName - Assigning name '%s' to instance of object with type '%s'."
-      " This can cause namespace linking issues.", getClassName(), name  );
+    if( dStricmp( getClassName(), name ) == 0 )
+        Con::errorf( "SimObject::assignName - Assigning name '%s' to instance of object with type '%s'."
+        " This can cause namespace linking issues.", getClassName(), name  );
 
-#if 0
-    Con::printf( "SimObject::assignName(%s)", name );
-#endif
+    // Is this name already registered?
+    if ( Sim::gNameDictionary->find(name) != NULL )
+    {
+        // Yes, so error,
+        Con::errorf( "SimObject::assignName() - Attempted to set object to name '%s' but it is already assigned to another object.", name );
+        return;
+    }
 
-   // Is this name already registered?
-   if ( Sim::gNameDictionary->find(name) != NULL )
-   {
-       // Yes, so error,
-       Con::errorf( "SimObject::assignName() - Attempted to set object to name '%s' but it is already assigned to another object.", name );
-       return;
-   }
+    StringTableEntry newName = NULL;
+
+    if (name[0])
+        newName = StringTable->insert(name);
+
+    if (mGroup)
+        mGroup->nameDictionary.remove(this);
+
+    if (isProperlyAdded())
+    {
+        // Unlink the old namespaces.
+        unlinkNamespaces();
+
+        Sim::gNameDictionary->remove(this);
+    }
 
-   StringTableEntry newName = NULL;
-   if(name[0])
-      newName = StringTable->insert(name);
+    objectName = newName;
 
-   if(mGroup)
-      mGroup->nameDictionary.remove(this);
-   if(mFlags.test(Added))
-      Sim::gNameDictionary->remove(this);
+    if (mGroup)
+        mGroup->nameDictionary.insert(this);
 
-   objectName = newName;
+    if (isProperlyAdded())
+    {
+        // Link the new namespaces.
+        linkNamespaces();
 
-   if(mGroup)
-      mGroup->nameDictionary.insert(this);
-   if(mFlags.test(Added))
-      Sim::gNameDictionary->insert(this);
+        Sim::gNameDictionary->insert(this);
+    }  
 }
 
 //---------------------------------------------------------------------------
@@ -487,7 +493,7 @@ void  SimObject::dumpClassHierarchy()
    while(pRep)
    {
       Con::warnf("%s ->", pRep->getClassName());
-      pRep	=	pRep->getParentClass();
+      pRep  = pRep->getParentClass();
    }
 }
 
@@ -928,7 +934,7 @@ bool SimObject::isChildOfGroup(SimGroup* pGroup)
    if(pGroup == dynamic_cast<SimGroup*>(this))
       return true;
 
-   SimGroup* temp	=	mGroup;
+   SimGroup* temp = mGroup;
    while(temp)
    {
       if(temp == pGroup)
@@ -1079,7 +1085,7 @@ void SimObject::initPersistFields()
 {
    Parent::initPersistFields();
    addGroup("SimBase");
-   addField("canSaveDynamicFields",		TypeBool,		Offset(mCanSaveFieldDictionary, SimObject), &writeCanSaveDynamicFields, "");
+   addField("canSaveDynamicFields",   TypeBool,   Offset(mCanSaveFieldDictionary, SimObject), &writeCanSaveDynamicFields, "");
    addField("internalName",            TypeString,       Offset(mInternalName, SimObject), &writeInternalName, "");   
    addProtectedField("parentGroup",        TypeSimObjectPtr, Offset(mGroup, SimObject), &setParentGroup, &defaultProtectedGetFn, &writeParentGroup, "Group hierarchy parent of the object." );
    endGroup("SimBase");
@@ -1225,107 +1231,160 @@ void SimObject::inspectPostApply()
 {
 }
 
+//-----------------------------------------------------------------------------
+
 void SimObject::linkNamespaces()
 {
-   if( mNameSpace )
-      unlinkNamespaces();
-   
-   StringTableEntry parent = StringTable->insert( getClassName() );
-   if( ( mNSLinkMask & LinkSuperClassName ) && mSuperClassName && mSuperClassName[0] )
-   {
-      if( Con::linkNamespaces( parent, mSuperClassName ) )
-         parent = mSuperClassName;
-      else
-         mSuperClassName = StringTable->EmptyString; // CodeReview Is this behavior that we want?
-                                                      // CodeReview This will result in the mSuperClassName variable getting hosed
-                                                      // CodeReview if Con::linkNamespaces returns false. Looking at the code for
-                                                      // CodeReview Con::linkNamespaces, and the call it makes to classLinkTo, it seems
-                                                      // CodeReview like this would only fail if it had bogus data to begin with, but
-                                                      // CodeReview I wanted to note this behavior which occurs in some implementations
-                                                      // CodeReview but not all. -patw
-   }
+    // Don't link if we already have a namespace linkage in place.
+    AssertWarn(mNameSpace == NULL, "SimObject::linkNamespaces -- Namespace linkage already in place");
 
-   // ClassName -> SuperClassName
-   if ( ( mNSLinkMask & LinkClassName ) && mClassName && mClassName[0] )
-   {
-      if( Con::linkNamespaces( parent, mClassName ) )
-         parent = mClassName;
-      else
-         mClassName = StringTable->EmptyString; // CodeReview (See previous note on this code)
-   }
+    if (mNameSpace)
+        return;
 
-   // ObjectName -> ClassName
-   StringTableEntry objectName = getName();
-   if( objectName && objectName[0] )
-   {
-      if( Con::linkNamespaces( parent, objectName ) )
-         parent = objectName;
-   }
+    // Start with the C++ Class namespace.
+    StringTableEntry parent = StringTable->insert( getClassName() );
 
-   // Store our namespace.
-   mNameSpace = Con::lookupNamespace( parent );
+    // Link SuperClass to C++ Class.
+    if ( mSuperClassName && mSuperClassName[0] )
+    {
+        if ( Con::linkNamespaces(parent, mSuperClassName) )
+            parent = mSuperClassName;
+        else
+            mSuperClassName = NULL;
+    }
+
+    // Link Class to SuperClass or Class to C++ Class.
+    if ( mClassName && mClassName[0] )
+    {
+        if ( Con::linkNamespaces(parent, mClassName) )
+            parent = mClassName;
+        else
+            mClassName = NULL;
+    }
+
+    // Get the object's name.
+    StringTableEntry objectName = getName();
+
+    // Link Object Name to Class/SuperClass/C++ Class.
+    if ( objectName && objectName[0] )
+    {
+        if ( Con::linkNamespaces(parent, objectName) )
+            parent = objectName;
+    }
+
+    // Store our namespace.
+    mNameSpace = Con::lookupNamespace(parent);
 }
 
+//-----------------------------------------------------------------------------
+
 void SimObject::unlinkNamespaces()
 {
-   if (!mNameSpace)
-      return;
-
-   // Restore NameSpace's
-   StringTableEntry child = getName();
-   if( child && child[0] )
-   {
-      if( ( mNSLinkMask & LinkClassName ) && mClassName && mClassName[0])
-      {
-         if( Con::unlinkNamespaces( mClassName, child ) )
-            child = mClassName;
-      }
+    // Stop if there is no assigned namespace.
+    if (!mNameSpace)
+        return;
 
-      if( ( mNSLinkMask & LinkSuperClassName ) && mSuperClassName && mSuperClassName[0] )
-      {
-         if( Con::unlinkNamespaces( mSuperClassName, child ) )
-            child = mSuperClassName;
-      }
+    // Get the object's name.
+    StringTableEntry child = getName();
 
-      Con::unlinkNamespaces( getClassName(), child );
-   }
-   else
-   {
-      child = mClassName;
-      if( child && child[0] )
-      {
-         if( ( mNSLinkMask & LinkSuperClassName ) && mSuperClassName && mSuperClassName[0] )
-         {
-            if( Con::unlinkNamespaces( mSuperClassName, child ) )
-               child = mSuperClassName;
-         }
+    // Unlink any possible namespace combination.
+    if ( child && child[0] )
+    {
+        // Object Name/Class
+        if ( mClassName && mClassName[0] )
+        {
+            if( Con::unlinkNamespaces(mClassName, child) )
+                child = mClassName;
+        }
+
+        // Object Name/SuperClass or Class/SuperClass
+        if ( mSuperClassName && mSuperClassName[0] )
+        {
+            if ( Con::unlinkNamespaces(mSuperClassName, child) )
+                child = mSuperClassName;
+        }
+
+        // Object Name/C++ Class or SuperClass/C++ Class
+        Con::unlinkNamespaces(getClassName(), child);
+    }
+    else
+    {
+        // No Object Name, so get the Class namespace.
+        child = mClassName;
+
+        // Unlink any possible namespace combination.
+        if ( child && child[0] )
+        {
+            // Class/SuperClass
+            if ( mSuperClassName && mSuperClassName[0] )
+            {
+                if ( Con::unlinkNamespaces(mSuperClassName, child) )
+                    child = mSuperClassName;
+            }
 
-         Con::unlinkNamespaces( getClassName(), child );
-      }
-      else
-      {
-         if( ( mNSLinkMask & LinkSuperClassName ) && mSuperClassName && mSuperClassName[0] )
-            Con::unlinkNamespaces( getClassName(), mSuperClassName );
-      }
-   }
+            // Class/C++ Class or SuperClass/C++ Class
+            Con::unlinkNamespaces(getClassName(), child);
+        }
+        else
+        {
+            // SuperClass/C++ Class
+            if ( mSuperClassName && mSuperClassName[0] )
+                Con::unlinkNamespaces(getClassName(), mSuperClassName);
+        }
+    }
 
-   mNameSpace = NULL;
+    // Reset the namespace.
+    mNameSpace = NULL;
 }
 
-void SimObject::setClassNamespace( const char *classNamespace )
+//-----------------------------------------------------------------------------
+
+void SimObject::setClassNamespace( const char* classNamespace )
 {
-    mClassName = StringTable->insert( classNamespace );
-    if (mFlags.test(Added))
+    StringTableEntry oldClass = mClassName;
+    StringTableEntry newClass = StringTable->insert(classNamespace);
+
+    // Skip if no change.
+    if (oldClass == newClass)
+        return;
+
+    // Unlink the old namespaces.
+    if ( isProperlyAdded() )
+        unlinkNamespaces();
+
+    // Assign the new class namespace.
+    mClassName = newClass;
+
+    // Link the new namespaces.
+    if ( isProperlyAdded() )
         linkNamespaces();
 }
 
-void SimObject::setSuperClassNamespace( const char *superClassNamespace )
+//-----------------------------------------------------------------------------
+
+void SimObject::setSuperClassNamespace( const char* superClassNamespace )
 {
-    mSuperClassName = StringTable->insert( superClassNamespace );
-    if (mFlags.test(Added))
+    StringTableEntry oldSuperClass = mSuperClassName;
+    StringTableEntry newSuperClass = StringTable->insert(superClassNamespace);
+
+    // Skip if no change.
+    if (oldSuperClass == newSuperClass)
+        return;
+
+    // Unlink the old namespaces.
+    if ( isProperlyAdded() )
+        unlinkNamespaces();
+
+    // Assign the new SuperClass namespace.
+    mSuperClassName = newSuperClass;
+
+    // Link the new namespaces.
+    if ( isProperlyAdded() )
         linkNamespaces();
 }
 
+//-----------------------------------------------------------------------------
+
 static S32 QSORT_CALLBACK compareFields(const void* a,const void* b)
 {
    const AbstractClassRep::Field* fa = *((const AbstractClassRep::Field**)a);

engine/source/sim/simObject.h

@@ -327,7 +327,7 @@ protected:
     virtual void onTamlCustomRead( const TamlCustomNodes& customNodes ) {}
     
 protected:
-    bool	mCanSaveFieldDictionary; ///< true if dynamic fields (added at runtime) should be saved, defaults to true
+    bool    mCanSaveFieldDictionary; ///< true if dynamic fields (added at runtime) should be saved, defaults to true
     StringTableEntry mInternalName; ///< Stores object Internal Name
 
     // Namespace linking
@@ -356,21 +356,6 @@ protected:
     inline S32 getScriptCallbackGuard( void )    { return mScriptCallbackGuard; }
 
 protected:
-    // By setting the value of mNSLinkMask in the constructor of a class that 
-    // inherits from SimObject, you can specify how the namespaces are linked
-    // for that class. An easy way to think about this change, if you have worked
-    // with this in the past is that ScriptObject uses:
-    //    mNSLinkMask = LinkSuperClassName | LinkClassName;
-    // which will attempt to do a full namespace link checking mClassName and mSuperClassName
-    // 
-    // and BehaviorTemplate does not set the value of NSLinkMask, which means that
-    // only the default link will be made, which is: ObjectName -> ClassName
-    enum SimObjectNSLinkType
-    {
-        LinkClassName = BIT(0),
-        LinkSuperClassName = BIT(1)
-    };
-    U8 mNSLinkMask;
     void linkNamespaces();
     void unlinkNamespaces();
 
@@ -427,9 +412,9 @@ public:
     inline void clearDynamicFields( void ) { if ( mFieldDictionary != NULL ) { delete mFieldDictionary; mFieldDictionary = new SimFieldDictionary; } }
 
     /// Set whether fields created at runtime should be saved. Default is true.
-    void		setCanSaveDynamicFields(bool bCanSave){ mCanSaveFieldDictionary	=	bCanSave;}
+    void        setCanSaveDynamicFields(bool bCanSave){ mCanSaveFieldDictionary =   bCanSave;}
     /// Get whether fields created at runtime should be saved. Default is true.
-    inline bool getCanSaveDynamicFields(void) const { return	mCanSaveFieldDictionary;}
+    inline bool getCanSaveDynamicFields(void) const { return    mCanSaveFieldDictionary;}
 
     /// These functions support internal naming that is not namespace
     /// bound for locating child controls in a generic way.
@@ -440,7 +425,7 @@ public:
     StringTableEntry getInternalName();
 
     /// Save object as a TorqueScript File.
-    virtual bool		save(const char* pcFilePath, bool bOnlySelected=false);
+    virtual bool        save(const char* pcFilePath, bool bOnlySelected=false);
 
     /// Check if a method exists in the objects current namespace.
     virtual bool isMethod( const char* methodName );
@@ -450,7 +435,7 @@ public:
     /// @{
 
     ///
-    SimObject( const U8 namespaceLinkMask = LinkSuperClassName | LinkClassName );
+    SimObject();
     virtual ~SimObject();
 
     virtual bool processArguments(S32 argc, const char **argv);  ///< Process constructor options. (ie, new SimObject(1,2,3))
@@ -720,8 +705,8 @@ public:
 
     /// @}
 
-	virtual void			dump();
-    virtual void			dumpClassHierarchy();
+    virtual void            dump();
+    virtual void            dumpClassHierarchy();
 
     static void initPersistFields();
     SimObject* clone( const bool copyDynamicFields );
@@ -734,4 +719,4 @@ public:
     DECLARE_CONOBJECT(SimObject);
 };
 
-#endif // _SIM_OBJECT_H_
+#endif // _SIM_OBJECT_H_