netbrain / zwift

Easily zwift on linux
The Unlicense
230 stars 27 forks source link

fix: do not require gamemode installed #151

Closed perrin4869 closed 3 weeks ago

perrin4869 commented 3 weeks ago

Unfortunately, screensaver shenanigans are not over yet... When I tested gamemode I used my Slackware desktop PC, and I had installed gamemode on in to test it out. Earlier when I tried to run zwift on my Arch Linux laptop, unfortunately I encountered the screensaver and monitor power off issue again... Turns out that gamemode needs to be installed on the host system for it to work. In the arch linux laptop, it was missing. When it is missing, you can see an error:

+ /usr/games/gamemoderun wineserver -w
GameMode ERROR: D-Bus error: Could not call method 'RegisterGame' on 'com.feralinteractive.GameMode': The name is not activatable
gamemodeauto: D-Bus error: Could not call method 'RegisterGame' on 'com.feralinteractive.GameMode': The name is not activatable
GameMode ERROR: D-Bus error: Could not call method 'UnregisterGame' on 'com.feralinteractive.GameMode': The name is not activatable
gamemodeauto: D-Bus error: Could not call method 'UnregisterGame' on 'com.feralinteractive.GameMode': The name is not activatable

That is because usr/share/dbus-1/services/com.feralinteractive.GameMode.service is missing. In this case, the screensaver Inhibit method will not be called, and the screen will go on to be activated. If gamemode is installed in the host system, this will work fine though.

The aim of this PR is to try to have the best of all worlds, we do not require gamemode to be available on the host system, and do the heartbeat like we used to... I don't think those will conflict now that I think about it, since they use different mechanisms to inhibit the screensaver. An alternative might be to mention that gamemode needs to be available on the host system to work...

perrin4869 commented 3 weeks ago

I mean, you can always turn off the screensaver functions before running zwift, but that's just one more thing to remember to do each time, and it's quite frustrating to be in the middle of a training and have the damned thing go off or have the screen powered off haha

I'm curious how do you handle this issue on your end? Am I the only one who has the computer setup to power off my monitors or to suspend when idle after a while?

I think this should be part of zwift, just like it is part of lutris or firefox... the problem here is that since it is running inside a container, some things are more complicated...

sHedC commented 3 weeks ago

I did try running gamemode outside of docker but it is then not aware. Gamemode is used by default on quite a few distros so maybe, warn on startup gamemode is not available so zwift won't inhibit screen saver by default?

Then if there use gamemode if not just run as normal?

Choice then is install gamemode on the host or manage screen saver yourself. Readme can be updated.

perrin4869 commented 3 weeks ago

I can live with installing gamemode on my hosts. I do think that the solution in this PR is more user friendly, but hardly fool-proof, so feel free to close it if you don't want to implement it

sHedC commented 3 weeks ago

I can update the Readme doing some workflow updates for the rebuild/ update improvmenets can do some work on that. Yes I think it we try to cover too much it could be a can of worms and game mode does so much already.

perrin4869 commented 3 weeks ago

having slept on it, I think another option to consider would be to forgo gamemode and call the inhibit methods directly:

COOKIE=$(dbus-send --session \
                  --dest=org.freedesktop.ScreenSaver \
                  --type=method_call \
                  --print-reply \
                  --reply-timeout=2000 \
                  /ScreenSaver \
                  org.freedesktop.ScreenSaver.Inhibit \
                  string:zwift \
                  string:zwift \
                  | grep uint32 | cut -d ' ' -f 5)

This would be called before starting zwift, and after zwift exits, we call

dbus-send --session \
                      --dest=org.freedesktop.ScreenSaver \
                      --type=method_call \
                      /ScreenSaver \
                      org.freedesktop.ScreenSaver.UnInhibit \
                      uint32:$COOKIE

The advantage here is not needing an additional dependency on the host machine (this could be mitigated by a package manager that would handle these dependencies though). Personally, now that I know about this, I have no problem installing gamemode on hosts where I'll use zwift, but I can see this being a headache for other people who may not dig into the README right away. Probably should have just gone for this method from the beginning, since tools like xdg-screensaver, in their attempt to handle as many cases as possible, can become quite convoluted on its own, not to mention inside a container.

The disadvantage to this method would be losing gamemode performance benefits, but those are not entirely clear to me, and I'm wondering if they would apply if gamemode is running inside a container?

Anyways, I realize that at this point you're probably tired of this issue which doesn't seem to affect you, sorry about that

netbrain commented 3 weeks ago

First off, I can assure you I'm not tired of the issue 🤗. And yes it doesn't affect me as I don't use a screensaver and generally I always have my monitor on.

