ReVanced / GmsCore

Free implementation of Play Services
https://microg.org
Apache License 2.0
2.09k stars 96 forks source link

make settings app distinguishable from official "microG settings" #86

Closed konradmoesch closed 4 months ago

konradmoesch commented 4 months ago

This PR changes the app icon background color to the one from the ReVanced logo to make the settings app distinguishable from the official one. As you can see, this PR is minimal-invasive (as you can see, one line in one xml file is changed) and requires less maintenance than for example the change of the basePackageName Please merge this PR, I think it is in your (and the microG-team's) best interest to improve distinguishability of the two forks (and therefore adds to the goal of being able to run those in parallel). Thanks!

oSumAtrIX commented 4 months ago

Unfortunately this PR violates the goals of this fork: https://github.com/ReVanced/GmsCore?tab=readme-ov-file#about-this-fork

Without any changes to the goals such a PR can't be merged. A discussion regarding it proposed to create a branch that fulfils the goal of the fork. A separate branch could get features such as this PR and the goal branch can be merged into that branch. The issue with that is that we would have to maintain non-goals which is yet another burden that does not contribute to the goal of this fork. In addition the feature change is arbitrary and changes the color of the icon to a random one, disgarding any design guidelines or motivations. For that reason such a feature should either be submitted to upstream or requested to be merged in a different fork. Thanks regardless.

konradmoesch commented 4 months ago

If I read it, the goal is to be able to run revanced-microg next to official microg. Without changes to the icon or label (or hiding the unnecessary settings app, it is not possible to distinguish the two apps / projects, thus violating the goal in my opinion. Please reconsider merging, as I said, it just changes one color -> no maintenance, no merge conflicts possible etc.

Any upstream changes seem illogical to me, could you please elaborate? Changing the icon or label upstream would mean the changes also get pulled into this fork -> no distinguishability achieved. I also don't see where a upstream change is necessary for a simple color change...

konradmoesch commented 4 months ago

The color is not arbitrary, it is the same as in you logo. One could also extend by creating an own icon for revanced-microg-settings, but that mean extra work (and should mean some work to put into this not really needed settings-app)

oSumAtrIX commented 4 months ago

If I read it, the goal is to be able to run revanced-microg next to official microg. Without changes to the icon or label (or hiding the unnecessary settings app, it is not possible to distinguish the two apps / projects, thus violating the goal in my opinion.

The app runs next other GMS installations. The icon is not preventing that. Changing the icon does not further help that goal as another app with the same icon could be installed. This app can not account for any other specific apps such as upstream GmsCore, but only account for a generalized view to any GMS apps. For that reason, changing the icon just to satisfy one kind of GMS does not satisfy any other kind.

Any upstream changes seem illogical to me, could you please elaborate? Changing the icon or label upstream would mean the changes also get pulled into this fork -> no distinguishability achieved. I also don't see where a upstream change is necessary for a simple color change...

The upstream PR would add the functionality to change the icon, and not change the icon.

konradmoesch commented 4 months ago

Why would any upstream functionality to change a icon be needed? This can easily be done using xml files, as the one changed line shows.

I dont get wgat you mean by "generalized view of to any GMS apps". revanced-microg is only used by revanced (and forks), upstream microg can not be used. But this is forked by upstream microg and therefore of course has to take it into account. Could you please elaborate what you mean by "just to satisfy one kind of GMS"? Changing the icon to differ from upstream would of course maily satisfy upstream. But it also satisfies any other kinds of GMS, except for that they would change to do exactly the same change. But why would they? You could then also say that changing the basePackage does not satisfy anything, since any other kind of GMS could also decide to 'app.revanced'. Please elaborate, thanks

oSumAtrIX commented 4 months ago

Why would any upstream functionality to change a icon be needed?

To satisfy using a different icon specific to any situation.

I dont get wgat you mean by "generalized view of to any GMS apps"

"Applicable to any situation involving any GMS reimplementation or real GMS"

Could you please elaborate what you mean by "just to satisfy one kind of GMS"

You take account for upstream GmsCore and only that. Upstream GmsCore is only one kind.

But why would they?

For the same reason you attempt to

since any other kind of GMS could also decide to 'app.revanced'.

Exactly. For the same reason upstream clashes with the fork in this current case, the same way a different case would clash with the package name. Just changing the package name won't fix this problem, because of the same reason as with the icon, you only look at one situation

konradmoesch commented 4 months ago

You take account for upstream GmsCore and only that. Upstream GmsCore is only one kind.

No, every GmsCore (upstream microG, official google, other microg forks) would be different from your "revanced branded" GmsCore settings icon. So, changing the settings icon background color (ideally also using a revanced-branded icon) would clearly state "this is the revanced GMS settings app" and would therefore be

"Applicable to any situation involving any GMS reimplementation or real GMS"

, since any other GMS reimplementation would use their own icon. It is not needed to be

specific to any situation

The only issue is that revanced-GMS is not branded, and therefore not able to be run "in parallel to other GMS installations" in a, from the users' perspective, comfortable way.

This is not a new issue, forks of any app are faced with having to change their apps' icon to reflect that they are used for a different purpose. This does not mean any forkable open-source app has to support icon changes - a fork can simply do this via some adjustments.

For the same reason you attempt to

what do you mean with "my reasons"? I simply want to run upstream microg and revanced microg next to each other - without confusing the upstream settings app (which I open quite often to configure FCM, for example) with the revanced one (which a user never has to open manually, even google log-in is done via a dialog)

Exactly. For the same reason upstream clashes with the fork in this current case, the same way a different case would clash with the package name. Just changing the package name won't fix this problem, because of the same reason as with the icon, you only look at one situation

What do you mean? IIRC, upstream never decided to also change their package name to app.revanced -> no clash there, also not currently

I have a honest question: I get that, as always, fixes for bugs caused by upstream code are not right here in this fork. You also want to focus on supporting revanced yt apps and don't want to add other features. This is a good decision to help keep maintenance work low. But, you also want to be able to coexist with other GMS impls. Great, thanks for that! But being distinguishable in the launcher is one important part of that coexistance. I think, you will agree on that, right?

So, what is the reason for the strong opposition? Is it maintainability? One changed line in one single commit, I believe this can easily be rebased onto upstream changes.

Do you want me to change something? I would also be willing to add a fully "revanced branded" settings icon, which further shows the difference. This PR includes the least needed change to make the apps distinguishable and thus improves the UX of "Run[ning] next to an existing GMS installation such as microG GmsCore or Google Play Services."

Please help me understand your motives, I believe we both have the same goal of creating a coexistance of GMSCore implementations (including a good UX, or, at least, making revanced microg settings any usable). Thanks

oSumAtrIX commented 4 months ago

would clearly state "this is the revanced GMS settings app" and would therefore be

We don't reserve any color. Any other GMS could use it as well. The only thing this solves is not clashing with upstream GMS and not with any other.

not able to be run "in parallel to other GMS installations"

That is not true as it does run in parallel to other GMS installations, I have already clarified multiple times that any other GMS installation could have the same color as you suggested in this PR and "invoke" a new PR asking to solve the clash. You only solve this problem for upstream.

what do you mean with "my reasons"

I said "for the same reason" and not "your reasons".

upstream never decided to also change their package name to app.revanced -> no clash there, also not currently

Good for upstream. Yet another GMS installation can, not excluding upstream.

distinguishable in the launcher is one important part of that coexistance

This is a specific situation and I have clarified already that the fork can't take any stance on any specific situation like that. The view must be general. In your specific case, the problem is not the fork not changing the logo to upstream, but your launcher not being able to distinguish between them. Thus, you must solve the problem at that end. For example by renaming them in the launcher or similar. Be creative.

So, what is the reason for the strong opposition Please help me understand your motives

It is incorrect design to change the logo just to suit a specific scenario, which for you is running it next to upstream. The fork can clash with any other GMS installation regardless of what logo you chose which is why just supporting one scenario doesn't really motivate any change to the logo. Your problem doesn't originate here at ReVanced, it originates at how you use our fork and for that reason you need to fix the problem wherever it originated. Changing the app icon in your launcher is one way to correctly handle your situation, as your situation is invoked by you installing the fork and upstream at the same time. If your launcher does not support this, it is an issue with the launcher and not the fork

konradmoesch commented 4 months ago

We don't reserve any color. Any other GMS could use it as well. The only thing this solves is not clashing with upstream GMS and not with any other.

Yes, if you want to think negative, you can. But than, in my opinion, you could also start saying "any commit adapting microg gms to revanced is irrelevant, they could change something and make it unusable"

That is not true as it does run in parallel to other GMS installations, I have already clarified multiple times that any other GMS installation could have the same color as you suggested in this PR and "invoke" a new PR asking to solve the clash. You only solve this problem for upstream.

No color is reserved, yes. But a conflict is solved and someone can always decide to choose exactly the same color. Why would some fork itentionally create such a new conflict?

upstream never decided to also change their package name to app.revanced -> no clash there, also not currently

Good for upstream. Yet another GMS installation can, not excluding upstream.

This would again mean a change that would only occur on purpose - why would anyone do that? Do you want to stop changing the package name now?

It is incorrect design to change the logo just to suit a specific scenario, which for you is running it next to upstream. The fork can clash with any other GMS installation regardless of what logo you chose which is why just supporting one scenario doesn't really motivate any change to the logo. Your problem doesn't originate here at ReVanced, it originates at how you use our fork and for that reason you need to fix the problem wherever it originated. Changing the app icon in your launcher is one way to correctly handle your situation, as your situation is invoked by you installing the fork and upstream at the same time. If your launcher does not support this, it is an issue with the launcher and not the fork

I find it quite funny that you name it a specific scenario, as if it would be something special. This is literally what you list in your goal, coexistence which other GmsCore implementations, such as (upstream) microG.

If the launcher and thus the UI, is only such a small, niche part of the coexistence, although the user facing one, what is the coexistence about then? App icon and label being the first thing that the user sees, I can hardly believe that it is such a specific, small issue, and I am the only one facing the problem.

Do you want to suggest that I try creating a PR for my launcher to support changing an apps' icon and/or label, because it (seemingly intentionally) uses some other apps' one? Then, the step to saying "patch android that you can rename package names" and removing this change from this fork is not a big step imo.

Please help me understand your strong opposition, what disadvantages do you get by making Revanced/GmsCore destinct from others, thus improving coexistence? I really want to understand why you are so strongly against this simple change. Given this heated response, I start feeling that you don't want to include any contributions, but I do not understand why. One could simply be against any changes and close any PRs. But one could also discuss the motives behind a proposed change, how to improve, alternative options or which changes to further include,... In my opinion, discussing users' issues or proposed changes benefits us all - even if no change gets merged, you know about problems, even if it's only in the back of your mind when working on something else, and we know about your motives

What is your motivation? Simply maintanability?

I want to thank you, however, for your answers so far, although for me, those still don't make much sense. For me, the decision of only changing the package name (could still clash, too), but not any user facing thing, seems to be chosen quite arbitrarily. I'd like to understand the motives. Thanks

oSumAtrIX commented 4 months ago

fork itentionally create such a new conflict

We didn't. It is a byproduct of being a fork. Changing the colour doesn't fix the issue.

Do you want to stop changing the package name now?

If another fork decides to use the same package (their full right to), we again have a conflict, further showing that there's no point in even bothering to do that.

This is literally what you list in your goal, coexistence which other GmsCore implementations, such as (upstream) microG.

Yes, "other GmsCore implementations" is a general view whereas upstream is a specific one. Just satisfying the specific one doesn't satisfy the general view.

creating a PR for my launcher to support changing an apps' icon and/or label

Absolutely. A launchers job is to display apps and launch them. It's primary purpose is that and therefor it's the launchers job to handle conflicting, specific situations, such as this one, because it has a view on your current specific situation, but this fork doesn't.

What is your motivation

Like I previously said:

It is incorrect design to change the logo [..]

Previous forks that attempted to realize what this fork does were doomed to die because of so many unnecessary changes that merging upstream became impossible feat. This is why the goal of this fork is strongly enforced. It's primary and only purpose is to run side by side an existing GMS installation and enable modified applications to be used with GMS.