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

Fix markers ui bug #5671

Merged
merged 4 commits into from
Feb 24, 2025
Merged

Fix markers ui bug #5671

merged 4 commits into from
Feb 24, 2025

Conversation

skier233
Copy link
Contributor

@skier233 skier233 commented Feb 20, 2025

Fixes the bug found here: #5633 (comment)

@WithoutPants
Copy link
Collaborator

@skier233 I moved loadMarkers into a separate callback function, and removed the async keywords from computeBaseHue and all its client functions. It looks to have maintained the fix, and I couldn't find a reason for having the async keyword in that function. If you can take a quick look and verify, I'll get this merged.

@WithoutPants WithoutPants added the bug Something isn't working label Feb 21, 2025
@WithoutPants WithoutPants added this to the Version 0.28.0 milestone Feb 21, 2025
@skier233
Copy link
Contributor Author

@skier233 I moved loadMarkers into a separate callback function, and removed the async keywords from computeBaseHue and all its client functions. It looks to have maintained the fix, and I couldn't find a reason for having the async keyword in that function. If you can take a quick look and verify, I'll get this merged.

Looks good. Originally the async was needed when it was using the other hashing lib and the hashing call in that lib was async but I guess that's no longer needed after that lib got swapped out.

@WithoutPants WithoutPants merged commit d915787 into stashapp:develop Feb 24, 2025
2 checks passed
@SyZeeGee
Copy link

I just had a chance to test the new dev build and although it seems better (there is less phantom markers) clicking it fast still seems to induce the bug. Figure since your mind space is still here, I'd let you know. Here is a theory though possibly specific to my use case: is it possible the signal being listened to clear markers isn't being received if the file isn't accessible ("No such file or directory" due to being on an external drive not plugged in?) I didn't have time to fully test this theory but it's possible.

Note I appreciate any work at all on this really, so thanks really. It's overall a really cool marker/video feature (for reasons past the main utility of this software. Well done

On that note, related here: @WithoutPants : when an external hard drive is not present (and the file is not playable), the way the software works currently is that it repeatedly dumps out errors (The "No such file or directory" found one, until the page or video is left.) It just doesn't seem to like the correct implementation (nor intended) here and wanted to let you know of it as well as it might be linked to the above issue. I just confirmed the CPU usage of the browser is going through the roof when it happens so it's not ideal for that reason alone. Let me know if you'd like me to open up a dedicated issue or if its known already (I didn't know what to search)

@WithoutPants
Copy link
Collaborator

On that note, related here: @WithoutPants : when an external hard drive is not present (and the file is not playable), the way the software works currently is that it repeatedly dumps out errors (The "No such file or directory" found one, until the page or video is left.) It just doesn't seem to like the correct implementation (nor intended) here and wanted to let you know of it as well as it might be linked to the above issue. I just confirmed the CPU usage of the browser is going through the roof when it happens so it's not ideal for that reason alone. Let me know if you'd like me to open up a dedicated issue or if its known already (I didn't know what to search)

Please raise a separate issue for this. I'm not aware any existing issue that covers this.

@SyZeeGee
Copy link

Thanks for opening those issues. I just posted the one about the looping file reads 👍

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

Successfully merging this pull request may close these issues.

3 participants