Skip to content

Conversation

@kaechele
Copy link
Contributor

This properly splits settings for this plugin into the new split configuration stores for app and user settings.

All user-editable settings go into the user store, all functional, non-user facing settings go in app store (e.g. timestamp of last update check).

On OBS versions prior to 31 there is no change in behaviour and all settings continue to be read from and stored to the global store only.

Fixes #1121

This properly splits settings for this plugin into the new split
configuration stores for app and user settings.

All user-editable settings go into the user store, all functional,
non-user facing settings go in app store (e.g. timestamp of last update
check).

On OBS versions prior to 31 there is no change in behaviour and all
settings continue to be read from and stored to the global store only.

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
@BitRate27
Copy link
Contributor

Thanks for putting this together. I liked how you separated the user from the app settings. Makes total sense. Only issue is that I was not able to build DistroAV with the LIBOBS_API_MAJOR_VER >= 31, so I was not able to test it. I was able to build and test with the < 31 condition and verified settings are preserved. So the code still works with old OBS includes and I still got the deprecated messages in the log when running with 31 dlls.
I feel the DEPRECATED message dumped by OBS when calling the obs_frontend_get_global_config in a 31.0.0 dll is a mistake on OBS side. I prefer the deprecated messages be only shown during builds and as warnings, as it is really only something the developer can resolve. This keeps the user out of it, as many here have thought these messages are the source of their woes, when it is irrelevant.

@kaechele
Copy link
Contributor Author

Thanks.

For what it's worth, I was building and testing this on Fedora 41, where the version of OBS available is currently obs-studio-31.0.0~beta1-4.fc41.x86_64
There, the LIBOBS_API_MAJOR_VER is correctly pulled, so the deprecation message goes away and settings are separated as defined in the code for global.ini and user.ini for new profiles.

For preexisting profiles the settings in global.ini remain intact but they are no longer read from or stored to there (and basically remain as cruft). Currently this leads to the DistroAV settings not being migrated from a global.ini to the user.ini. I will have to revisit that.

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
@kaechele
Copy link
Contributor Author

Took a first stab at a migration functionality. Works fine in my testing here, but I'd certainly appreciate some more testing.
It should:

  • migrate the settings that are now user settings from the global scope to the user scope
  • delete migrated settings from the global scope

If everything worked your original settings from global.ini should now be in user.ini and only app scope settings should remain in global.ini.

@Trouffman
Copy link
Collaborator

Thank you for this change proposal.
Note for testers : This should be tested on OBS 31 RC1 or more recent for pre-validating the changes.

@Trouffman Trouffman added this to the 6.1.0 milestone Dec 3, 2024
@Trouffman Trouffman self-assigned this Dec 22, 2024
@Trouffman Trouffman added enhancement Dependencies Relate to a dependency (package / version) labels Dec 31, 2024
@Trouffman Trouffman changed the base branch from master to obs_31_min_req_update February 4, 2025 22:24
@Trouffman
Copy link
Collaborator

Merging things from the updated OBS Plugin Template for OBS 31 "base".

Due to limited feedback from PR submitter, will try to continue from this to a workable solution.

@Trouffman
Copy link
Collaborator

Update to some concerns in this PR :

PARAM_SKIP_UPDATE_VERSION
Can be set & influenced by the user, the decision is to make it a "User" config

The other elements are not influenced / presented to the User and will be in the App Config:
PARAM_LAST_UPDATE_CHECK
PARAM_MIN_AUTO_UPDATE_CHECK_INTERVAL_SECONDS
PARAM_AUTO_CHECK_FOR_UPDATES

This is based on the commit infos :obsproject/obs-studio@2635cf3

The app config instance contains system-wide settings that mainly
concern the working of the app itself, whereas the user config instance
contains settings actually exposed to the user for the configuration.

obsproject/obs-studio#11055

@Trouffman Trouffman added the Seeking Testers PRs with this label will package the plugin so that others can test label Feb 5, 2025
@Trouffman
Copy link
Collaborator

While digging to update the plugin i foudn out that the check with LIBOBS_API_MAJOR_VER will only check at compile time and not Runtime.

if we want to make the plugin backward compatible we will need to either expand on obs_get_version or simply drop this complexity and make OBS 31 a requirement.

@Trouffman Trouffman deleted the branch DistroAV:obs_31_min_req_update February 10, 2025 02:50
@Trouffman Trouffman closed this Feb 10, 2025
@Trouffman
Copy link
Collaborator

As we made OBS 31 a full requirement for DistroAV 6.1 this can be revisited and simplified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Dependencies Relate to a dependency (package / version) enhancement Seeking Testers PRs with this label will package the plugin so that others can test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: OBS log reports obs_frontend_get_global_config is deprecated.

3 participants