ratalaika / angel-engine

Automatically exported from code.google.com/p/angel-engine
0 stars 0 forks source link

SwitchBoard requires MessageListener class? #75

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
This isn't so much an issue as a flaw; the switchboard works fine as designed, 
but the design is contrary to stated goals (ease for beginners).

Because switchboard requires a MessageListener class, in order to receive 
several different message types, one is forced to either use several auxillary 
MessageListener's (breaking easy of use or encapsulation) or to do expensive 
comparison operations and switches (also breaking ease of use).

Why can't I subscribe a member or a function or anything besides a single base 
case class to switchboard? Using boost::function would immedietely fix this 
issue, and provide an enormous boost to ease of use, by allowing each message 
to be routed to a specific function or member or function object or whatever 
that is designed to accept it.

As example code, I provide...the default Angel2D examples. SuperDude has to 
catch four messages, two of which are merely hooks for non-returning, 
non-argument-passed functions: "Jump" and "PowerUpDone".

With the ability to register a member function to the switchboard, Jump would 
require zero additional handling, and the refactored code would be both more 
effecient and drastically easier to read: a message calls jump, jump checks if 
it can jump, and then jump jumps. None of this ineffecient and frankly hard to 
read if/else checks.

This is in Angel2D 3.1, and it is one of the most glaring of many small flaws 
I've found, but the only one that drastically deviates from the goals of the 
library.

As I said, a much simpler, easier to understand, and frankly more extendable 
solution would be to simply use boost::function.

By using boost::function (It would look like SwitchBoard would accept 
boost::function<Message*>) you'd allow every type of "function like" to be 
called (functors, members, regular functions, lambdas even), and the syntax is 
standard C++ syntax. 

Example: 
http://www.boost.org/doc/libs/1_53_0/doc/html/function/tutorial.html#idp58406736

There's not a whole lot easier than that, but once you've included boost as a 
dependency, now you can add boost::bind and boost::lambda tutorials and 
drastically increase the power of what can be quickly and easily done with 
Angel2D. But even if you don't decide to do that, you could add an overload to 
SubscribeTo that takes a boost::function, with the original SubscribeTo just 
taking the member of the MessageListener. This tiny change opens the door for 
messages being more effecient and more powerful and easier to use (without 
breaking any source compatibility).

Original issue reported on code.google.com by LoveOver...@gmail.com on 16 Mar 2013 at 7:31

GoogleCodeExporter commented 9 years ago
What you're bringing up is the balance between ease of understanding and ease 
of use. I'd argue that you can more readily understand what's happening in the 
existing message handling code because it's completely straightforward, if 
slightly verbose. 

I'm slightly hesitant to include boost as a dependency since I've had some 
pretty miserable experiences with it in the past. That said, if it can be done 
in an isolated and modular way to just provide the overload you mention, it 
might be worth it. Would you be able to put together a patch showing exactly 
what you're talking about? 

Original comment by lieseg...@gmail.com on 16 Mar 2013 at 8:42

GoogleCodeExporter commented 9 years ago
Here is my patch; it's a helper class that uses boost::function and boost::bind 
to allow redirecting messages to specific functions, functors, lambdas, and 
members without any changes to the code base.

Usage is simple: for each message redirection, a MessageTrampoline is made, 
assigned a function to call, and used anywhere a MessageListener is allowed.

Example:
theSwitchboard.SubscribeTo(_shootHook.Hook(this,&PlayerShip::Shoot), "Shoot");

Allows for the switchboard to directly redirect the message "shoot" to the 
shoot member in PlayerShip of "this" instance. 

I'd argue this is much easier to understand at a glance, as there is no "spooky 
action at a distance"; the message goes directly to the place it should go, 
with no intermediate and possibly error prone string checks. It's also just 
much easier to use.

This design is different then my original design, but serves the same purpose, 
requires no core changes, and is a little easier to read. 

Possible improvements:
1. Current trampoline requires called functions to accept Message*; the point 
of the trampoline is to reduce the need for message string operations, so I 
feel an optimization for the special case where the message is utterly unneeded 
would be appropriate (but would reduce code readability OR maintainability).

2. Creation of a MessageListenerManager that maintains a collection of 
trampolines and allows for quick creation of hooks, to replace MessageListener 
in classes which derive from MessageListener currently, which derives from 
MessageListener and provides the same interface. This would allow for the 
current model of deriving from MessageListener and passing "this" to 
theSwitchboard to continue to work, while allowing a much easier syntax for the 
extremely common case where a message ought to be redirected.

Possible example if Actor derived from MessageListenerManager:

theSwitchboard.SubscribeTo(this->Hook(this,&PlayerShip::Shoot), "Shoot");
theSwitchboard.SubscribeTo(this->Hook(this,&PlayerShip::Move), "Move");
etc

with the trampolines created automatically, and destroyed when the object 
returns (as is the case now, and the case with data members of 
MessageTrampoline, ensuring safety)

3. It'd be really nice if MessageListener's had a name. This is unrelated to 
the rest of this, but messages have names, actors have names, all sorts of 
things have names, but messages are sent with a name and a listener, and 
there's no real way to log that listener because listeners do not have a name.

Also, it's a little strange that MessageListener is who sends messages, not 
MessageSender or a related class. If MessageListener is also a Sender, it only 
makes sense that there is some "pretty" way to identify them besides a raw 
pointer.

If you need an example test case to demonstrate how MessageTrampoline works, I 
will be happy to send you a small example.

Thanks!

Original comment by LoveOver...@gmail.com on 17 Mar 2013 at 3:46

Attachments: