search

ubuntu-flutter-community / settings

:penguin: :orange_heart: :blue_heart: An Ubuntu Desktop system settings app made with Flutter.
GNU General Public License v3.0
272 stars 47 forks source link

Changing some settings do not work well #310

Closed mivoligo closed 2 years ago

mivoligo commented 2 years ago

I've noticed that changing some settings do not really work. For example: In Power page I change "Blank screen" to any value and it always results in 300 seconds. That value is the default value for that settings according to Dconf Editor. power

Similar issue is with:

  1. Privacy > House Keeping > Recycle bin > Period for automatic deletion
  2. Privacy > House Keeping > Screen Lock > both delay settings
mivoligo commented 2 years ago

FWIW I've noticed all of those values are of type Unsigned 32-bit integer according to dconf. Meanwhile the setting which works properly: "Period of file history" (/org/gnome/desktop/privacy/recent-files-max-age) is of type Signed 32-bit integer.

mivoligo commented 2 years ago

So I think the issue might be here: https://github.com/Feichtmeier/settings/blob/32b7b128f0b38a5c3d504ee9112e8f360614808e/lib/services/settings_service.dart#L80

Here's the code in dbus for unsigned integer: https://github.com/canonical/dbus.dart/blob/e003b66747ba0dea6ed4084a74331bd35d0a8af5/lib/src/dbus_value.dart#L208

Feichtmeier commented 2 years ago

Nice finding. This could be indeed an issue. So should we add one case for each type of int in the settingsservice?

mivoligo commented 2 years ago

I believe so but not sure how to handle that properly. Maybe @jpnurmi could help here.

jpnurmi commented 2 years ago

Hah, yes, that's wrong. Too much convenience bites back. :slightly_smiling_face: Anyway, maybe pair the get<Type>Value() getters with respective set<Type>Value() setters, to eliminate ambiguity?

mivoligo commented 2 years ago

Hah, yes, that's wrong. Too much convenience bites back. slightly_smiling_face Anyway, maybe pair the get<Type>Value() getters with respective set<Type>Value() setters, to eliminate ambiguity?

Looks like quite a big change :slightly_smiling_face: setValue is used more than 130 times :smile:

BTW shouldn't this line: https://github.com/Feichtmeier/settings/blob/32b7b128f0b38a5c3d504ee9112e8f360614808e/lib/services/settings_service.dart#L72 be changed to if (_values[key] == value) { ?

jpnurmi commented 2 years ago

@Feichtmeier This is why tests pay off 😉