Rework Outgoing MessagePath #3356

volundmush · 2023-12-03T05:26:02Z

Brief overview of PR changes/additions

Added settings.SENDABLE_CLASS_MODULES for loading up (replaceable) Sendable classes from modules using callables_from_module. They're cached to evennia.SENDABLES (a dictionary)

Added evennia.server.sendables with some example Sendables.

Adds .send() method to Characters, Sessions, Accounts, Commands which handles the new outgoing path. This method takes a list of Sendable objects and a metadata dict to associate with the Sendables. Everything involved must be pickleable for this to work.

Altered the .msg() methods on Character, Sessions, Accounts, and Commands to convert their outgoing messages into a list of Sendables (the EvString placeholder and OOBFunc Sendable). These two ultimately are rendered exactly as existing Evennia messages are, for backwards compatibility.

Added a feature to AMP so it can actually send the Sendables. This process is actually a bit more efficient than the old way, since it can multi-cast the message to multiple sessids with a single AMP transmission.

Created a new PortalSession base class and made it customizable via class_from_module. It has methods to handle the Sendables.

Motivation for adding to Evennia

Well, just gotta read over issue #3354 to get the picture.

Other info (issues closed, discussion etc)

Well I haven't exactly rewritten all of the documents and some more documentation is probably needed.

Griatch

I must say that this looks quite good overall. I appreciate your work on this. I have made many comments below; apart from minor things, like naming conventions and a few questions, we need extensive unit tests for this:

Unit tests for the new default sendable logics on the server side, including serialization/deserialization tests.
Unit tests for portal, making sure sendables output expected results (mocking out any AMP/wire calls, enough to test what is generated)
Full unit test suite for the new ansi-to-html (ravensgleanin.py) module.
Also, legacy unit tests are not passing as it is; that needs fixing too; to make it clear how backwards-compatible this is with the legacy message path.
Performance. We are still serializing to tuples across the AMP connection, so that should be fine - but is there any impact to serializing to/from a Sendable on each side? Would be good to have some comparisons or discussions around this.

Also, as a note (mainly to myself); if this merges; it will mean rewriting all documentation for the message path and how to extend these parts of Evennia.

Note to @volundmush Please don't rebase your fixes as you make them, since that loses the history and it becomes very hard for me to see what changes. The PR can be rebased once it's ready to merge, but then it's a good idea to add github comments to the code to make things a little easier to review 🙏

Griatch · 2023-12-10T16:54:21Z

evennia/accounts/accounts.py

+ sendables, metadata = msg_to_sendables(**kwargs)
+ self.send(sendables, metadata=metadata, sessions=sessions)
+
+ def send(self, sendables: list["Any"], metadata: dict = None, **kwargs):


This needs a docstring on Google-style format, as for everywhere else in Evennia.

Griatch · 2023-12-10T17:08:41Z

evennia/server/amp_client.py

@@ -142,6 +142,14 @@ def data_to_portal(self, command, sessid, **kwargs):
 self.errback, command.key
 )

+ def data_multi_to_portal(self, command, sessions, data):


Looks like a docstring is needed here too, it's not clear how this differs from the data_to_portal command.

Griatch · 2023-12-10T17:39:51Z

evennia/server/portal/portalsession.py

+ render_types = ("oob", "ansi", "html", "json")
+
+ @lazy_property
+ def ev_options(self):


Rather than ev_options, maybe session_options? Would also be good to have the docstring be a bit more informative about what this is.

Griatch · 2023-12-10T17:40:08Z

evennia/server/portal/portalsession.py

+ load_kwargs={"category": "option"},
+ )
+
+ def sendables_out(self, sendables: list["Any"], metadata: dict):


Docstring. Not sure about the name of this. I presume this is akin to data_out?

Griatch · 2023-12-10T17:41:26Z

evennia/server/portal/portalsession.py

