Bad Behavior

#1 You're right, it should return a behavior instance.

/*! gets a behavior
    @param BehaviorTemplateName The name of the template of the behavior instance you want
    @return (BehaviorInstance bi) The behavior instance you requested
*/
ConsoleMethodWithDocs( BehaviorComponent, getBehavior, ConsoleInt, 3, 3, (string BehaviorTemplateName))
{
   BehaviorInstance* pBehaviorInstance = object->getBehavior( StringTable->insert( argv[2] ) );

   return pBehaviorInstance ? pBehaviorInstance->getId() : 0;
}

If it does not then something strange is happening.

But if you have more than one instance of a behavior on an object I think you have to do some fiddling to get a particular one, something like this:

// loop through behaviors and look for the Idle In Launcher animation behavior
    for(%i = 0; %i < %projectile.getBehaviorCount(); %i++)
    {
        %tempBeh = %projectile.getBehaviorByIndex(%i);
        if (%tempBeh.template.getName() $= "AnimationEffectBehavior")
        {
            if (%tempBeh.instanceName $= "IdleInLauncher")
                return %tempBeh.getAsset();
        }
    }

And this requires you to name instances to differentiate among them on a particular object.

#2 There's only once instance of the behavior on the object. (I'm using the AudioToy in the development branch to test)

The reason for all of this is to figure out why behavior IDs are not being written out in TAML.

It is strange. I'm peppering the source now with console spam. In addBehavior, I can see the correct instance id:

bool BehaviorComponent::addBehavior( BehaviorInstance* bi )
{
    if( bi == NULL || !bi->isProperlyAdded() )
        return false;

    // Store behavior.
    mBehaviors.pushObject( bi );

    // Notify if the behavior instance is destroyed.
    deleteNotify( bi );

    // Set the behavior owner.
    bi->setBehaviorOwner( this );

    // Allocate a behavior Id.
    bi->setBehaviorId( mMasterBehaviorId++ );

    Con::warnf("Master behavior id is '%d'.", mMasterBehaviorId);

    Con::warnf("Instance id is '%d'.", bi->getBehaviorId() );

    if( bi->isMethod("onBehaviorAdd") )
        Con::executef( bi , 1, "onBehaviorAdd" );

    return true;
}

But a thousand lines of code down in the same file, the onTamlCustomWrite doesn't work, the ID is always zero:

// Iterate behaviors.
    for( SimSet::iterator behaviorItr = mBehaviors.begin(); behaviorItr != mBehaviors.end(); ++behaviorItr )
    {
        // Fetch behavior.
        BehaviorInstance* pBehaviorInstance = dynamic_cast<BehaviorInstance*>( *behaviorItr );

        // Fetch template.
        BehaviorTemplate* pBehaviorTemplate = pBehaviorInstance->getTemplate();

        // Add behavior node.
        TamlCustomNode* pBehaviorNode = pCustomBehaviorNode->addNode( pBehaviorInstance->getTemplateName() );

        // Add behavior Id field.
        pBehaviorNode->addField( behaviorIdFieldName, pBehaviorInstance->getBehaviorId() );

        Con::warnf( "BehaviorComponent::onTamlCustomWrite() - Encountered a behavior Id of '%d'.", pBehaviorInstance->getBehaviorId());

#3 More strangeness...somewhere the wires are crossed.

id is 1741 // this is the AnimationBehavior instance on the scene object in question
==>echo(1741.getName());

==>echo(1741.getInternalName());

==>echo(1741.getClassName());
BehaviorInstance
==>1741.dumpClassHierarchy();
BehaviorInstance ->
SimObject ->
==>echo(1741.getBehaviorId());  // I added this to my version of T2D as a script method for BehaviorInstance
<input> (0): Unknown command getBehaviorId.
  Object (1741) AnimationBehavior -> BehaviorTemplate -> SimObject

==>echo(1741.getBehaviorFieldCount()); // This is a method that belongs to BehaviorTemplate
0
==>echo(AnimationBehavior.getBehaviorFieldCount());
1

I won't post the dump results but all the listed methods are from BehaviorTemplate/SimObject and not BehaviorInstance/SimObject.

#4 Time to track back a ways - this all used to work. In fact, Three-Step Studio depended on it. The "physics launcher" template projectile had something like 17 different behavior instances on it to control damage, animations, animation state changes, state change sound effects, etc. and all of that was written to TAML and loaded back in. So somewhere along the line a change was made that broke something critical.

#5 I might try bisecting through commits on Windows to find. Do we know if this problem occurs on the master branch?

#6 To summarize things, there are currently the following identified issues:

1. You cannot export an object with behaviors using TAML and then import it back into the engine. Behavior Ids are being written out as zero (invalid). Temp solution is to manually edit the TAML file and remove the Id field - the engine will correctly assign Ids then internally upon import.
2. Behavior Connections made through TAML are broken. Part of the issue is the same as #1, in that the connection Id is being written out as zero. The other part is that the Connection node tag is being read in as a behavior name instead of a behavior connection. I've posted a fix for second part of this issue on Github, but that code is not yet in development. This allows you to manually edit Ids and make connections that work upon import.
3. The BehaviorInstance class suffers from a split personality. As highlighted above, for some SimObject methods a BehaviorInstance will identify itself as a BehaviorInstance but all ConsoleMethods and fields are inherited only from BehaviorTemplate. This may or may not be causing the mBehaviorId variable to get lost/reset to 0 and affecting exports in issue #1 and #2. The BehaviorTemplate cloning is happening in the BehaviorInstance constructor but I don't really see an issue there - unless somehow the namespaces are getting muddled together.

I've had a casual look through the commit history and there were major changes made in the post-MIT launch master branch with TAML custom nodes and how behavior connections are made. I haven't had a chance to look at the current master branch or the first MIT commit version to test things though. I'd even like to look at the TGB 1.8 source to see how the BehaviorInstance class worked back in the commercial engine.

#7 What happens when you comment out setting mNameSpace in the BehaviorInstance's onAdd?

#8 It's like winning the battle but losing the war. Commenting out the mNameSpace assignment does fix the BehaviorInstance thinking it is a BehaviorTemplate issue but you lose complete access to all the script functions that are part of the template's namespace. I don't know if a better approach would be to link the namespaces like you would a class or superclass?

It has no impact on the Id = 0 problem.

Doing a bit more debugging - through the onBehaviorAdd callback, I can get the Id and it is the correctly assigned number. Afterwards, in the onAddToScene callback, the Id is back to 0. So somewhere in-between the Id is being reset.

#9 Looks like the taml read was always overriding the default behavior id, regardless of whether the id was actually in the taml file.

Here's the commit: github.com/camporter/Torque2D/commit/3b8913c2b807ef8cfa56ef14ecc04d226b552a8e

#10 github.com/GarageGames/Torque2D/pull/183

Works like a charm now. Thanks for your help Frogger.