fuzzball-muck / fuzzball

Ongoing development of the Fuzzball MUCK server software and associated functionality.
Other
47 stars 27 forks source link

Correct Location Reporting of messages in MUF for _listen (SF issue) #622

Closed tanabi closed 2 years ago

tanabi commented 3 years ago

Currently, if a person or object moves, generating osucc and odrop messages which are picked up by a global or environment listen, there is no way to correctly determine which room heard the osucc and which heard the odrop. We need a better way of determining locations during listen events in MUF. If I'm missing something here, someone please correct me.

aidanPB commented 2 years ago

Is there a reason this is labelled "bug" rather than "enhancement"? It reads like a request for new behaviour, rather than a security issue or something otherwise strictly "incorrect".

Something related that I would consider a bug, if exceedingly minor: the MUF manual describes how connect and disconnect propqueues set the system variables. (Relevant to this issue, loc @ is the location of the connecting/disconnecting player, regardless of what object the propqueue is on.) No such information is listed for listeners or for arrive and depart propqueues.

After a bit of grovelling through source code, it looks like loc @ for listen propqueues depends on how the message was generated: MUF NOTIFY and friends appear to set loc to the place the message appeared, but otherwise, it's set to the current location of the player that caused the message in the first place. Odrop messages are sent before the move actually takes place, and the built-in "has left" message is sent after.

Still, I'd argue that the real bug here is trying to use listeners to do the job of arrive and depart propqueues: it isn't documented in the manual, but loc seems to be correctly set to the place that was arrived at or departed from.

tanabi commented 2 years ago

It used to be Fuzzball had a bunch of old issues on SourceForge, so I brought them over en masse. I think I just picked the same issue type as was used on SourceForce without doing a judgement call -- when you're doing bulk, manual labor like that, you just kinda go with the flow :)

I don't know the initial inspiration for this issue (it wasn't me, I wasn't working on this project back in the SourceForge days) so it's hard to defend the rationale here. But, as someone who's written a lot of vehicle code and some really complex listeners in the past to handle stuff like recording roleplays and auto-posting to websites and the sort ... I would find every scrap of information I can gleen pretty useful.

However, if there's not a clean and logical way to present the message or to support this, I'd be fine with dropping it :) It sounded interesting to me but I agree its probably not necessary.

aidanPB commented 2 years ago

The more I dig through the code, the more I feel like listener (as distinct from other propqueue) handling needs a massive overhaul. It looks like the only MUF prims to obey tp_allow_listeners_env are OTELL and NOTIFY_EXCLUDE. Every other program-generated message just never leaves the room where it happened.

It actually looks like exit odrops are the odd messages out; for every other omessage (including odrops on things, programs, and rooms), the location of the triggering player happens to be the Right Thing. I think getting exit odrops to set loc to the destination room for listener programs would involve a fair amount of work for relatively little gain, and be in its own way inconsistent. It's also fairly easy to work around: either use an arrive propqueue, or place the listener prop directly on the accessible rooms (instead of an environment room) and use trig (or trigger @) instead of loc @ in the listener program.

tanabi commented 2 years ago

You're right about listen being kind of a mess. I thought about devil's advocating for odrop having loc be the destination room, because there are times where having individual listeners in each accessible room is not a good solution, and relying on a separate prop queue altogether requires a level of interprocess communication MUF may or may not be able to do based on the individual situation.

HOWEVER you are 100% right that modifying the loc behavior just for odrop is introducing inconsistent behavior, and to me, consistency is way more important than pandering to an edge case (albiet one I've flirted with myself). So I agree with your notes here, and we should fix the problems you found. If you don't mind putting your findings in a new issue, I'll close this one and we'll consider it something we're not going to do.

aidanPB commented 2 years ago

I don't see how using the same prop queue avoids IPC, given that every trigger is queued as its own process. Non-listen prop queues do have the very real limitation that they don't have access to messages related to their triggers, but I belatedly realised that while the odrop message is sent (and listen triggered) before the move takes place, the program doesn't get any chance to run until after the player has moved: for a listener triggered by an exit odrop, loc @ is where the player was when the message was sent, but me @ location is where the player is now (the destination).

tanabi commented 2 years ago

Re. IPC. If the goal is to do something with the odrop message that involves the source and destination rooms, you could handle that in the listen handler without having to hybridize with the arrive queue which would avoid IPC ... however the more I talk about this, the more edge-case-y and dumb it sounds, so I'm gunna drop it :)

To my mind, it sounds like you have solved the original issue brought up by this ticket by noting how loc @ vs. me @ location works; perhaps the original requestor didn't know about loc @ or perhaps the request came from a time before that call existed because some of those SF issues date back to the FB 5.x days

So this issue is now well and thoroughly closed. Though please still feel free to create a new ticket with the problems you found.