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

Received messages only contain the content #230

Open
jwaa opened this issue Oct 14, 2020 · 6 comments
Open

Received messages only contain the content #230

jwaa opened this issue Oct 14, 2020 · 6 comments
Labels
bug Something isn't working core Work related to the core functionality

Comments

@jwaa
Copy link
Member

jwaa commented Oct 14, 2020

Describe the bug
Within an AgentBrain the self.received_messages only contains the message content. This prevents the agent from knowing who send the message and to whom else it was send.

To Reproduce
Send a message from one agent to another. The receiving agent's self.received_messages is a list of the message content it received. See also this line in the AgentBrain.py

Expected behavior
To have the self.received_messages contain a list of Message objects (or a child type of it).

Screenshots
N/A

Stacktrace
N/A

Additional context
Bug found in the DASH project.

@jwaa jwaa added the bug Something isn't working label Oct 14, 2020
@Wouter1
Copy link

Wouter1 commented Jan 14, 2021

Yes one more vote for fixing this.
It's important to know especially who sent the message, for instance if you have more trust in some messages than in others

@jwaa
Copy link
Member Author

jwaa commented Jan 14, 2021

@thaije To resolve this we can fill the send_messages with the actual Message object instead of its content on this line.

However, the Message class has public attributes exposed that allow MATRX users to alter the sender and recipient of the actual Message instance. So either we should hide them (e.g., setting them as a Property) or add a copy of the instance to the list instead of a reference. Or we can ofcourse just change the line and trust users not to do this.

What do you think?

@Wouter1
Copy link

Wouter1 commented Jan 14, 2021

Yes it's only logical to include the full Message object.

Message objects should be immutable.

@Wouter1
Copy link

Wouter1 commented Jan 14, 2021

I vote against "trust the user". It's not about trusting the user, but to avoid him making mistakes. imho good code by design prevents users making mistakes.

@thaije
Copy link
Collaborator

thaije commented Jan 14, 2021

@Wouter1 thanks for notifying on the issue. It was already known that the current message system is not very intuitive (see #204), however this will put it higher on the priority list.

Issue #112 explains nicely why the message system is as it is now: we started off with a closed message system where users could only provide message content, but later we moved to a more open message system where users could extend the Message object to contain specific properties or extend the class itself. However it is now a bit of a mishmash between the two systems it seems.

I would also vote for messages containing the entire Message object with 3 required properties (content, from_id, to_id) that are immutable as Wouter suggests, and the ability for the user to add properties such as message.read, to put messages on read after they have been processed by the agent. Doing so also still allows users to use custom classes that extend the Message object with a number of custom properties by default that suit their needs.

@jwaa
Copy link
Member Author

jwaa commented Jan 14, 2021

@Wouter1 @thaije Thanks for the input. I added this issue to the list in #112. I keep this issue open as it might receive priority over the redesign of the message system.

@jwaa jwaa added the core Work related to the core functionality label Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core Work related to the core functionality
Projects
None yet
Development

No branches or pull requests

3 participants