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

feat: settings patch #2366

Closed
oSumAtrIX opened this issue Jul 11, 2022 · 6 comments
Closed

feat: settings patch #2366

oSumAtrIX opened this issue Jul 11, 2022 · 6 comments
Assignees

Comments

@oSumAtrIX
Copy link
Member

oSumAtrIX commented Jul 11, 2022

🐞 Issue

Currently, no patch to display settings in the YouTube application exists. Patches that depend on a settings menu will not work correctly.

❗ Solution

Add a patch to integrate the settings into the application.

⚠ Additional context

The implementation of this patch will have to be discussed thoroughly. It will work as a dependency for other patches and should support a basic framework for other patches to include their settings.

Pseudo implementation in other patches:

// this may be any class that implements Patch
addSetting { // patch name derived from class
    category("Category 1") {
        checkbox("Checkbox 1", true)
    }
    category("Category 2") {
        checkbox("Checkbox 2", false)
    }
}
@shadow578
Copy link
Contributor

Proposal

My proposal is to keep runtime preference handling (so definition preferences & screen layout) in revanced-integrations.
Preferences during patching should be handled elsewhere.

Prerequisites / ToDo

the following points are definitively missing for this implementation to work well:

  • metadata: list of active patches accessible from revanced-integrations: can be as simple as injecting a list of patches into BuildConfig of integrations, or something similar
  • registration: to create the screens, instances of all PatchPreferencesScreen must be available to the activity. I haven't found a good way of doing this

API

So, when writing a integration for a patch, you'd create a MyPatchPreferences class like this (see code comments for more details):

/**
 * every patch gets their own Preferences class that handles creating the settings screen & housing the preference wrappers
 *
 * the [PatchPreferencesScreen] class gets a reference to the patch name & can decide if it should enable or disable the patch
 */
class MyPatchPreferences : PatchPreferencesScreen("swipe-controls") {
    /**
     * preferences are defined per- patch in a companion object
     * (behaves similar to static class members)
     */
    companion object {
        // 'PreferenceWrapper' is a generic class, it's type is inferred from the defaultValue parameter
        val myBooleanPreference = PreferenceWrapper("my_boolean", false)

        // all primitives (string, boolean, int, long, double, float) + enums are supported, more can be added
        val myEnumPreference = PreferenceWrapper("my_enum", FooBar.NONE)

        // patch preferences using this api should be written in kotlin (makes stuff easier)
        // when using java for the rest of the patch, mark the preference as @JvmStatic
        @JvmStatic
        val myOtherBooleanPreference = PreferenceWrapper("my_other_more_different_boolean", true)
    }
}

the layout on the preference screen is written per-patch in a DSL, either as a loose wrapper around android(x).preference or with custom views:

   /**
     * createScreen handles creating the settings screen.
     * Each patch has it's own sub-screen that is shown on a common 'ReVanced Settings' screen, so only one entry must be injected into xml
     */
    override fun initScreen() = screen {
        // screens are created in a dsl, either directly
        // exposing components from android.preference.* (easiest) or wrappers for custom controls (harder)
        title = "My Patch's Preferences"
        summary = "Idk, something about how great this sample patch is and what it does..."

        // preference screens can be divided into categories
        category {
            title = "My category"

            // controls are automagically bound to the preferences and update when the preference changes (and vise-versa)
            enum(myEnumPreference) {
                title = "Enums are fun!"

                // mapping from enum ordinal to display names shown to the user
                mapOf(
                    FooBar.NONE to "No FooBar for you!",
                    FooBar.FOO to "Enable Foo",
                    FooBar.BAR to "Okay, just a bit of Bar"
                )
            }
            // dependencies to other preferences (or state in general) can be expressed
            // with a 'dependsOn' statement at any level in the hierarchy
            switch(myBooleanPreference) {
                title = "This is a switch, only for 'BAR' enjoyers!"
                summaryOff = "you turned stuff off"
                summaryOn = "and now it's turned back on"

                // the switch for 'myBooleanPreference' is only enabled if 'myEnumPreference' is set to 'BAR'
                dependsOn(myEnumPreference) { myEnumPreference.value == FooBar.BAR }
            }
        }

        // you can have multiple categories, and they can also have dependencies
        category {
            title = "Debug settings"

            // preferences are automatically updated when changed, so when this checkbox is checked...
            checkBox(myOtherBooleanPreference) {
                title = "Another boolean, this time a checkbox"
                summaryOff = "it's off"
                summaryOn = "you get the idea..."
            }

            // ... this switch will toggle at the same time
            switch(myOtherBooleanPreference) {
                title = "running out of text..."
            }

            // only enable 'debug settings' category when in a debug build
            dependsOn { BuildConfig.DEBUG }
        }
    }

