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

[ExternalPluginNotifier] Replacements looks to be required when it shouldn't be and getting client thread error #675

Closed
2 tasks done
adamk33n3r opened this issue Mar 6, 2025 · 5 comments · Fixed by #677 or #678
Labels
bug Something isn't working Notifier: External

Comments

@adamk33n3r
Copy link

Checklist

Describe your issue

When testing v1.11.2 I'm getting an exception that replacements can't be null even though it's not supposed to be required.
Image

Image

Then after adding an empty map for replacements just for testing, I get this error. Looks like it's trying to get a varbit from a non-client thread?

Image

Screenshots

No response

Runelite Logs

client.log

Runelite Version

1.11.3

@pajlada pajlada added bug Something isn't working and removed issue-report labels Mar 6, 2025
@pajlada
Copy link
Member

pajlada commented Mar 6, 2025

#676 will fix the replacements issue

For the client thread issue, we've previously assumed enrichBody will either be called from the client thread, or that the account type is already filled in.

If that's the only place where we do client-thread-only access to data, we might just want to cache the account type ourselves & allow for any thread to access that data.

Any thoughts on preferred approach for that @iProdigy ?

@iProdigy
Copy link
Member

iProdigy commented Mar 6, 2025

there's more data being queried within enrichBody that should be done from the client thread; will make a PR to ensure it's not called off-thread

@adamk33n3r
Copy link
Author

there's more data being queried within enrichBody that should be done from the client thread; will make a PR to ensure it's not called off-thread

Just for transparency, my two tests were from the swing thread, and from my own thread (triggered by a game message event) which I guess makes sense. All of my notifications get put onto a thread of their own so they don't cause any game lag (sounds and tts were doing that).

@pajlada
Copy link
Member

pajlada commented Mar 6, 2025

there's more data being queried within enrichBody that should be done from the client thread; will make a PR to ensure it's not called off-thread

Just for transparency, my two tests were from the swing thread, and from my own thread (triggered by a game message event) which I guess makes sense. All of my notifications get put onto a thread of their own so they don't cause any game lag (sounds and tts were doing that).

That's perfectly fine, we'll invoke the relevant part in the client thread now https://github.com/pajlads/DinkPlugin/pull/677/files

@adamk33n3r
Copy link
Author

Can confirm it works now

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Notifier: External
Projects
None yet
3 participants