search

googlesamples / unity-jar-resolver

Unity plugin which resolves Android & iOS dependencies and performs version management
Other
1.22k stars 339 forks source link

Issues when saving analytics settings to GvhProjectSettings.xml #387

Open ricardobustaw opened 4 years ago

ricardobustaw commented 4 years ago

Please fill in the following fields:

Unity editor version: 2018.4.23f1 External Dependency Manager version: 1.2.156 Source you installed EDM (from .unitypackage or Unity Package Manager): .unitypackage Features in External Dependency Manager in use (Android Resolver, iOS Resolver, VersionHandler, etc.): Android Resolver, iOS Resolver, VersionHandler Plugins SDK in use (Firebase, Admob, Facebook, etc.): Firebase, Facebook Platform you are using the Unity editor on (Mac, Windows, or Linux): Mac Platform you are targeting (iOS, Android, and/or desktop): Android Scripting Runtime (Mono, and/or IL2CPP): IL2CPP

Please describe the issue here:

Analytics settings are not being properly saved. After the popup shows up, if you select "No" it will add a line like this: <projectSetting /> The same thing happens if you change the settings for "Yes" then change it back to "No".

Even if values are committed to the repository, other users will still see both popups (For the dependency manager analytics and for firebase analytics).

image

image

image

Please answer the following, if applicable:

What's the issue repro rate? (eg 100%, 1/5 etc) 100%

chkuang-g commented 4 years ago

@ricardobustaw

Sorry for the confusion. The consent prompts per machine per module. The flag to determine whether to show the popup is stored in system level, not in GvhProjectSettings.xml. What stored in GvhProjectSettings.xml is whether the analytics is enabled or not.

https://github.com/googlesamples/unity-jar-resolver/blob/master/source/VersionHandlerImpl/src/EditorMeasurement.cs#L149

I am not sure what that extra line of <projectSetting /> is for but as long as AnalyticsEnabled flag is set, you are good to go.

Does this answer your question?

ricardobustaw commented 4 years ago

@ricardobustaw

Sorry for the confusion. The consent prompts per machine per module. The flag to determine whether to show the popup is stored in system level, not in GvhProjectSettings.xml. What stored in GvhProjectSettings.xml is whether the analytics is enabled or not.

https://github.com/googlesamples/unity-jar-resolver/blob/master/source/VersionHandlerImpl/src/EditorMeasurement.cs#L149

I am not sure what that extra line of <projectSetting /> is for but as long as AnalyticsEnabled flag is set, you are good to go.

Does this answer your question?

Hi

About that being by machine, I think there should be a way to disable it, both for remote build machines that have the pop-up being displayed after we updated the resolver, and also because that will show to everyone that updates their branches to the latest version, including artists and designers.

About the empty tag, that seems to be a bug. If you click no and the settings file already has a no value, it fills it with an empty tag. Same happens if you toggle it on and back to off again. Not just for me, but for every member of the project.

Which adds the risk of someone committing the broken settings file by accident.

There must be a way to configure that as a company, and not as a user/machine. Mainly if the compliance is with "no", since nothing is tracked without permission. It could read settings file information, and if already set, avoid showing the pop-up.

Thanks! Hope we can help if more information is needed.

alexames commented 4 years ago

Hi,

I've created an issue to our internal issue tracker for this feature request (b/161486820). Unfortunately right now I don't have a timeframe for when we'll be able to get to this issue.

If you want a workaround in the meantime, one kind of hacky way would be to just change the line of code here so that it doesn't create the dialog at all. Or, you can change the ConsentRequired property here to pass the value SettingLocation.Project so that it will be saved with the rest of the project settings when you check it in.

However, making one of these changes means you'll have to re-apply your patch any time you upgrade Firebase.

ricardobustaw commented 4 years ago

Thanks! We'll see if it's worth doing the workaround here. If we also think on some solution for that, we might create a PR. But I guess that depends a lot on the direction you want to go with the configurations.

chkuang-g commented 3 years ago

I think what we can do is to add a project level flag like "Disable Analytics Consent", false by default, which can be stored in GvhProjectSettings.xml

Although there should not be a concern but we will go through privacy review for this as well. Will keep you updated