accessing the preferences is possible in a type-safe (and typo- free) way from both java and kotlin:

class MyPatch {
    fun foobar() {
        // preferences can be accessed in a type-safe manner
        if (MyPatchPreferences.myBooleanPreference.value) {
            // 'myBooleanPreference' is set to 'true'
        }

        // if the preference value is accessed very frequently, use the cached value (read- only)
        for (i in 0..100000) {
            // vv this effectively only loads the preference once, and serves the cached value on all other accesses
            if (MyPatchPreferences.myOtherBooleanPreference.valueCached) {
                Log.i("Fast", "i am speed")
            }
        }

        // you can also set the preference, ofc
        // it is automatically written to SharedPreferences
        MyPatchPreferences.myEnumPreference.value = FooBar.FOO
    }
}
public class MyJavaPatch {
    public void foobar() {
        // accessing preference not marked with @JvmStatic from java code
        if (MyPatchPreferences.Companion.getMyBooleanPreference().getValue()) {

        }

        // vs. accessing with @JvmStatic
        if (MyPatchPreferences.getMyOtherBooleanPreference().getValue()) {

        }
    }
}

as you may tell, i used something similar in a different app already. Relevant snippets can be found here.
But please keep discussions out of the gist and in this issue, so they can be found easily

@Sculas
Copy link
Contributor

Sculas commented Jul 12, 2022

@shadow578 Your proposal looks better. If we want to go this route, we need to figure out how to tightly couple integrations and patches together so it will work that way. The patches will have to depend on integrations as a dependency, while revanced-integrations is an APK. That means we'd have to make the patches an Android project. This would instantly fix the problem of resources not being available to the manager. But now, how should we make a patches JAR for the CLI?

@TheJeterLP
Copy link

TheJeterLP commented Jul 12, 2022

I don't actually see the reason for all this hassle. Why not just keep the current (working system). Inject all the installed patches with a patch into the integrations, and then only show the prefScreens that are actually installed? Would save so much time and work because its already nearly finished.

@shadow578
Copy link
Contributor

I don't actually see the reason for all this hassle. Why not just keep the current (working system). Inject all the installed patches with a patch into the integrations, and then only show the prefScreens that are actually installed? Would save so much time and work because its already nearly finished.

exactly what i was thinking too.
just keep the system as-is, but make a list of active patches available (even just making it available to the execute() of patches would be enough, that way the settings patch can handle it in whatever way).

That way, patches stays (mostly) the same with a settings patch that can be a dependency to the other patches (via @Dependency), and integrations handles the rest via PreferenceWrapper and DSL.

On the note of localisation, if we leave revanced (or just preferences, for that matter) english only, we can just use string constants. Otherwise, we'd have to inject string resources for every patch that has preferences (tho that's a one-liner with ResourceData.injectStrings)

@TheJeterLP
Copy link

okay, so @oSumAtrIX have been in a voicecall now and discussed about all that. I understand what he's trying to do and it makes sense to prevent future problems from the beginning. Maybe he can explain to you

@github-actions
Copy link

πŸŽ‰ This issue has been resolved in version 2.40.0 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

@revanced-bot revanced-bot transferred this issue from ReVanced/revanced-patches-template Dec 14, 2023
@alexandreteles alexandreteles transferred this issue from another repository Dec 14, 2023
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

No branches or pull requests

4 participants