Event Manager postEvent script callback error

[capnlove]

Context

This occurs when an eventManager posts an event (via its postEvent ConsoleMethod) to its subscribers.
The code loops through all Simobject listeners (subscribers) and executes a script callback on each one.

/source/messaging/eventManager.cc
function EventManagerListener::onMessageReceived(...

const char* conResult = Con::executef( iter->listener, 2, iter->callback, data );
      if( dStrcmp( conResult, "" ) != 0 && dAtob( conResult ) == false )
         return false;

If the callback function returns a value and this value is evaluated to "false", the loop stops and all remaining subscribers are completely ignored.

In my code, the callback function contained one line of code

function GameManager::onPlayBall_EVT(%this, %data)
{
%this.CurrentState = "Restart";
}

This code assigns a string to a dynamically created variable (CurrentState) on a ScriptObject.
The ScriptObject named GameManager is a subscriber to the Event Manager.

Here is the problem :

The C++ code takes the string "Restart" as the return value of the callback function.
Why would this value be returned from Con::executef?

Using functions like echo(...) or complex physics manipulation never cause this problem, the only way I've managed to reproduce this is with simple dynamic variable assignations as illustrated here.

Hack city

Adding "return true;" at the end of the script function solves the issue.
I still think this points to a problem with the scripting engine.

[dottools]

I can't find it at the moment, but seems awfully similar to an issue Torque 3D ran into couple years back.

[greenfire27]

So you're saying that if you set %this.currentState to false or 0 that it would prevent other callbacks from firing? Sounds like trouble...

[capnlove]

@greenfire27 No, the code above prevents further callbacks as it is (with "Restart" as a value), setting it to false or 0 has the same effect. i.e. T2D evaluates the last statement in these callback methods as the return value of the method, even when no return value is specified. Sounds like a nested issue in Con::executef.

For the record, in my test project, I had to modify every callback function so that it ended with the statement :

return true;

and no further problems were had.

- Testbed -

I know the Event Manager sees very little use so here's a quick blurb to get started if that's the case.

new EventManager(EVTMGR){
queue = "My Event Manager"; //If queue is not specfied, engine crashes when posting events
}
EVTMGR.registerEvent("MyEvent");
EVTMGR.subscribe(Myobject, "MyEvent");
EVTMGR.subscribe(Myotherobject, "MyEvent");

EVTMGR.postEvent("MyEvent");

Now just make sure Myobject and Myotherobject have the following methods :

function Myobject::onMyEvent(%this, %data)
{
%this.somevar = "somestring";
}

In theory, Myotherobject shouldn't be triggered.
Change the function to

function Myobject::onMyEvent(%this, %data)
{
%this.somevar = "somestring";
return true;
}

And everything should work.

[capnlove]

Ran into another instance of this.
An event is posted by an Event Manager to a single subscriber, let`s call it PlayerObject.

This subscriber(PlayerObject) has a behavior which handles the event (the object itself has no handler defined within its bare namespace).
The behavior handling the event then calls a method on its owner (PlayerObject) and the method is actually defined in another behavior installed on the same PlayerObject.

This second behavior then calls a method on a bunch of other behaviors (all also installed on PlayerObject). Let's call these the children behaviors.

If I add an echo("Something random"); command in each of the children behaviors` methods, everything works fine.

If I don't, all children behaviors will execute their method properly, one after the other, as expected. However, when the last of the children behaviors exits the function, all children methods except that last one are called again but without passing any parameters.

Probably something no one ever encounters but....well, I did so it probably points to something deep-rooted in the namespace/behavior/component code.

I`m writing this down to also help visualize the issue.

[capnlove]

For those following at home (lol) Behavior Connections are THE way to circumvent this self-referential namespace mess.

[greenfire27]

I'm actually hanging on every word. Can you describe for the class Behavior Connections?

[capnlove]

Sure. I`ve made the modifications to my soon-to-be-shared-on-github project and it works perfectly.

The start is still the same

An event is posted by an Event Manager to a single subscriber, let`s call it PlayerObject.

This subscriber(PlayerObject) has a behavior which handles the event (the object itself has no handler defined within its bare namespace).

This behavior now has an empty method called dispatch
During PlayerObject`s creation, I create an instance of this Behavior, followed by all other behaviors I will need on this object.

Each behavior which needs to be notified has its template created as such :

if(!isObject(PlayerWalk_BEH))
{
    new BehaviorTemplate(PlayerWalk_BEH);
    PlayerWalk_BEH.addBehaviorInput(Action, "Input Action", "Input Action");
}

An Action method, where all the magic takes place, is defined on these behaviors.

Then I connect the initial behavior's output to all the behaviors' inputs as such :
%this.connect(%sourcebehavior, %targetbehavior, dispatch, Action);

Then, whenever my InputMatrix dispatches an event to %sourcebehavior, the method handling the event calls

%this.Owner.raise(%this, dispatch);

And all behaviors connected to that output have their Action method executed.

The wiki explains it very well https://github.com/GarageGames/Torque2D/wiki/Behavior-Connections but I had never used them before.

Only drawback is that you cannot (as far as I know) pass arguments to/from the connected functions.

[greenfire27]

Returning to the original problem, if you look at the notes for the function onMessageReceived in eventManager.cc, the code actually take the return values of the callbacks and uses them to decide if it should keep making the callbacks. Basically, a return value of true means that it will stop doing the callbacks (not sure if this is a useful feature or not). Most of the callbacks that the engine does, like onAdd() when you create an object, do not expect a return value. However, these particular callbacks expect a bool returned and are basically trying to make your last line in the callback into something it can use. In the event that it get's a string that doesn't equal "false" it uses it as true stops passing the callback to more listeners.

To be honest, unless we decide to make it work differently, the best thing to do is simply always return false in your callbacks.

Now, to keep things interesting, I found a better EventManager bug! It seems if you remove a listener from an event during that listener's callback the engine crashes.

PlayerObject::onJump(%this, %data)
{
%myEventManager.remove(%this, "Jump"); //crash!
}

My guess is that we're changing the length of the iterator in the middle of using it.

[capnlove]

Good find!

I am not suggesting we change it, I simply tried to provide as much info as I could.
Now that we know what's going on I'm thinking it should at least be documented in the wiki.
Maybe add some safeguards, I dunno.

The event manager will also crash if you do not specify a queue name upon creation as shown below.

new EventManager(){
   queue = "InputManagerQueue";
};

[greenfire27]

Yes, the EventManager obviously has rough edges that need to be cleaned up or at the very least documented. Unfortunately, as far as I know, we don't have an Event Manager page yet in the wiki, which might explain why nobody really uses it and reports these issues with it.

Is it possible to create multiple queues within the same EventManager? Is there really a point to naming a queue? It doesn't really seem like the name is used anywhere after creation.

I would also love to see more useful script functions like a removeAll, or removeAllForSubscriber. Joining could be similar: subscribeToAll(%subscriber) or subscribeToQueue(%subscriber) if we had multiple queues which could be used to group events.

[greenfire27]

Pull Request #376 is my attempt to fix all the problems around the event manager. I wrote a wiki page to describe how it works. Basically it allows any object to listen to any other object. Any object can now post events that will go out to all of its listeners. No need to set up a special object, no messaging queue, no registering events, and no returning false just to keep things running. I'll give the community time to play with this new event system, but at some point I hope to remove the old EventManager entirely. Its services are no longer required.