galister / wlx-overlay-s

Access your Wayland/X11 desktop from Monado/WiVRn/SteamVR. Now with Vulkan!
GNU General Public License v3.0
100 stars 18 forks source link

fix: loading pipewire tokens #24

Closed alexdenerqal closed 3 months ago

alexdenerqal commented 3 months ago

ignore pw_tokens.yaml when loading files from conf.d, and fix load_pw_token_config

galister commented 3 months ago

I think it'd be more proper to fix it so that the current files load without an issue. maybe better to revert the tokens part of the recent config change.

galister commented 3 months ago

nevermind, i guess this makes it so that no new file is needed?

either way, i don't really like the skip file by name part, can you try if just removing the pw tokens from the main config will make it happy?

diff --git a/src/config.rs b/src/config.rs
index 2116da7..63ff6a9 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -179,9 +179,6 @@ pub struct GeneralConfig {
     #[serde(default = "def_one")]
     pub long_press_duration: f32,

-    #[serde(default = "def_pw_tokens")]
-    pub pw_tokens: PwTokenMap,
-
     #[serde(default = "def_osc_port")]
     pub osc_out_port: u16,
alexdenerqal commented 3 months ago

(removing pw_tokens from the main config did not make it happy)

For me it fails at loading the pw_tokens.yaml, because it expects a map but that file has a sequence in it

thread 'main' panicked at src/config.rs:341:13:
Failed to build settings: invalid type: sequence, expected a map in ../../.config/wlxoverlay/conf.d/pw_tokens.yaml in ../../.config/wlxoverlay/conf.d/pw_tokens.yaml

but skipping it although I do admit is not an optimal solution, it did work, because pw_tokens.yaml is loaded elsewhere anyways.

I am open to more elegant fixes, but I could not come up with one, as my experience with rust is limited.

galister commented 3 months ago

I don't quite get why it would deserialize it as a map. Previously it was a Vec<(String, String)> , now it's a Vec<(Arc<str>, String)>. We never used a map for serialization.

galister commented 3 months ago

Arguably the ideal solution would have been to keep the pw_tokens out of the conf.d folder.

galister commented 3 months ago

does this work? (without your changes)

diff --git a/src/config.rs b/src/config.rs
index 2116da7..63ff6a9 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -179,9 +179,6 @@ pub struct GeneralConfig {
     #[serde(default = "def_one")]
     pub long_press_duration: f32,

-    #[serde(default = "def_pw_tokens")]
-    pub pw_tokens: PwTokenMap,
-
     #[serde(default = "def_osc_port")]
     pub osc_out_port: u16,

diff --git a/src/overlays/screen.rs b/src/overlays/screen.rs
index fea1d40..539c318 100644
--- a/src/overlays/screen.rs
+++ b/src/overlays/screen.rs
@@ -629,8 +629,9 @@ fn get_pw_token_path() -> PathBuf {
 }

 #[cfg(feature = "wayland")]
-pub fn save_pw_token_config(tokens: &PwTokenMap) -> Result<(), Box<dyn Error>> {
-    let yaml = serde_yaml::to_string(tokens)?;
+pub fn save_pw_token_config(tokens: PwTokenMap) -> Result<(), Box<dyn Error>> {
+    let conf = TokenConf { pw_tokens: tokens };
+    let yaml = serde_yaml::to_string(&conf)?;
     std::fs::write(get_pw_token_path(), yaml)?;

     Ok(())
@@ -718,7 +719,7 @@ pub fn create_screens_wayland(

     if pw_tokens_copy != pw_tokens {
         // Token list changed, re-create token config file
-        if let Err(err) = save_pw_token_config(&pw_tokens) {
+        if let Err(err) = save_pw_token_config(pw_tokens) {
             log::error!("Failed to save Pipewire token config: {}", err);
         }
     }
alexdenerqal commented 3 months ago

does this work? (without your changes)

yep, works perfect

galister commented 3 months ago

cool, wanna submit this?

alexdenerqal commented 3 months ago

cool, wanna submit this?

sure

galister commented 3 months ago

thanks!