Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Rework the message path concept, decoupling from ANSI #3354

Open
Griatch opened this issue Dec 3, 2023 · 8 comments
Open
Labels
discussion A discussion or a request for one. feature-request A suggestion for new functionality.

Comments

@Griatch
Copy link
Member

Griatch commented Dec 3, 2023

Is your feature request related to a problem? Please describe.

Today, while Evennia's message path is generic, it's heavily tied to ANSI markup: Text strings are handled with ANSI codes, and conversion to e.g. HTML also go via ANSI-to-HTML conversion. Furthermore, there is no generalized way to handle more context output types, like menus or tables/forms. It's all going to the Portal as ANSI-marked strings and sent over the wire that way, despite certain clients potentially being able to do more advanced things with them. An example is that the webclient could theoretically render an EvTable using HTML <table> tags.

Describe the solution you'd like

We want to decouple Evennia's string handling from ANSI. ANSI is a telnet-specific standard that is irrelevant for other client types. We want to keep the Server as protocol-agnostic as possible. After discussion with @volundmush, this is the suggestion:

  • Use Evennia's internal markup (|r markup) for as long as possible. @InspectorCaracal's EvString PR would be required here, to move away from ANSI-dependence.
  • Use some other additional library (Such as rich) for more expressive markup.
  • When sending data from the Server to the Portal, data elements are converted into Sendables. A Sendable is a container that can contain any pickle-able python data. A Sendable could also wrap multiple entities, if they are intended to be rendered together (such as a set of tables going to a specific webclient pane). Finally, the Sendable can and will contain metadata related to the send, such as command-id.
  • The Sendable (or a list of them) are (depending on MULTISESSION_MODE) routed to different sessions, pickled (they must always be possible to pickle) and sent to the Portal. On the Portal side, they are in turn routed to all client protocols that should be receiving this data.
  • The client protocols contain customizable/extendable handlers for processing different Sendables in to the right form for that protocol. For example the telnet protocol would undertstand to convert table Sendables to MSDP TABLE, and/or just convert to ANSI concersion. Meanwhile the webclient protocol would understand other Sendable primitives, converting them to HTML elements as needed.
  • The protocol then sends this 'rendered' data to the client.

Discussion

  • This means changing the outgoing message path; including how msg() works (or introduce some lower-level send mechanism). This is a major backwards-incompatible change.
  • What about the incoming data? Should we allow custom machinery for converting advanced client concepts into Evennia-primitives as well? Initially, keeping the current (funcname, (args), {kwargs}) may be enough.
  • We have friar's PR for moving rendering to the web client. It's unclear if this would be possible or if it would be better to keep processing on the Server-side, where it can easily be updated/replaced.
  • Leaving rendering replace-code on the Portal (rather than having it in each Sendable) means updating of this machinery would require updating the Portal.

Additional context

@Griatch Griatch added feature-request A suggestion for new functionality. discussion A discussion or a request for one. labels Dec 3, 2023
@InspectorCaracal
Copy link
Contributor

InspectorCaracal commented Dec 3, 2023

At the moment, my EvString PR includes calling rendering methods on the outgoing object (regardless of what the object is) rather than using a hardcoded renderer module. Since that looks relevant to the goal here, I think it'd be useful for me to get some kind of an idea of what kind of rendering methods are ultimately expected so I can avoid working cross-purposes.

I have .clean() for markup-stripped plain strings, .raw() for the original strings with any markup, .ansi() for rendering with ANSI codes, and .html() for rendering to HTML tags. (The idea being that these can be overloaded or customized in custom classes or subclassed Evennia objects for the built-in protocols, or additional methods added for custom protocols.)

@volundmush
Copy link
Contributor

The beauty of my proposal is that, ultimately, the PortalSession gets to determine what to DO with these Sendable objects it receives. They could be ANYTHING, as long as it's pickled. That means Evennia can implement its own basic standard for how to handle rendering them/dealing with them, but developers can also add their own if they're brave enough to subclass the PortalSessions.

It is, in a way, having a standard by not having a standard.

I mean, yes, we need to come up with a standard Evennia scheme, of course.

@volundmush
Copy link
Contributor

As far as the API goes on the Sendables, I would prefer something more like .render_as_html(session, metadata) than just .html(session, metadata) because otherwise there's a decent chance of running into a conflict if you're like me and trying to use things like RichRenderables as Sendables as an alternative/addition to Evennia's.

