Game Development Community

Feature Patch - Full Behavior Propagation

by Charlie Patterson · in Torque Game Builder · 02/10/2012 (5:14 pm) · 4 replies

Feature Patch - Full Behavior Propagation

(Best to install other patches first)
This will probably work best with the two other behavior patches I put out here, but I don't think it's necessary. patch 1 and patch 2

Code Base: TGB Pro 1.7.5 (should work for 1.7.6 also)

Problem:

Torque2D has a most excellent idea called Behaviors. With it, you can disperse your code into pieces and these pieces will work together as one big script. For instance, say you have a way of handling collisions and you share it with different types of game entities, we'll say player and rock. You can write the "onCollision()" function in a behavior and put this behavior in both the player and the rock. Then, when your t2dSceneObject, say player, receives an "onCollision()", player will try each behavior to see if it will handle it. This is most excellent.

However, I wanted to do a bit more. :) I would like to think of behaviors as independent and interchangeable. Here's two usage examples:

Use 1)
Let's say I have a type of weapon that requires any entity using it to stand still. I'll write this weapons firing as a behavior but I don't want the weapon to have to know more about the entity's movement. For instance, I don't want to set the scene object's velocity to 0. And I don't want to have to find the movement behavior for the entity (assuming there is one) and call it's "stop()" method. I shouldn't know the type of the other behavior anyway. Maybe there are five possible movement behaviors I've written and I don't want to know which one we have as a "sister behavior" in this object. Instead I want a general method name that all movement behaviors will respect, let's say "stopMoving()". If I could send the entire object a stop movement and get it to propagate to any movement behaviors, assuming there are any, I would. Unfortunately, the following will not work!

%this.owner.stopMovement();  // doh!  won't work

The industrious may have figured out that you can do a hack for this by doing "%this.owner.schedule(0, stopMovement)" but yuck.

Use 2)
I want to break my behaviors into "back-end" simulations and "front-end" presentation logic. This isn't always possible, but it's nice for prototyping and what-not. For instance, I write a monster and he uses a behavior called "takeDamageBehavior" which I think of as a backend behavior. It keeps track of his health, maybe heals a bit over time, and ultimately dies. But how to show he is getting hurt? Well, I could gum up my damage behavior by writing something -- maybe he shrinks, or turns red, or changes sprites. But what I'd like to do is write a "presentation" behavior that understands to respect calls to "changeHealth(%amount)" and adjust the look of the character accordingly. This plays out like "use 1" above. We would like to do

%this.health -= %damage;
	%this.owner.changeHealth(%this.health)

Any "presentation" behavior would have a changeHealth() method and could shrink, blend red, or whatever.

So why doesn't this work?

Torque2D has a heuristic for when to propagate methods to behaviors and when not to. In short, once a (the first) behavior method has been called, the ability to propagate to more methods is blocked. In other words, even though the engine can call an object method and have it propagate to the behaviors -- "onCollision" or "onUpdate" or "onMouseDown", a behavior method doesn't get the same privilege. When a behavior method calls its owner with a method, such as "%this.owner.stopMovement()" intended for sister behaviors, nothing will happen. On purpose.

The industrious may have also noticed that this isn't true *across* objects. For instance, if my bullet where to call "enemy.stopMovement()" that would get propagated to the behaviors of the enemy. But if the enemy where to say on itself "%this.owner.stopMovement()" that won't propagate. Ugh. So to me, this leads to more confusion than safety or whatever.

Got a fix?
How about a different "heuristic" for when to propagate to behaviors and when not to:

* engine calls don't pass down to child behaviors. for instance, setPosition, delete, schedule, etc. don't propagate
* script calls always propagate to child behaviors. for instance, onUpdate, stopMovement, changeHealth, etc pass to behaviors.

For a few months now I've been using this variation and I love it. I was a bit concerned at first, but it works great! It even removes a few bugs. For instance, I was getting two calls to behaviors for scheduled event on the owner. Can you see why? (hint schedule() propagated to owner and children and then also ran on owner and children) But now, the code recognized schedule() as an engine function so it only runs on the owner, which is what I wanted.

One last bonus. You will get the old behavior until you set

$Con::fullBehaviorPropagation = true;

This should be completely backwards compatible. If T2D doesn't see that variable, or you set it to false, you will still get the classic way of working with behaviors.

Patch:
Files Affected:
component/behaviors/behaviorComponent.cpp
console/console.cc
console/console.h
console/simBase.cc
console/simBase.h

Included:
included in-line. The original file name was 'tgb175_behaviorpropagate_fix_2012-02-10.patch' and it was a "unified diff" meaning it has context in it so that line-numbers aren't as important. It should be patched from your "source/" directory.

