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 Internet Detector Background/Taskbar Issues #1221

Merged
merged 3 commits into from
Dec 17, 2024
Merged

Conversation

emtuls
Copy link
Member

@emtuls emtuls commented Dec 17, 2024

This fixes both #1216 and #1217

The issue with the wallpaper (#1216) was actually that internet_detector ended up in a broken state after the installer.vm performed a VM-Refresh-Desktop, which made internet_detector continue to run but unable to modify the background and also display an indicator in the taskbar. This has been fixed by having a detection for if explorer.exe is restarted and/or if the taskbar icon is removed, and if so, it attempts to restart itself or die gracefully so that the scheduled task can start it back up again properly.

The Taskbar issue (#1217) has been resolved by storing the old/default taskbar color scheme into a registry key that will be checked if it exists before setting a new default. It then reads this value for when it adjusts back to the state of when it does not detect internet.

I have also adjusted the package to re-enable the scheduled task and also made the time to kick off every 1 minute instead of 2 minutes.

@emtuls emtuls added 🐛 bug Something isn't working 🌀 FLARE-VM A package or feature to be used by FLARE-VM labels Dec 17, 2024
@emtuls emtuls self-assigned this Dec 17, 2024
@emtuls emtuls requested a review from Ana06 December 17, 2024 05:55
if not load_default_settings():
# If not found, get the current settings and save them as defaults
default_color_prevalence = get_current_color_prevalence()
default_color = get_current_taskbar_color()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should check before saving the color that it is not pink and use a default gray if it is. This way we ensure that there can never be an error, as these bug show how tricky implementing it correct is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the end, with this implementation we are setting a default (the one the user had before the tool started running), as the user can't change the color anymore. So I wonder, if it would be simpler and more reliable to just default to black and pink.

If we keep it this way, we should document somewhere (not necessary in this PR, maybe in the wiki) how the user could change the color. In the long term we could move this tool to its own project with a readme.

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested locally and it seems to work well 👍 I think we can merge it and continue discussing if we need further improvements afterwards. I expect our community to share feedback too. 😉

@Ana06 Ana06 merged commit 3f5fdf0 into main Dec 17, 2024
6 checks passed
@Ana06 Ana06 modified the milestone: FLARE-VM 2024 Q4-P1 Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🌀 FLARE-VM A package or feature to be used by FLARE-VM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants