T3D 1.1 Beta 2 - namespace linking changed - LOGGED

#1 Logged as TQA-1023 for the QA Team to verify.

#2
Looking into this now.

The problem is that while the new code allows deeper hierarchies, it only takes namespace hierarchies produced by class linking into account and will not work when the package functionality is rearranging namespace hierarchies.

In fact, what you show above works as intended if you make sure the class linking code gets to execute before the compiler tries to link in the package.

Looking into a fix now. The new functionality is good so I'd like to preserve that but it should cope with packages linking namespaces too.

#3 Rene, thanks for you attention to this. I know that it has caused me issues with packages as it has with Yuri's code.

#4 @Rene
"In fact, what you show above works as intended if you make sure the class linking code gets to execute before the compiler tries to link in the package."

Can you give an example of this? Should the script Yuri posted be moved back into gameCore.cs for instance?

#5
Moving it into gameCore.cs will lead to the same problem. Unfortunately, placement of the code alone will not solve the problem. What you need to ensure is that in this particular case the namespace linking triggered by the "Game" ScriptObject that loadMissionStage2() creates executes *before* you activate your package.

Put another way, the whole problem would go away if SimObject::linkNamespaces and SimObject::unlinkNamespaces would deactivate all packages before doing their thing and re-activate them after they're done. However, that would be prohibitively expensive. The stuff they do already takes up way too many cycles due to the entire setup not making too much sense and the namespace code being rather poor.

I have a few improvements on the performance front but I'm still thinking about how to solve the core problem here without introducing any big changes. If that weren't an issue, I'd completely rip out the nonsense 'class' and 'superClass' fields and do the whole thing quite differently (and in a far less convoluted way).

#6
Addendum: Best solution I have so far is making the class namespace linking code correctly deal with the hybrid class/package hierarchies by patching the parent links correctly around the packages but this is starting to get ridiculously complex.

#7
Ok, finally sat down to solve this one. Thinking about it, the solution is rather simple. All the code has to do is simply parent-link to the bottom-most namespace in a package chain and child-link to the topmost namespace in each chain. While in the process though, I ended up completely redoing the linkNamespaces() and unlinkNamespaces() methods to clean everything up and also do away with all the needless Con::lookupNamespace() calls.

So far in preliminary testing, everything seems to be working well but I'll be posting my changes here in the following posts and it'd be great if you guys could give this a quick test run.

#8 Replace SimObject::linkNamespaces() with:

void SimObject::linkNamespaces()
{
   // Don't link if we already have a namespace linkage in place.
   // If you want to change namespace linking, first call unlinkNamespaces()
   // while still having the class namespace fields matching the current
   // setup.

   AssertWarn( mNameSpace == NULL, "SimObject::linkNamespaces -- Namespace linkage already in place" );
   if( mNameSpace )
      return;

   // Get the namespace for the C++ class.

   Namespace* cppNamespace = getClassRep()->getNameSpace();

   // Parent namespace defaults to namespace of C++ class.

   Namespace* parentNamespace = cppNamespace;

   // Perform superclass linking, if requested.

   if( mSuperClassName && mSuperClassName[ 0 ] )
   {
      // Look up the superclass namespace.

      Namespace* superClassNamespace = Con::lookupNamespace( mSuperClassName );

      // If packages are active and adding to the superclass namespace, then we will
      // have multiple packages in a parent chain that all have the same name.
      // Con::lookupNamespace returns the bottom-most package in the chain to us so
      // in order to properly link namespace here without conflicting with the package
      // mechanism, we need to properly link child namespaces to the bottom-most namespace
      // while linking parent namespaces to the topmost namespace.  To find the latter
      // one, we walk up the hierarchy here.

      Namespace* superClassNamespacePackageRoot = superClassNamespace->getPackageRoot();

      // Link the superclass namespace to the C++ class namespace.

      if( superClassNamespacePackageRoot->getParent() == NULL )
      {
         // The superclass namespace isn't linked yet so we just
         // link it to the C++ class namespace and make that our parent.
         // No increasing parent reference counts is needed in this case.

         bool ok = superClassNamespacePackageRoot->classLinkTo( cppNamespace );
         AssertFatal( ok, "SimObject::linkNamespaces - failed to link new namespace to c++ class name" );
         parentNamespace = superClassNamespace;
      }
      else
      {
         // In debug builds, make sure the namespace hierarchy that's been
         // put into place actually makes sense and leads back to the C++
         // class namespace.

#ifdef TORQUE_DEBUG

         bool foundClassNameNS = false;
         for(  Namespace* linkWalk = superClassNamespacePackageRoot->getParent(); linkWalk != NULL;
               linkWalk = linkWalk->getParent() )
         {
            if( linkWalk == cppNamespace )
            {
               foundClassNameNS = true;
               break;
            }
         }

         if( !foundClassNameNS )
         {
            // C++ class namespace not in parent link chain.  Warn about it.

            Con::errorf(
               "SimObject::linkNamespaces - cannot link object to superclass %s because c++ class %s is not in the parent namespace chain.  Linking object to c++ class.",
               mSuperClassName,
               getClassName()
            );
            
            // Clear out superclass name so we don't come across it during
            // unlinking.

            mSuperClassName = NULL;
         }
         else

#endif

// CONTINUED IN NEXT POST DUE TO CHARACTER LIMIT

#9
Rest of linkNamespaces()

{
            // Super link is ok.

            parentNamespace = superClassNamespace;

            // Now increase the reference count of all namespaces in the parent hierarchy
            // (up to the C++ class).

            for( Namespace* linkWalk = parentNamespace;
                 linkWalk != NULL && linkWalk != cppNamespace && linkWalk->getParent() != NULL;
                 linkWalk = linkWalk->getParent() )
            {
               // Skip namespaces coming from packages.
               if( linkWalk->getPackage() != NULL )
                  continue;

               linkWalk->incRefCountToParent();
            }
         }
      }
   }

   // If class name is set, link it in as the new parent
   // which itself inherits from the current parent.

   if( mClassName && mClassName[ 0 ] )
   {
      Namespace* classNamespace = Con::lookupNamespace( mClassName );
      if( classNamespace && classNamespace->classLinkTo( parentNamespace ) )
      {
         parentNamespace = classNamespace;
      }
      else
      {
         // Clear out class name so we don't perform a bogus unlink
         // in unlinkNamespaces().
         mClassName = NULL;
      }
   }

   // Finally, if we have an object name, link its namespace
   // as the child to the current parent namespace and let it
   // become the final namespace of this object.

   StringTableEntry objectName = getName();
   if( objectName && objectName[ 0 ] )
   {
      Namespace* objectNamespace = Con::lookupNamespace( objectName );
      if( objectNamespace && objectNamespace->classLinkTo( parentNamespace ) )
      {
         parentNamespace = objectNamespace;
      }
   }

   // Store our namespace.

   mNameSpace = parentNamespace;
}

