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

fix(youtube/custom-video-buffer): user notice of patch deprecation #1718

Merged
merged 4 commits into from
Mar 12, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ import app.revanced.patches.youtube.misc.videobuffer.fingerprints.PlaybackBuffer
import app.revanced.patches.youtube.misc.videobuffer.fingerprints.ReBufferFingerprint
import org.jf.dexlib2.iface.instruction.OneRegisterInstruction

@Patch
// TODO: either delete or hide this patch (remove @Patch annotation)
@Patch(include = false)
@Name("custom-video-buffer")
@Description("Lets you change the buffers of videos.")
@Description("(deprecated) Lets you change the buffers of videos.")
Copy link
Member

@oSumAtrIX oSumAtrIX Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rather unconventional. No other patch just adds (deprecated) before the description. The patch should inject settings which are in a disabled state and add a text along with it explaining why they are disabled instead of just adding those comments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a patch add a disabled setting preferences? Or can a patch add a text description (with no settings that can be changed?)

The patch itself adds only numerical settings, and there are no switches to set to a disabled state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither a text preference nor the disabed attribute are inmplemented, which are both trivial to add though. It would be necessary to add them in order for this PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added NonInteractivePreference, which is a non selectable Java Preference.
Feel free to change any code as I'm not as familiar with Kotlin.

@DependsOn([SettingsPatch::class])
@CustomVideoBufferCompatibility
@Version("0.0.1")
Expand All @@ -45,7 +46,7 @@ class CustomVideoBufferPatch : BytecodePatch(
SettingsPatch.PreferenceScreen.MISC.addPreferences(
PreferenceScreen(
"revanced_custom_video_buffer",
StringResource("revanced_custom_video_buffer_title", "Video buffer settings"),
StringResource("revanced_custom_video_buffer_title", "Notice: custom buffer will soon be removed"),
listOf(
TextPreference(
"revanced_pref_max_buffer_ms",
Expand Down Expand Up @@ -81,7 +82,9 @@ class CustomVideoBufferPatch : BytecodePatch(
)
)
),
StringResource("revanced_custom_video_buffer_summary", "Custom settings for video buffer")
StringResource("revanced_custom_video_buffer_summary",
"Due to recent changes by YouTube, this patch no longer functions correctly" +
" and this patch will be removed in a future ReVanced release.")
)
)
BufferType.values().forEach { type ->
Expand Down