Review and Document Listener Behaviour
aidanPB opened this issue · 4 comments
The triggering of listener propqueues is currently a bit of a mess. The low-level call that actually puts a listen event on the timequeue is listenqueue
in timequeue.c. That's used in the various notify functions in interface.c, which are in turn used all over the place. There are a few issues with them:
notify_listeners
andnotify_from_echo
are simultaneously too similar (going by doc comment and apparent intent, the latter could be a thin wrapper around the former) and bizarrely inconsistent with each other (theif
tests around the vehicle-handling section are different).
That's actually the only major one. Certain other details (especially in MUF prim implementations) were initially surprising, but actually make sense on reflection. (The reason that the only MUF prims to respect tp_allow_listen_env
are OTELL
and NOTIFY_EXCLUDE
is that anything else where environment listeners make sense is an in-server define.)
A related problem is that listeners have poor player-facing documentation. The MUF manual gives the impression that they only trigger on MUF-generated messages, and fails to describe the initial state of a listener program. The word "listen" appears exactly once in the entirety of the help file, in the entry for the @sweep
command. I didn't even know the ~listen
and ~olisten
propqueues existed until I went grovelling through source for issue #622 , because they're only mentioned in mpi-intro. That's also the only place players can learn about the way listener props can be conditional.
Per my commentary on notify_listeners, I am going to have notify_listeners confirm to notify_from_echo's vehicle logic because that seems to be correct. It WILL result in what could be perceived as new behavior. @wyld-sw when doing release notes, consider this:
This will fix MUF notifications to vehicles so they no longer (wrongly) depend on zombies being enabled (tp_allow_zombies) to receive MUF messages. This puts it in line with vehicles recceiving non-MUF based messages which already correctly does not hinge on the zombie flag.
So the logic turns out to be as follows to see if a message receives a vehicle's prefix:
if ((Typeof(obj) == TYPE_THING) && /* obj is a thing */
(FLAGS(obj) & VEHICLE) && /* And a vehilcle */
/* And not DARK, unless WIZARD owned */
(!(FLAGS(obj) & DARK) || Wizard(OWNER(obj))) &&
/* isprivate toggles if we're going to show the prefix or not */
(!isprivate) &&
/* The person who did it is in the same ocation as the thing */
(LOCATION(who) == ref) &&
/* And the owner is a wizard, or the thing is located in NOTHING,
* or the location of thing is not a room, or the location of the
* thing is not a vehicle.
*/
(Wizard(OWNER(obj)) || ref == NOTHING || Typeof(ref) != TYPE_ROOM ||
(!(FLAGS(ref) & VEHICLE)))) {
I'm really not sure why its this complicated and of course nothing explains why. I guess I should keep it that way. And as I'm working on this, I'm thinking maybe I should condense the logic because now this jenky stuff is repeated in two places which is nasty. I'm going to go ahead and do that, and so if we choose to simplify this, perhaps we can just simplify it in one place :)
Also perhaps notify_from_echo is sufficiently duplicate to just get rid if it anyway, I'll investigate that too. Hmm!
notify_from_echo is actually a complete subset of notify_listeners, with the exception that notify_listeners doesn't return anything but notify_from_echo does. I'm going to change notify_listeners to return the same as notify_from_echo does and eliminate notify_from_echo.
Re. documentation. 'man listeners' is actually on par with the other propqueue docs (such as 'man arrive' and 'man connect').
That said, the documentation isn't great. I think what I would personally like to see is a documentation of what a propqueue actually is (because nothing, as far as I know, defines it unless you read the source code) and how they each work. Perhaps a 'man propqueues' doc page that lists every one and provides additional notes, and then reference to man propqueues in the existing man listeners / man arrive / man connect pages.
I'll see if I can get that together in a reasonable fashion as part of this issue.