@volundmush
Copy link
Contributor

volundmush commented Dec 3, 2023

So, I got a PR in which seems to be a solid baseline to build upon.

This means changing the outgoing message path; including how msg() works (or introduce some lower-level send mechanism). This is a major backwards-incompatible change.

My PR currently maintains backwards compatibility using the 'oob' render_type.

What about the incoming data? Should we allow custom machinery for converting advanced client concepts into Evennia-primitives as well? Initially, keeping the current (funcname, (args), {kwargs}) may be enough.

Doing the same in reverse seems... sort of doable, but I'm unsure about the benefits. My crusade is largely around breaking free of the limitations on how output formatting is done, and ensuring that input and output can be related. What sort of elaborate input could we be using that can't be adequately handled with the inputfunc data? a command, list, and dictionary is a pretty powerful data structure. I'm drawing a blank here.

We have friar's PR for moving rendering to the web client. It's unclear if this would be possible or if it would be better to keep processing on the Server-side, where it can easily be updated/replaced.

If it helps, my friend has a JavaScript library which is capable of turning ANSI text into HTML spans with a much higher accuracy than Evennia's current approach. I've converted it to Python and added it to this PR.

Leaving rendering replace-code on the Portal (rather than having it in each Sendable) means updating of this machinery would require updating the Portal.

Well, as I understand it, the Portal will import object classes as necessary as it receives objects to unpickle... but since it won't reload modules on each attempt, the Portal will need to be restarted if new classes of Sendables are added.

@owllex
Copy link
Contributor

owllex commented Dec 6, 2023

I think the idea of packaging outbound data into a protocol-agnostic container and keeping it in that form for as long as possible is a good one.

However, I would argue that Sendables should be as simple as possible and should not attempt to interact with the DB once they reach the Portal. Even doing DB reads from the portal side can be risky at best, and cause a performance headache at worst, especially if said reads are happening once per session. And Sendables absolutely should never write to the DB since that will cause a desync.

I think we should adopt an invariant that everything a sendable needs to render itself should already be packaged in the sendable when it goes to the portal. In this way, Sendables should be as simple as possible with no need to ever fetch additional data from anywhere. Really just a bundle of data with logic to transform it directly into outbound text, markup, or whatever without any side effects on the DB or other kind of state. Using third party libraries will likely be necessary to do things like render HTML tables, but I don't think we should encourage Sendables doing anything more complicated than that. (Obviously anyone can do what they want in their own game, so this proposed invariant in for core code and documentation)

@volundmush
Copy link
Contributor

Absolutely agreed, @owllex . That's kinda necessary given that the Portal avoids maintaining a connection to the database. Having to Pickle the objects also imposes a few other sanity-checks on the system:

Pickle works by recursively scanning through the object and its instance variables, saving the class path etc... it can then instantiate a clone when unpickling. So, many things cannot be pickled. Recursive data structures are the devil, as are data structures which must ask for additional information from other things or query the database. The IDEAL Sendable would definitely be one where it's __dict__ contains only JSON'ish primitive data structures and references to other objects which meet the same standard.

@owllex
Copy link
Contributor

owllex commented Dec 6, 2023

I don't think "can it be pickled" should be equivalent to "should it be in a sendable", just to be clear. Yes pickle does enforce some restrictions but I think that's tangential here. I'd push for a strong stance on just passing primitives through (and we don't need JSON since the data should be pure python primitives). This keeps things very simple, avoids some of the quirks of pickling. I also think we should use __slots__ for efficiency rather than __dict__, but that's a small implementation detail that can be sorted out later.

@volundmush
Copy link
Contributor

One of my hopes is that this system not overly restrict what CAN be done with a Sendables. One of the hopes here is the inversion of control - instead of having the Server tightly regulating what kinds of things can be sent back to a Client, it is now the job of the Portal to talk to Sendables - a task which can be overloaded and altered by developers.

There are certain technical limitations involved, and there are things which are good ideas and bad ideas that need to be made clear for anyone using the system and making custom Sendables, but I'd veer away from making the system itself prevent things from happening.

We might be saying the same here; I dunno.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion A discussion or a request for one. feature-request A suggestion for new functionality.
Projects
None yet
Development

No branches or pull requests

4 participants