+ """
+ pass
+
+ def filter_sendables(self, sendables: list["Any"]) -> dict[str, list["Any"]]:


Griatch · 2023-12-10T18:17:21Z

evennia/utils/ravensgleaning.py

+ print(f"command: {command}")
+ if command:
+ parts = command.split(";")
+ print(f"parts: {parts}")


Another print

Griatch · 2023-12-10T18:17:48Z

evennia/utils/ravensgleaning.py

+ return self.html_for_state()
+
+
+class RavensGleaning:


Needs to be renamed to something more informative. Also needs a docstring.

Griatch · 2023-12-10T18:19:27Z

evennia/utils/ravensgleaning.py

+ if not state.is_reset() or not mush_log:
+ ret += "</span>"
+
+ return ret


This module requires extensive unit tests.

Griatch · 2023-12-10T18:21:16Z

evennia/utils/utils.py

@@ -3018,3 +3018,32 @@ def ip_from_request(request, exclude=None) -> str:

 logger.log_warn("ip_from_request: No valid IP address found in request. Using remote_addr.")
 return remote_addr
+
+
+def split_oob(data) -> ["any", dict]:


Docstrings needed for everything here.

Griatch · 2023-12-10T18:22:22Z

evennia/utils/utils.py

+
+
+def split_oob(data) -> ["any", dict]:
+ if isinstance(data, (tuple, list)) and len(data) == 2:


The utils/utils.py is long enough as it is. Stuff in utils/utils.py should normally be things that people may want to grab and use elsewhere in their game, while this seems pretty specific to the message path. Maybe it would be better to have these utility functions in the evennia/server/ folder, like in a utils.py module, or as part of the relevant sendables.py etc.

I'm skeptical if server/utils.py is going to be any more informative than utils/utils.py when you get down to it...

volundmush · 2023-12-16T23:58:22Z

Had to rebase due to merge conflicts.

volundmush · 2023-12-17T09:31:42Z

Geeze this has come a long way.

Griatch · 2023-12-19T22:27:14Z

Thanks for your work on this, @volundmush!
This will take a bit longer to review and and I think it needs to sit a bit in main before it goes into a release. So I'll review and hopefully merge this after I get Evennia 3.0 out.

volundmush · 2024-01-02T15:19:46Z

I am starting to have second thoughts about the specific implementation of this. We should pow-wow and go over the details because yikes this is not something I want to screw up.

Griatch · 2024-01-08T18:36:28Z

@volundmush Ok, I'm back now, so we can discuss. :)

Griatch · 2024-01-14T16:58:31Z

@volundmush I know you had doubts about this implementation, but I still think this Sendables concept is better than passing JSON around :/

Griatch · 2024-01-28T13:26:44Z

@volundmush So where are we on this one?

volundmush force-pushed the message_sendables branch 3 times, most recently from a5bc469 to 6be1c48 Compare December 3, 2023 06:14

volundmush mentioned this pull request Dec 5, 2023

Breakup DefaultObject.msg() and add an at_post_msg_receive() hook #3326

Closed

volundmush force-pushed the message_sendables branch from 6be1c48 to 8a95490 Compare December 5, 2023 21:10

Griatch requested changes Dec 10, 2023

View reviewed changes

volundmush force-pushed the message_sendables branch from a4d7c91 to 38ce1c9 Compare December 16, 2023 23:57

volundmush force-pushed the message_sendables branch 3 times, most recently from 9f0b859 to 78119df Compare December 17, 2023 08:38

volundmush requested a review from Griatch December 17, 2023 09:31

volundmush added 4 commits January 24, 2024 21:10

Implementing the new Sendables infrastructure.

8444fdf

Added Docstrings for new features.

f2f6b6a

Further improved Sendables API.

264927a

Fixing tests.

20f940f

volundmush force-pushed the message_sendables branch from 78119df to 20f940f Compare January 25, 2024 02:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework Outgoing MessagePath #3356

Rework Outgoing MessagePath #3356

volundmush commented Dec 3, 2023

Griatch left a comment •

edited

Griatch Dec 10, 2023

Griatch Dec 10, 2023

Griatch Dec 10, 2023

Griatch Dec 10, 2023

Griatch Dec 10, 2023

Griatch Dec 10, 2023

Griatch Dec 10, 2023

Griatch Dec 10, 2023

Griatch Dec 10, 2023

Griatch Dec 10, 2023

volundmush Dec 16, 2023

volundmush commented Dec 16, 2023

volundmush commented Dec 17, 2023

Griatch commented Dec 19, 2023

volundmush commented Jan 2, 2024

Griatch commented Jan 8, 2024

Griatch commented Jan 14, 2024

Griatch commented Jan 28, 2024



		def split_oob(data) -> ["any", dict]:
		if isinstance(data, (tuple, list)) and len(data) == 2:

Rework Outgoing MessagePath #3356

Are you sure you want to change the base?

Rework Outgoing MessagePath #3356

Conversation

volundmush commented Dec 3, 2023

Brief overview of PR changes/additions

Motivation for adding to Evennia

Other info (issues closed, discussion etc)

Griatch left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volundmush commented Dec 16, 2023

volundmush commented Dec 17, 2023

Griatch commented Dec 19, 2023

volundmush commented Jan 2, 2024

Griatch commented Jan 8, 2024

Griatch commented Jan 14, 2024

Griatch commented Jan 28, 2024

Griatch left a comment •

edited