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

Update 16.9.1 #671

Merged
merged 57 commits into from
Aug 23, 2024
Merged

Update 16.9.1 #671

merged 57 commits into from
Aug 23, 2024

Conversation

Zerwin
Copy link
Contributor

@Zerwin Zerwin commented Aug 16, 2024

Changes

Updates the app to 16.9.1

Checklist

  • My change builds (Note: I used an base APK url from APK Mirror, the link will probably expire at some point.
  • I tested my change
  • I use the "bttv_" prefix for all resources I propose
  • When adding a string I also added it to the bttv.Res.strings Enum and res/values/strings.xml (in mod) and res/values/public.xml (in disass)
  • If my change is significant enough, I added it to the CHANGELOG.md under master
  • I'll add myself and everyone else who contributed to this change to the contributors list using all-contributors (Can I even add myself ?)
  • I license my changes according to the MIT License.

Copy link

welcome bot commented Aug 16, 2024

Thank you for opening this pull request ♥️ A maintainer should be by to give feedback soon.
modCheck

@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 17, 2024

@FoseFx Anything I should change about this ?

Copy link
Member

@FoseFx FoseFx left a comment

Choose a reason for hiding this comment

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

I am now done with a brief static review.

  • Changes look largely good, thank you!
  • Please revert the changes to the scripts and CI
  • I do not see any modifications to the mod/twitch package, which is unlikely. Please run the ubi script, after installing ubi, and running buildsource once. You will likely see inconsistencies between the mocks in mod/twitch, and the actual methods in the apk.
  • I have not built and tested your changes yet, please apply these changes first

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
build Outdated Show resolved Hide resolved
buildsource Outdated Show resolved Hide resolved
disassemble Outdated Show resolved Hide resolved
initworkspace Outdated Show resolved Hide resolved
ubi Outdated Show resolved Hide resolved
patches/res.values.ids.xml.patch Outdated Show resolved Hide resolved
@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 18, 2024

How do I run the ubi script ? I found your ubi repo and build an ubi release, but if I run the script I just get

Found argument '--no-diff' which wasn't expected, or isn't valid in this context

If I remove --no-diff I then get

Found argument '/opt/bttv/baksmali-2.4.0.jar' which wasn't expected, or isn't valid in this context

Is there something else I need to do ?

@FoseFx
Copy link
Member

FoseFx commented Aug 18, 2024

How do I run the ubi script ? I found your ubi repo and build an ubi release, but if I run the script I just get

Found argument '--no-diff' which wasn't expected, or isn't valid in this context

If I remove --no-diff I then get

Found argument '/opt/bttv/baksmali-2.4.0.jar' which wasn't expected, or isn't valid in this context

Is there something else I need to do ?

Did you build the master or the v2 branch? The v2 branch is not ready to use.

@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 18, 2024

Yep, that was it. I was on the v2 branch for some reason. Switched to master and now it ran. Do I need to manually fix the diffs or does the script do that automatically ?

@FoseFx
Copy link
Member

FoseFx commented Aug 18, 2024

No, the script is purely informational. Open up the mod project in Android Studio and rename / remove / create (new) classes / interfaces / methods of the twitch library. It hosts the shims, which are used in the app library (which is copied into the APK in buildsource). The goal here is not to perfectly replicate all internal twitch classes, but to have the minimal amount needed for the app to work.

Often classes / interfaces move places, sometimes they get split apart and sometimes they just get removed without replacement.

My advice is to use Android Studio's refactor tools (for example, it can move a class to a different package, and update all references).

@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 18, 2024

What does this warning mean ?

WARN  ubi] not found in disass: tv/twitch/android/models/chat/AutoModMessageFlags.smali

I tried just removing the file in the mod/twitch folder but even after a buildsource and running the ubi script again it still prints the warning

@FoseFx
Copy link
Member

FoseFx commented Aug 18, 2024

What does this warning mean ?

That the AutoModMessageFlags class does not exist (in that package) anymore. Try to find it in the disass directory, it has moved somewhere else, or it got removed / renamed.

I tried just removing the file in the mod/twitch folder but even after a buildsource and running the ubi script again it still prints the warning

rm /tmp/bttv-* -rf && ./buildsource ... && ./ubi

should work

@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 18, 2024

Okay, all done.

@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 21, 2024

@FoseFx anything stopping this from merging now ?

@FoseFx
Copy link
Member

FoseFx commented Aug 23, 2024

I just tried it out, Nice work!

@all-contributors please add @Zerwin for code

Copy link
Contributor

@FoseFx

I've put up a pull request to add @Zerwin! 🎉

@FoseFx FoseFx merged commit 3726177 into bttv-android:master Aug 23, 2024
0 of 2 checks passed
Copy link

welcome bot commented Aug 23, 2024

Thanks for your contribution to bttv-android! 🎉
PepoDance

@FoseFx
Copy link
Member

FoseFx commented Aug 23, 2024

Your change was just Released!

@Zerwin Zerwin deleted the update-16.9 branch August 23, 2024 16:31
@Zerwin
Copy link
Contributor Author

Zerwin commented Aug 24, 2024

@FoseFx Do you think I could get permissions for handling Github issues ? Then I would comb through some of the existing ones to see if they still make sense with the new version.

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