I do however want all parties involved to find the best solution to this problem, as it is clearly a problem for users running screensavers and power management.

So I think it's worth thinking about alternative solutions as stated in my previous post, as you now clearly have. 👍

I'm still on vacation, but I'll try to delve deeper into this when I get back home.

At the same time that I want to find a good solution I also want it to be as minimal as possible (separation of concerns and all that good stuff)

The main focus of the project should be the ease of use and installation of zwift but also keeping complexity to a minimum for users and maintainers.

With this in mind, I think we can handle this issue.

netbrain commented 3 weeks ago

Oh, and I do like your last suggestion here with just invoking dbus.

Maybe we should have pre and post Zwift hooks that can be configured and maybe extended with custom functionality?

Could be as simple as two bash scripts being invoked before and after container startup and exit.

And maybe screensaver could be it's first citizen?

Just a suggestion, or food for thought. As it might just increase complexity as I can't think of any other things that would require pre-post hooks.

sHedC commented 3 weeks ago

Just a note on game mode, this is some of the key things it does, also on my laptop it disabled powersave whilst running so no sleep.

It gives about a 10% boost in performance, which is a lot less noticable on my current over spec'd machine that it used to be when my FPS was 50.

It comes pre installed on most of the distros I use, some games in linux are starting to have it enabled by default and steam uses it.

I guess downside is for distros where it is not pre-installed, I guess distros where you start with more base install and add what you want on top. Reasons I suggested:

Not sure how good it is on systems without systemd but apparently it does support them.

I am not sure of many downsides to using it by default but maybe adding ability to set alternative by custom scripts embedded.

Open for anything with one priority for me: ease of use for non-technical people who just want to switch to linux because windows is spyware.

sHedC commented 3 weeks ago

FYI: Wine Builds are not working because of some depenancy issues between 64 and 32 bit libraries since 9th July. Its throwing version discrepencies between libavif16 and libavif16:i386.

perrin4869 commented 3 weeks ago

Maybe we should have pre and post Zwift hooks that can be configured and maybe extended with custom functionality?

Right, so I had a very similar thought, it feels like a natural solution to the problem, to add extensibility, but I didn't pursue this mainly because, I couldn't think about any other use case for the pre- and post- hooks, other than this screensaver. If we can't think of another use-case for this hooking functionality, I think that it is overkill for just one feature.

It gives about a 10% boost in performance, which is a lot less noticable on my current over spec'd machine that it used to be when my FPS was 50.

Interesting, I also didn't notice any performance improvements on my PC, but if you could benchmark some noticeable improvements, that sounds good!

I'm going to close this PR, since we are all convinced that xdg-screensaver is not the way at this point. I think of 2 different options here, if we want to improve this:

  1. No new functionality gets added, but add a warning on startup about gamemode needing to be available for best features. In this case, it might be worth it to build packages with dependencies for different distros, that way less users need to think about hunting down dependencies.
  2. In addition to gamemode, we add the dbus calls. I don't think having multiple calls to Inhibit is problematic, and if someone forgets to install gamemode, they will still get a fallback that should work on most common cases.

If you are interested in pursuing any of those, I will happily put together a PR!

sHedC commented 3 weeks ago

Interesting, I also didn't notice any performance improvements on my PC, but if you could benchmark some noticeable improvements, that sounds good!

Difference of machines, noticing a powerful machine like an AMD 5700 and GeForce 3050 difference might be beetween 300 adn 330 fps which is not noticable. The 10% comes from other people doing tests but I noticed this when playing games on my laptops.

  1. No new functionality gets added, but add a warning on startup about gamemode needing to be available for best features. In this case, it might be worth it to build packages with dependencies for different distros, that way less users need to think about hunting down dependencies.

Gamemode does require systemd, so yes there are distro's that it won't work on. Not sure what you mean by dependancies for different distros?

  1. In addition to gamemode, we add the dbus calls. I don't think having multiple calls to Inhibit is problematic, and if someone forgets to install gamemode, they will still get a fallback that should work on most common cases.

True, if that is what you meant by item 1. So basically gamemode unless not installed then fallback to another method? Can we tell if gamemode works without using the install script (i.e. in the docker)

If you are interested in pursuing any of those, I will happily put together a PR!

Works for me, I am happy to update the README, I was thinking becuase of the additions updating the readme to add at the top a simple install guide for non-tech, setting up zwift/config, a little more detail for tech and re-layout the FAQ?

I am currently working on a rebuild workflow.

perrin4869 commented 3 weeks ago

