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] Power saver blocker disabling after library updates #3519

Merged
merged 5 commits into from
Aug 9, 2024

Conversation

Etaash-mathamsetty
Copy link
Member

Title, I think this is reason why the power saver blocker doesn't work

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@Etaash-mathamsetty Etaash-mathamsetty added the pr:ready-for-review Feature-complete, ready for the grind! :P label Feb 4, 2024
@arielj
Copy link
Collaborator

arielj commented Feb 4, 2024

I'm curious how is the lock even working when playing a game, because if we are playing with no other operation then pendingOps would be false and it would never lock the thing in the first place

I'm always bad with filtering and negative logic but I think we should remove the game.status !== 'playing' from the pendingOps filter maybe instead?

then playing would be considered as a pending op and it will not only not trigger the unlock but also trigger the lock

@Etaash-mathamsetty
Copy link
Member Author

Etaash-mathamsetty commented Feb 4, 2024

I'm curious how is the lock even working when playing a game, because if we are playing with no other operation then pendingOps would be false and it would never lock the thing in the first place

I'm always bad with filtering and negative logic but I think we should remove the game.status !== 'playing' from the pendingOps filter maybe instead?

then playing would be considered as a pending op and it will not only not trigger the unlock but also trigger the lock

yeah me as well, i think it almost instantly unlocks it again
your logic makes sense to me, I will modify this PR.

The original logic doesn't make sense either, it will prevent from sleeping when a game is simply not installed?

@flavioislima
Copy link
Member

flavioislima commented Feb 5, 2024

I think we broke the logic at some point a long time ago.

I believe that we used to have two logics:
For playing, we were blocking the screen to turn off and the machine to sleep as well.

For downloading, etc. We were blocking only the machine to sleep.

Looking at the code now in main.ts, I can see that we are only blocking the machine to suspend. So even if you are playing the screen will turn off and should not be like this.

The Electron docs says:

Note: prevent-display-sleep has higher precedence over prevent-app-suspension.
Only the highest precedence type takes effect. In other words, prevent-display-sleep always takes precedence over prevent-app-suspension.

For example, an API calling A requests for prevent-app-suspension, 
and another calling B requests for prevent-display-sleep. prevent-display-sleep will be used 
until B stops its request. After that, prevent-app-suspension is used.

so your logic looks good @Etaash-mathamsetty , but a proper fix would need to include the display-sleep as well.

So in the past we used to pass an argument to the 'lock' command. Could be either play or download, and then we treated the priority on the backend with the correct command.

At some point we removed it and kept only one, which won't work properly.

@Etaash-mathamsetty Etaash-mathamsetty added pr:wip WIP, don't merge. and removed pr:ready-for-review Feature-complete, ready for the grind! :P labels Apr 1, 2024
@Etaash-mathamsetty Etaash-mathamsetty added pr:ready-for-review Feature-complete, ready for the grind! :P and removed pr:wip WIP, don't merge. labels Apr 23, 2024
@cheesegoldfish
Copy link

Any updates here? I NEEEEEEEED this. Ever since I installed heroic my windows machine will not put the display to sleep, nor will it go into suspended mode.

Since I don't care to compile a build of heroic myself, I'm going back to the epic launcher (yuck) until this can get merged in.

@arielj
Copy link
Collaborator

arielj commented Apr 24, 2024

Since I don't care to compile a build of heroic myself, I'm going back to the epic launcher (yuck) until this can get merged in.

you can download the portable binary from https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/actions/runs/8804826042?pr=3519 no need to compile heroic yourself (actually it would be good to have someone test this to check if it solves the issue, since it seems to be system and DE dependent

@cheesegoldfish
Copy link

cheesegoldfish commented Apr 25, 2024

(actually it would be good to have someone test this to check if it solves the issue, since it seems to be system and DE dependent

On Win 11 pro.

Looks like this actually sorta works.

I can launch the game and it acquires the screen lock

DISPLAY:
[PROCESS] \Device\HarddiskVolume3\Users\aalosb\AppData\Local\Temp\2fVm3vUXQMipsCIrZVhtZQBckLh\Heroic.exe
Electron

EXECUTION:
None.

and then I can exit the game, and the request is released and no long shows

C:\Windows\System32>powercfg /requests
DISPLAY:
None.

C:\Windows\System32>powercfg /requests
DISPLAY:
None.

EXECUTION:
[PROCESS] \Device\HarddiskVolume3\Users\aalosb\AppData\Local\Temp\2fVm3vUXQMipsCIrZVhtZQBckLh\Heroic.exe
Electron

Whereas using the current installer version 2.14.1.321 it will acquire the display power request, but never release it after closing out the game.

However this branch still does acquire an EXECUTION lock, so it still prevents the PC from going to sleep while heroic is running (minimized to the system icon tray), but at least it now lets my screens turn off, so slight improvement.

@Etaash-mathamsetty
Copy link
Member Author

i don't see any bug in my code but it's possible that there's a bug, will let someone else look into it

Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

Tested here and everything seems to be working fine

@flavioislima flavioislima merged commit b5c8adf into main Aug 9, 2024
9 checks passed
@flavioislima flavioislima deleted the fix-powersaver-blocker branch August 9, 2024 16:11
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators Aug 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants