-
Notifications
You must be signed in to change notification settings - Fork 356
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 project settings resetting on domain reload #678
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Update: 1 month in at my org we're not seeing setting file corruption anymore on Windows and MacOS. |
Hey thanks for the PR, let me get some of our engineering folks to take a look into this. |
Thanks for the fix, and the detailed write up actually explaining what the issue was, that is definitely a weird case so really appreciate that you tracked it down. I will work on getting a release out with this fix. As for building it yourself, it can definitely be a bit finicky and depends on a bunch of tools. I wasn't aware of a Unity 5.6 bias, the tests we have run with Unity 2019, but I do know that Apple Silicon can have issues because it depends on a specific python version which isn't available. |
This resolves #524 by avoiding to needlessly save
GvhProjectSettings.xml
on every domain reload.In all 3 modified files, the line
does 3 things on domain reload because of
[InitializeOnLoad]
:GvhProjectSettings.xml
if not already doneGvhProjectSettings.xml
unconditionallylogger.Level
This PR removes step 2) which is unneeded and the cause of #524.
It's potentially possible that writing
GvhProjectSettings.xml
as a result of user interaction might still in some cases have a race condition. I didn't investigate this. However it's much less problematic than intermittent problems on domain reload.Why the problem happens
The current code is correctly protecting critical sections, including saving
GvhProjectSettings.xml
, withlock (classLock)
. However a problem occurs with parallel asset importing enabled in Unity. In this case there are other Unity Editor processes aside from the "main" one that take care of parallel imports. These other processes each have their own separate instance ofclassLock
.This results in multiple processes reading and writing to
GvhProjectSettings.xml
on domain reload. Eventually one of them reads the file while it's being written and ends up with empty settings fromGoogle.ProjectSettings.Load
which it then re-saves. As the screenshot in #524 shows, when corruption happens it's always one or moreVerboseLoggingEnabled
that remain in the settings.About asset import worker processes
The command line of asset import worker processes looks like:
The
-adb2
argument is unique to asset import worker processes so could be used to identify them as needed.Build on Windows
Binaries here https://github.com/berniegp/unity-jar-resolver/releases/tag/v1.2.179
I could not build the plugin as it stands. The (reverted) commit ff03ffd has the changes I needed to do to
build.gradle
.As far as I can see, the build process prefers Unity 5.6. This version (obviously) isn't supported on Apple Silicon so I tried on Windows.
Steps for successful build:
Not sure if I'm doing something wrong regarding the build?