Gamemode does require systemd, so yes there are distro's that it won't work on. Not sure what you mean by dependancies for different distros?

Actually, on Slackware we use sysvinit, but gamemode is available from SBo and the inhibit functionality works, so systemd is not required, or only a subset of the functionality requires systemd. What I meant, is to have a package for zwift on aur, SBo, etc, that will specify gamemode as a dependency

Can we tell if gamemode works without using the install script (i.e. in the docker)

I think we can detect gamemode from within the container by calling the Introspect method for com.feralinteractive.GameMode:

$ dbus-send --session           \
  --dest=com.feralinteractive.GameMode \
  --type=method_call          \
  --print-reply               \
  /com/feralinteractive/GameMode       \
  org.freedesktop.DBus.Introspectable.Introspect
method return time=1720622791.599156 sender=:1.62 -> destination=:1.759 serial=25 reply_serial=2
   string "<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
"https://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
<node>
 <interface name="org.freedesktop.DBus.Peer">
  <method name="Ping"/>
  <method name="GetMachineId">
   <arg type="s" name="machine_uuid" direction="out"/>
  </method>
 </interface>
 <interface name="org.freedesktop.DBus.Introspectable">
  <method name="Introspect">
   <arg name="xml_data" type="s" direction="out"/>
  </method>
 </interface>
 <interface name="org.freedesktop.DBus.Properties">
  <method name="Get">
   <arg name="interface_name" direction="in" type="s"/>
   <arg name="property_name" direction="in" type="s"/>
   <arg name="value" direction="out" type="v"/>
  </method>
  <method name="GetAll">
   <arg name="interface_name" direction="in" type="s"/>
   <arg name="props" direction="out" type="a{sv}"/>
  </method>
  <method name="Set">
   <arg name="interface_name" direction="in" type="s"/>
   <arg name="property_name" direction="in" type="s"/>
   <arg name="value" direction="in" type="v"/>
  </method>
  <signal name="PropertiesChanged">
   <arg type="s" name="interface_name"/>
   <arg type="a{sv}" name="changed_properties"/>
   <arg type="as" name="invalidated_properties"/>
  </signal>
 </interface>
 <interface name="com.feralinteractive.GameMode">
  <property name="ClientCount" type="i" access="read">
  </property>
  <method name="RegisterGame">
   <arg type="i" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="UnregisterGame">
   <arg type="i" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="QueryStatus">
   <arg type="i" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="RegisterGameByPID">
   <arg type="i" direction="in"/>
   <arg type="i" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="UnregisterGameByPID">
   <arg type="i" direction="in"/>
   <arg type="i" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="QueryStatusByPID">
   <arg type="i" direction="in"/>
   <arg type="i" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="RegisterGameByPIDFd">
   <arg type="h" direction="in"/>
   <arg type="h" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="UnregisterGameByPIDFd">
   <arg type="h" direction="in"/>
   <arg type="h" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="QueryStatusByPIDFd">
   <arg type="h" direction="in"/>
   <arg type="h" direction="in"/>
   <arg type="i" direction="out"/>
  </method>
  <method name="RefreshConfig">
   <arg type="i" direction="out"/>
  </method>
  <method name="ListGames">
   <arg type="a(io)" direction="out"/>
  </method>
  <signal name="GameRegistered">
   <arg type="i"/>
   <arg type="o"/>
  </signal>
  <signal name="GameUnregistered">
   <arg type="i"/>
   <arg type="o"/>
  </signal>
 </interface>
 <node name="Games"/>
</node>
"

This is the result when the object is available on dbus, meaning gamemode is installed on the host.

sHedC commented 3 weeks ago

Actually, on Slackware we use sysvinit, but gamemode is available from SBo and the inhibit functionality works, so systemd is not required, or only a subset of the functionality requires systemd. What I meant, is to have a package for zwift on aur, SBo, etc, that will specify gamemode as a dependency

The zwift startup application part. So would have to have Podman/ Docker and if you have Nvidia the Nvidia Toolkit, well I suppose not really a bother for me as I use flatpak now in preference (where maintained properly).

I think we can detect gamemode from within the container by calling the Introspect method for com.feralinteractive.GameMode:

That sounds like being useful, but then just document about gamemode in the readme rather than the zwift startup file, or use the zwift startup to detect gamemoded -v, then we can show a warning if not there and pass a variable?

sHedC commented 2 weeks ago

I think if adding support, maybe Gamemode if available, simple screen inhibit if not and add potential to support local pre/ post scripts?

I think check for game mode in the startup script then a warning can be displayed?

Maybe create an issue as I think better place to discuss?