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

Can we avoid storing data in the chat message content? #107

Open
TheComamba opened this issue Jul 28, 2024 · 3 comments · Fixed by #110 or #112
Open

Can we avoid storing data in the chat message content? #107

TheComamba opened this issue Jul 28, 2024 · 3 comments · Fixed by #110 or #112
Labels
quality management Documentation, testing, etc.

Comments

@TheComamba
Copy link
Owner

We are currently placing some chat data directly inside the content, and replacing it at a later point.
This is hacky, and has some side effects. For example, any module working with the chatMessage hook (Talking Actors, for example) sees and processes our flag and json before we can replace it.

The ui.chat.processMessage function returns the message object, but I think only after it has been posted. Can we modify that object afterwards?

I am also not sure if some async functions are called in a fire-and-forget manner, meaning we could be dealing with concurrency issues if we chose this path.

@TheComamba TheComamba added the question Further information is requested label Jul 28, 2024
@TheComamba TheComamba added quality management Documentation, testing, etc. and removed question Further information is requested labels Nov 24, 2024
@cyelis1224
Copy link
Collaborator

cyelis1224 commented Nov 29, 2024

This needs to be re-opened, I noticed that its not my biography changes that seem to have broken /whisper and /talk, but this PR that replaced how we were storing the data from the message content to flags. since we are storying that data as a flag now it is not properly getting the prefix and inserting it into the message content. I restored the branch from where you previously fixed the /talk prefix to see if i can figure it out.

Strangely, I restored your most recent commit before you merged my changes and it seems like it works if you set the /talk flag in the settings but not on the actor sheet itself?

@cyelis1224 cyelis1224 reopened this Nov 29, 2024
@TheComamba
Copy link
Owner Author

Ok, no worries, the changes have not yet made it into a release. :)
And apparently we have a test gap then, if changes can break the behaviour and still pass all checks.
Thanks for the investigation.

@TheComamba
Copy link
Owner Author

Jah, ok.
In chat-message-response.js:59 we call:

ui.chat.processMessage(response, chatData)

The calee is found in foundry.js:89654 and has the signature:

async processMessage(message, {speaker}={})

I, with my limited JavaScript knowledge, read this as "Give me the 'speaker' field of ChatData, I do not give a damn about the other fields."
And indeed, some 5 lines later a const chatData is initialised, ignoring any input but the speaker.

I am once again sceptical if we can avoid this ruddy JSON parsing.

Now I remember why this topic frustrated me. 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality management Documentation, testing, etc.
Projects
None yet
2 participants