Index: component/behaviors/behaviorComponent.cpp
===================================================================
--- component/behaviors/behaviorComponent.cpp	(revision 192)
+++ component/behaviors/behaviorComponent.cpp	(working copy)
@@ -105,7 +105,9 @@
    // again on the behavior and cause an infinite loop so we mark it when calling the behavior
    // method and trap it if it reenters and force it to call the method on this object.
    // This is a quick fix for now and I will review this before the end of the release.
-   if( mInBehaviorCallback ) 
+   // [charliep, 12/7/2011]
+   // then again, if you *do* want recursive calls you can get it by turning on gFullBehaviorPropagation
+   if(!gFullBehaviorPropagation && mInBehaviorCallback)
       return Parent::_callMethod( argc, argv, true );
    
    // CodeReview The tools ifdef here is because we don't want behaviors getting calls during
@@ -150,7 +152,10 @@
          // [neo, 5/10/2007 - #3010]
          // Set flag so we can call the method on this object directly, should the
          // behavior have 'overloaded' a method on the owner object.
-         mInBehaviorCallback = true;
+         // [charliep, 12/7/2011]
+         // want to allow recursion if the developer wants it, though.
+         if (!gFullBehaviorPropagation)
+			   mInBehaviorCallback = true;
 
          // Change the Current Console object, execute, restore Object
          SimObject *save = gEvalState.thisObject;
@@ -160,6 +165,8 @@
 
          // [neo, 5/10/2007 - #3010]
          // Reset flag
+         // [charliep, 12/7/2011]
+         // won't hurt to set this to false, so why test for full behavior propagation?
          mInBehaviorCallback = false;
       }
    }