#10
Replace SimObject::unlinkNamespaces() with:

void SimObject::unlinkNamespaces()
{
   if( !mNameSpace )
      return;

   Namespace* cppNamespace = getClassRep()->getNameSpace();
   Namespace* parentNamespace = cppNamespace;

   // Handle superclass.

   if( mSuperClassName && mSuperClassName[ 0 ] )
   {
      // Get the superclass namespace.

      Namespace* superClassNamespace = Con::lookupNamespace( mSuperClassName );

      // Make it the parent namespace.
      
      parentNamespace = superClassNamespace;

      // Decrease parent refcounts on the superclass hierarchy.

      for( Namespace* linkWalk = superClassNamespace;
           linkWalk != NULL && linkWalk != cppNamespace && linkWalk->getParent() != NULL; )
      {
         // Store the parent link since it may disappear once we
         // decrease the reference count.
         Namespace* parent = linkWalk->getParent();

         // Decrease the refcount.
         if( linkWalk->getPackage() == NULL ) // Skip namespaces coming from packages.
            linkWalk->decRefCountToParent();

         // Walk up.
         linkWalk = parent;
      }
   }

   // Handle class.

   if( mClassName && mClassName[ 0 ] )
   {
      Namespace* classNamespace = Con::lookupNamespace( mClassName );
      if( classNamespace )
      {
         classNamespace->decRefCountToParent();
         parentNamespace = classNamespace;
      }
   }

   // Handle object name.

   StringTableEntry objectName = getName();
   if( objectName && objectName[ 0 ] )
      mNameSpace->decRefCountToParent();

   mNameSpace = NULL;
}

#11
Add these methods to the Namespace class interface in consoleInternal.h:

/// Return the topmost package in the parent hierarchy of this namespace
      /// that still refers to the same namespace.  If packages are active and
      /// adding to this namespace, then they will be linked in-between the namespace
      /// they are adding to and its real parent namespace.
      Namespace* getPackageRoot()
      {
         Namespace* walk = this;
         while( walk->mParent && walk->mParent->mName == mName )
            walk = walk->mParent;

         return walk;
      }

      /// Return the package in which this namespace is defined.
      StringTableEntry getPackage() const { return mPackage; }

      /// Increase the count on the reference that this namespace
      /// holds to its parent.
      /// @note Must not be called on namespaces coming from packages.
      void incRefCountToParent()
      {
         AssertFatal( mPackage == NULL, "Namespace::incRefCountToParent - Must not be called on a namespace coming from a package!" );
         mRefCountToParent ++;
      }

      /// Decrease the count on the reference that this namespace
      /// holds to its parent.
      /// @note Must not be called on namespaces coming from packages.
      void decRefCountToParent()
      {
         unlinkClass( NULL );
      }

#12
Replace Namespace::unlinkClass() with

bool Namespace::unlinkClass( Namespace *parent )
{
   AssertFatal( mPackage == NULL, "Namespace::unlinkClass - Must not be called on a namespace coming from a package!" );

   // Skip additions to this namespace coming from packages.

   Namespace* walk = getPackageRoot();

   // Make sure "parent" is the direct parent namespace.

   if( parent != NULL && walk->mParent && walk->mParent != parent )
   {
      Con::errorf(ConsoleLogEntry::General, "Namespace::unlinkClass - cannot unlink namespace parent linkage for %s for %s.",
         walk->mName, walk->mParent->mName);
      return false;
   }

   // Decrease the reference count.  Note that we do this on
   // the bottom-most namespace, i.e. the one guaranteed not 
   // to come from a package.

   mRefCountToParent --;
   AssertFatal( mRefCountToParent >= 0, "Namespace::unlinkClass - reference count to parent is less than 0" );

   // Unlink if the count dropped to zero.

   if( mRefCountToParent == 0 )
   {
      walk->mParent = NULL;
      trashCache();
   }

   return true;
}