search

NadyaNayme / VoiceOfSeren

Track the current and last Voice of Seren through crowdsourced data
MIT License
4 stars 1 forks source link

Draft: Alert on Favorite VOS #2

Closed FlooflesDev closed 2 months ago

FlooflesDev commented 2 months ago

I found your plugin very useful but was really missing an alert option. I took the liberty to code an initial implementation which works 'okay'. It can definitely still use some polish though.

Let me know what you think! Currently, the settings look like this:

vos_settings

The alert is currently a tooltip which disappears after 5 seconds:

vos_alert

What I still need to fix:

I primarily wanted to create this Draft pr to get your opinion :)

NadyaNayme commented 2 months ago

Very interesting behavior that & doesn't render within tooltips. I don't think Skillbert will fix that but I'm going to report it anyways because....that's just weird.

The app has all of 10-15 users so I'm not too fussed about the somewhat spammy behavior. This has been a commonly enough requested feature that I think people will appreciate having it.

There's a number of bugs in the code base regarding proper timing/debouncing of events. I tried to fix things a while ago and while I squashed many of them a few are still poking around and are probably really obvious logic errors. Fetching multiple times after a successful fetch for that hour is one such bug. There is another much more insidious bug that results in someone voting with Stale/Outdated data which causes the VoS to report incorrect clans for that hour if there aren't at least 2 other voters with the correct data. I only mention this because it is very unlikely I'll be fixing those bugs anytime this year so if you're interested in making VoS more reliable for yourself you have my blessing to change whatever you need to change to get it working more reliably.

NadyaNayme commented 2 months ago

Other than that - the code LGTM. No real comments on my part other than its better quality than anything else in the code base. :) Thanks for the PR!

FlooflesDev commented 2 months ago

Ah alright! I was intending to fix some more bugs before merging, but I understand that with the current amount of users you're not too fussed on things not being 100% reliable.

I pushed this rather late yesterday so I just wanted to get it out there and fix things later. I might submit another pr today if I manage to fix any more bugs :)