@@ -178,6 +185,13 @@
 
 bool BehaviorComponent::handlesConsoleMethod( const char *fname, S32 *routingId )
 {
+   // [charlie; 12/15/2011]
+   // Check if this is a function from the "engine" (getPosition, setName, schedule, delete, etc.)?
+   // If so, do not check if behaviors could handle it, because we're not interested in propagating it to them.
+   if (gFullBehaviorPropagation) {
+      if (this->isMethod(fname) && ! this->isConsoleMethod(fname))
+      return Parent::handlesConsoleMethod( fname, routingId );
+   }
 
    // CodeReview [6/25/2007 justind]
    // If we're deleting the BehaviorComponent, don't forward the call to the
@@ -188,9 +202,18 @@
    if( dStricmp( fname, "delete" ) == 0 )
       return Parent::handlesConsoleMethod( fname, routingId );
 
+   // [charlie, 12/7/2011]
+   // Seems like a bad place to block recursive behaviors?!
+   // If you want to avoid accidental recursion, that can be done when calling the behavior methods,
+   // not when reporting if it exists as we do here.  (The process of blocking recursion does indeed
+   // happen at that point.)
+   // In any case, if gFullBehaviorPropagation is on, we always check behaviors here.  Otherwise, we
+   // get the original logic.
+
    // [neo, 5/10/2007 - #3010]
    // Make sure we honor the flag!
-   if( !mInBehaviorCallback )
+
+   if(gFullBehaviorPropagation || !mInBehaviorCallback )
    {
       for( SimSet::iterator nItr = mBehaviors.begin(); nItr != mBehaviors.end(); nItr++ )
       {
Index: console/console.cc
===================================================================
--- console/console.cc	(revision 192)
+++ console/console.cc	(working copy)
@@ -27,6 +27,7 @@
 StmtNode *statementList;
 ConsoleConstructor *ConsoleConstructor::first = NULL;
 bool gWarnUndefinedScriptVariables;
+bool gFullBehaviorPropagation;
 
 static char scratchBuffer[4096];
 
@@ -240,6 +241,7 @@
    logFileName                   = NULL;
    newLogFile                    = true;
    gWarnUndefinedScriptVariables = false;
+   gFullBehaviorPropagation       = false;
 
 #ifdef TORQUE_MULTITHREAD
    // Note the main thread ID.
@@ -258,6 +260,7 @@
    addVariable("Con::logBufferEnabled", TypeBool, &logBufferEnabled);
    addVariable("Con::printLevel", TypeS32, &printLevel);
    addVariable("Con::warnUndefinedVariables", TypeBool, &gWarnUndefinedScriptVariables);
+   addVariable("Con::fullBehaviorPropagation", TypeBool, &gFullBehaviorPropagation);
 
    // Current script file name and root
    Con::addVariable( "Con::File", TypeString, &gCurrentFile );
Index: console/console.h
===================================================================
--- console/console.h	(revision 192)
+++ console/console.h	(working copy)
@@ -22,6 +22,7 @@
 ///
 /// @note This is set and controlled by script.
 extern bool gWarnUndefinedScriptVariables;
+extern bool gFullBehaviorPropagation;
 
 enum StringTableConstants
 {
Index: console/simBase.cc
===================================================================
--- console/simBase.cc	(revision 190)
+++ console/simBase.cc	(working copy)
@@ -996,6 +996,23 @@
    return false;
 }
 
+bool SimObject::isConsoleMethod( const char* methodName )
+{
+   if( !methodName || !methodName[0] )
+      return false;
+
+   StringTableEntry stname = StringTable->insert( methodName );
+
+   Namespace::Entry* nsEntry = NULL;
+   if( getNamespace() )
+       nsEntry = getNamespace()->lookup( stname );
+
+   if( nsEntry )
+      return nsEntry->mType == Namespace::Entry::ScriptFunctionType;
+
+   return false;
+}
+
 ConsoleMethod(SimObject, isMethod, bool, 3, 3, "obj.isMethod(string method name)")
 {
    return object->isMethod( argv[2] );
@@ -1748,8 +1765,17 @@
 // #ifdef DEBUG
 //    Con::printf("Executing schedule: %d", sequenceCount);
 // #endif
-   if(mOnObject)
-      Con::execute(object, mArgc, const_cast<const char**>( mArgv ));
+   if(mOnObject) {
+      if (gFullBehaviorPropagation) {
+         // engine methods (setPosition, delete, schedule, etc.) shouldn't propagate to behaviors.
+         // note that the method may not exist in parent, in which case assume children may have console methods
+         bool ownerOnly = (object->isMethod(mArgv[0]) && ! object->isConsoleMethod(mArgv[0]));
+         Con::execute(object, mArgc, const_cast<const char**>( mArgv ), ownerOnly );
+      }
+      else {
+         Con::execute(object, mArgc, const_cast<const char**>( mArgv ));
+      }
+   }
    else
    {
       // Grab the function name. If '::' doesn't exist, then the schedule is
Index: console/simBase.h
===================================================================
--- console/simBase.h	(revision 192)
+++ console/simBase.h	(working copy)
@@ -615,6 +615,9 @@
    /// Check if a method exists in the objects current namespace.
    virtual bool isMethod( const char* methodName );
    /// @}
+   /// Check if a method exists and is a console function in the objects current namespace.
+   virtual bool isConsoleMethod( const char* methodName );
+   /// @}
 
    /// @name Initialization
    /// @{

Caveats:
I think it's important to put the other two patches in first but not necessary. They are listed at the top of this post.

I don't think I saw any problems due to this, believe it or not. The other patches are more likely to uncover some latent bug. I also went through my code and replaced many, many "%this.owner.schedule(0, function);" with "%this.owner.function()". Yay!

#1
02/11/2012 (10:19 am)
Nice idea. How is the performance compared to stock? How does it work with onUpdate or other basics?
#2
02/11/2012 (11:25 am)
That's cool, nice! Every time I run into behavior problems I always just run to scripting. GG needs to implement your patches into the next build. I'm a pro owner but I don't mess with the engine code because it time confusing. I usually find a hack like yours or a completely new way to solve the problem. I appreciate your effort. You are pushing the progress of T2D forward to reach it's potential.
#3
02/11/2012 (2:06 pm)
@Ronny, I'm "on-site" with my artist today (yay!) so I only have a minute.

In general, it works the same for onUpdate, etc, because these were always "script" functions, not engine functions. They propagate to behaviors as always. Now, however, if you do try to call %this.owner.something from within onUpdate, it can propagate to the behaviors where before it couldn't. Someone may have some code that was relying on NOT propagating inside of onUpdate, but it didn't happen to me.

As for performance, I didn't check the performance but I imagine it could possibly be faster! Before, T2D checked each behavior to see if it supplied the method. On the good hand, it only did this on the first call to an owner. But on the bad hand it did this to *every* call including engine functions such as setPosition, setName, getCollisionMask or whatever. (Yes, for the industrious, you could write a behavior method B::setPosition and have it called! But then what? This is almost certainly a bug in the original behavior design.)

The new code does not even run through behaviors and check for the method if the owner handles the call with an engine-based method. I'd guess, pulling a number out of my butt that 9 out of 10 method calls are either to the engine or to the current behavior (%this.fire(), not %this.owner.fire()). So it is only a small portion of the time that you are checking the behaviors for a method, whereas before you checked on every onUpdate, onCollision, %bullet.stop(), etc, etc. but only the first call. Phew. (-:
#4
02/11/2012 (2:07 pm)
Thanks @Glenn!

I love T2D. I also tried to avoid engine coding, but I had too many things I needed. I'm just glad to hear the community uses behaviors and may enjoy these updates. I'd love to see them get into T2D so I wouldn't have to maintain them myself.