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

Add onProfileSettingChanged #159

Closed

Conversation

cram42
Copy link

@cram42 cram42 commented Aug 3, 2024

Allows registering a new callback to be called when a profile setting is changed.

@Holt59
Copy link
Member

Holt59 commented Aug 3, 2024

Thanks for the contribution.

Some notes:

  • The setting "name" should probably be an enum. Unlike plugin were we have no control over the list of settings, we have a fixed set of settings for profiles, so this would probably be better than using strings.
  • I am not sure I like the QVariant approach here, for the same reason that we know the list of settings. Maybe the callback could be something like this:
enum class BooleanProfileSetting {
    LocalSaves,
    LocalSettings,
    ArchiveInvalidation
};

// in the future
enum class StringProfileSetting {
    Whatever
};

// for direct access like ProfileSetting::LocalSaves
struct ProfileSetting {
    using enum BooleanProfileSetting;
    using enum StringProfileSetting;
};

 virtual bool onProfileSettingChanged(
    BooleanProfileSetting setting,
    std::function<void(IProfile*, bool, bool) const& func) = 0;

 virtual bool onProfileSettingChanged(
    StringProfileSetting setting,
    std::function<void(IProfile*, const QString&, const QString&) const& func) = 0;

Then you could registered it like this:

organizer->onProfileSettingChanged(IOrganizer::ProfileSetting::LocalSaves, [](auto* profile, auto, auto enabled) {
    MOBase::log::info("local saves for profile '{}' are now {}", 
                      profile->name(), enabled ? "enabled" : "disabled");
});

Not sure when we will be able to merge this, we are currently on the verge of releasing 2.5.2 and there are big changes coming for 2.6. Also I plan on refactoring the way profiles are exposed to uibase at some point, so I'm not sure this would be merged before that.

@cram42
Copy link
Author

cram42 commented Aug 4, 2024

Hey, that's a great idea! I was originally planning to use the ProfileSetting enum, but it's not accessible to the UI base as it's sitting in profile.h.

Main reason I want to add this is an advanced plugin for The Sims 4 to handle the whole data directory. Tried using InitializeProfile as it seems to get called sometimes when the checkboxes change, but the setting values are inconsistent during my testing.

Will work on it some more and resubmit. Not expecting it to get added anytime soon. Gotta get a stable public release out first of course!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants