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

client.rs: remove items from 'items' variable when receiving remove e… #17

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

Levizor
Copy link
Contributor

@Levizor Levizor commented Feb 5, 2025

Client contains items field which stores all the tray items in a HashMap.
Upon receiving a new item client adds it to the mentioned map, so I assume that this map should be synchronized with the real state of the tray. However, when we receive a remove update, the item isn't cleaned from the map, which I find strange. I also don't see updates supplied to this map, so maybe it should be as well implemented.

So my suggestions is to remove items when we receive respective update. If you open and close an application that adds items to a tray now, it will be duplicated over and over, which is clearly not intended.

This problem was mentioned in this issue.

@JakeStanger
Copy link
Owner

Thanks for this. That'd certainly explain it.

Yes, that items map is supposed to be kept up to date to reflect the current state, as its how the current state is retrieved from an existing client (ie to be able to rebuild on reload).

You might find that updates are okay, as it's a map of HashMaps so updating the inner map will already update the outer state, unless I missed that too?

@Levizor
Copy link
Contributor Author

Levizor commented Feb 5, 2025

No, I guess you're right about updates. I missed that part. I'll remove the comment and rebase.

@Levizor
Copy link
Contributor Author

Levizor commented Feb 5, 2025

Sorry about my formatter, should I make function calls in one line?

@JakeStanger
Copy link
Owner

Sorry about my formatter, should I make function calls in one line?

It's using the default cargo fmt settings so whatever that does :P

@JakeStanger JakeStanger merged commit a3d8421 into JakeStanger:master Feb 5, 2025
4 checks passed
@JakeStanger JakeStanger mentioned this pull request Feb 5, 2025
@Levizor Levizor deleted the items-remove-fix branch February 9, 2025 08:40
@Levizor Levizor mentioned this pull request Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants