Bug Patch: Scheduling Causes Parameter Mangling with Behaviors (A Small Patch)

OK. I'm going to try a bug fix patch and see if others can apply it. This one is drop-dead simple because it involves a few lines in one function (in one file). It should be good to see if my patches (created with Subversion) can be used from whatever tool you guys like. (What tools *do* you use for patches?)

-----

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

Bug:

When scheduling a method on a scene object with behaviors, the call should, and does, propagate to the children as well as the scene object. However, as it does, the scheduled function's parameters get mangled. The first behavior that receives the scheduled function gets it fine, but then all other children and parent get mangled copies of the schedule call. The reason is that argv is tweaked but not restored in the ConsoleMethod(SimObject,schedule, ...) function.

Example:

Assume a scene object class Obj1 that has behaviors B1 and B2. Now make an instance of Obj1 called %obj. (Assume for the sake of clarity that it's behaviors are "named" %obj.b1 and %obj.b2 even though behaviors don't have names.) Further, assume you have a method you want to call, named doIt() and you want to schedule it like so:

%obj.schedule(100, doIt, param1, param2);

This should, and almost does, create a scheduled event on the parent %obj, and also it's behavior instances. Ideally 100ms later, you would get three calls:

// 100ms from now do these should happen
%obj.doIt(param1, param2);
%b1.doIt(param1, param2);
%b2.doIt(param1, param2);

Unfortunately, do to some parameter mangling, you will create something more like:

%obj.1417(mess, param1);
%b1.doIt(param1, param2);
%b2.1432(moremess, whoknows);

So what will happen is that the first behavior will work, but the second behavior and the owner will get botched function calls, with names that typically will silently fail out -- and cost you engine time for no output.

Patch:

Files Affected:
simbase.cc

Included:
included in-line. The original file name was 'tgb175_schedule_fix_2012-02-06.patch' and it was a "unified diff" meaning it has context in it so that line-numbers aren't as important.

Index: simBase.cc
===================================================================
--- simBase.cc	(revision 193)
+++ simBase.cc	(revision 194)
@@ -811,9 +811,16 @@
 																"@sa See the schedule console function and its corresponding helper functions.")
 {
    U32 timeDelta = U32(dAtof(argv[2]));
+
+   // quick (hack) way to swap some parameters in argv,
+   // and then put them back when we're done.
+   const char* argv2 = argv[2];
+   const char* argv3 = argv[3];
    argv[2] = argv[3];
    argv[3] = argv[1];
    SimConsoleEvent *evt = new SimConsoleEvent(argc - 2, argv + 2, true);
+   argv[2] = argv2;
+   argv[3] = argv3;
    S32 ret = Sim::postEvent(object, evt, Sim::getCurrentTime() + timeDelta);
 // #ifdef DEBUG
 //    Con::printf("obj %s schedule(%s) = %d", argv[3], argv[2], ret);

Caveats!
If you are used to the broken behavior, this fix may cause one unexpected result.

Specifically, if you schedule a method on an owner from outside that owner (ah, the quirks of behaviors) you may run some methods twice. Why? The schedule call itself will be run for the owner and the behaviors. Now both the owner and the behaviors have a scheduled method set to run. When the time comes, the behaviors will run the method. So far so good. But when the owner also runs the scheduled method, it will naturally propagate the method to the behaviors, so they will each run it again.

In practice, out of 100+ behaviors this only occurred once on me (that I saw). I think the main reason is that you usually schedule methods on an object from within that object (%this.owner.schedule(...)). In this case, as T2D is currently designed, the schedule call itself will only run on the owner, not its children. When the time comes, only the owner has the method to run and it will propagate that method to the children, for a total of once. QED.

(I do have another fix for that, though. :P)