Skip to content

Commit 950379f

Browse files
committed
Fixed dialog window may be cleared when Unity reload assemblies.
When Unity reload assemblies, opened EditorWindow will remain on the screen but the content of the EditorWindow may be clear if the data cannot be serialized, ex. delegate. This fix alleviate the issue with the following approaches: * Close the dialog before Unity reload assemblies. * Try to postpone analytics event reporting after Unity finishes compiling. Change-Id: I6f06a8aeb75af1f9afd694e4e8828e437a29c11d
1 parent f162d81 commit 950379f

File tree

5 files changed

+202
-8
lines changed

5 files changed

+202
-8
lines changed

source/VersionHandler/src/VersionHandler.cs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,87 @@ public static object InvokeMethod(
554554
}
555555
return foundMethod.Invoke(objectInstance, parameterValues);
556556
}
557+
558+
/// <summary>
559+
/// Call a method on a static event.
560+
/// </summary>
561+
/// <param name="type">Class to call the method on.</param>
562+
/// <param name="eventName">The name of the event.</param>
563+
/// <param name="action">Action to add/remove from the event.</param>
564+
/// <param name="getMethod">The func to get the method on the event.</param>
565+
/// <returns>True if the method is called successfully.</returns>
566+
private static bool InvokeStaticEventMethod(Type type, string eventName, Action action,
567+
Func<EventInfo, MethodInfo> getMethod) {
568+
EventInfo eventInfo = type.GetEvent(eventName);
569+
if (eventInfo != null) {
570+
MethodInfo method = getMethod(eventInfo);
571+
Delegate d = Delegate.CreateDelegate(
572+
eventInfo.EventHandlerType, action.Target, action.Method);
573+
System.Object[] args = { d };
574+
if (method.IsStatic) {
575+
method.Invoke(null, args);
576+
return true;
577+
}
578+
}
579+
return false;
580+
}
581+
582+
/// <summary>
583+
/// Call adder method on a static event.
584+
/// </summary>
585+
/// <param name="type">Class to call the method on.</param>
586+
/// <param name="eventName">The name of the event.</param>
587+
/// <param name="action">Action to add from the event.</param>
588+
/// <returns>True if the action is added successfully.</returns>
589+
public static bool InvokeStaticEventAddMethod(Type type, string eventName, Action action) {
590+
return InvokeStaticEventMethod(type, eventName, action,
591+
eventInfo => eventInfo.GetAddMethod());
592+
}
593+
594+
/// <summary>
595+
/// Call remover method on a static event.
596+
/// </summary>
597+
/// <param name="type">Class to call the method on.</param>
598+
/// <param name="eventName">The name of the event.</param>
599+
/// <param name="action">Action to remove from the event.</param>
600+
/// <returns>True if the action is removed successfully.</returns>
601+
public static bool InvokeStaticEventRemoveMethod(Type type, string eventName, Action action) {
602+
return InvokeStaticEventMethod(type, eventName, action,
603+
eventInfo => eventInfo.GetRemoveMethod());
604+
}
605+
606+
// Name of UnityEditor.AssemblyReloadEvents.beforeAssemblyReload event.
607+
private const string BeforeAssemblyReloadEventName = "beforeAssemblyReload";
608+
609+
/// <summary>
610+
/// Register for beforeAssemblyReload event.
611+
/// Note that AssemblyReloadEvents is only availabe from Unity 2017.
612+
/// </summary>
613+
/// <param name="action">Action to register for.</param>
614+
/// <returns>True if the action is registered successfully.</returns>
615+
public static bool RegisterBeforeAssemblyReloadEvent(Action action) {
616+
Type eventType = VersionHandler.FindClass("UnityEditor",
617+
"UnityEditor.AssemblyReloadEvents");
618+
if (eventType != null) {
619+
return InvokeStaticEventAddMethod(eventType, BeforeAssemblyReloadEventName, action);
620+
}
621+
return false;
622+
}
623+
624+
/// <summary>
625+
/// Unregister for beforeAssemblyReload event.
626+
/// Note that AssemblyReloadEvents is only availabe from Unity 2017.
627+
/// </summary>
628+
/// <param name="action">Action to unregister for.</param>
629+
/// <returns>True if the action is unregistered successfully.</returns>
630+
public static bool UnregisterBeforeAssemblyReloadEvent(Action action) {
631+
Type eventType = VersionHandler.FindClass("UnityEditor",
632+
"UnityEditor.AssemblyReloadEvents");
633+
if (eventType != null) {
634+
return InvokeStaticEventRemoveMethod(eventType, BeforeAssemblyReloadEventName, action);
635+
}
636+
return false;
637+
}
557638
}
558639

559640
} // namespace Google

source/VersionHandler/test/reflection/Assets/PlayServicesResolver/Editor/TestReflection.cs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
public class Greeter {
2626
private string name;
2727

28+
public event Action instanceEvent;
29+
30+
static public event Action staticEvent;
31+
2832
public Greeter(string name) {
2933
this.name = name;
3034
}
@@ -77,6 +81,18 @@ public string HelloWithCustomerNameAndPronoun(string customerName,
7781
return Greeter.GenericHelloWithCustomerNameAndPronoun(customerName,
7882
pronoun: pronoun) + MyNameIs();
7983
}
84+
85+
public static void InvokeStaticEvent() {
86+
if (staticEvent != null) {
87+
staticEvent.Invoke();
88+
}
89+
}
90+
91+
public void InvokeInstanceEvent() {
92+
if (instanceEvent != null) {
93+
instanceEvent.Invoke();
94+
}
95+
}
8096
}
8197

8298
[UnityEditor.InitializeOnLoad]
@@ -105,6 +121,7 @@ static TestReflection() {
105121
TestInvokeInstanceMethodWithNoArgs,
106122
TestInvokeInstanceMethodWithNamedArgDefault,
107123
TestInvokeInstanceMethodWithNamedArg,
124+
TestInvokeStaticEventMethod,
108125
}) {
109126
var testName = test.Method.Name;
110127
Exception exception = null;
@@ -257,4 +274,61 @@ static bool TestInvokeInstanceMethodWithNamedArg() {
257274
new object[] { "Smith" },
258275
new Dictionary<string, object> { { "pronoun", "Mrs" } }));
259276
}
277+
278+
// Check if the delegate is properly added/removed from the given event using the function to
279+
// test.
280+
static bool CheckEvent(Func<Action, bool> funcToTest, Action invokeEvent,
281+
bool expectSuccess, bool expectInvoked) {
282+
283+
bool invoked = false;
284+
285+
Action actionToInvoke = () => {
286+
invoked = true;
287+
};
288+
289+
bool result = funcToTest(actionToInvoke);
290+
if (result != expectSuccess){
291+
throw new Exception(
292+
String.Format("Expect funcToTest returns '{0}', but actually returned '{1}'",
293+
expectSuccess, result));
294+
}
295+
invokeEvent();
296+
if ( invoked != expectInvoked) {
297+
throw new Exception(String.Format(
298+
"Expect event invoked: {0}, but actually event invoked: {1}",
299+
expectInvoked, invoked));
300+
}
301+
302+
return true;
303+
}
304+
305+
// Test adding/removing delegate to a static event.
306+
static bool TestInvokeStaticEventMethod() {
307+
CheckEvent( funcToTest: (action) =>
308+
VersionHandler.InvokeStaticEventAddMethod(typeof(Greeter), "staticEvent", action),
309+
invokeEvent: Greeter.InvokeStaticEvent,
310+
expectSuccess: true,
311+
expectInvoked: true);
312+
313+
CheckEvent( funcToTest: (action) =>
314+
VersionHandler.InvokeStaticEventRemoveMethod(typeof(Greeter), "staticEvent",
315+
action),
316+
invokeEvent: Greeter.InvokeStaticEvent,
317+
expectSuccess: true,
318+
expectInvoked: false);
319+
320+
CheckEvent( funcToTest: (action) =>
321+
VersionHandler.InvokeStaticEventAddMethod(typeof(Greeter), "foo", action),
322+
invokeEvent: Greeter.InvokeStaticEvent,
323+
expectSuccess: false,
324+
expectInvoked: false);
325+
326+
CheckEvent( funcToTest: (action) =>
327+
VersionHandler.InvokeStaticEventRemoveMethod(typeof(Greeter), "foo", action),
328+
invokeEvent: Greeter.InvokeStaticEvent,
329+
expectSuccess: false,
330+
expectInvoked: false);
331+
332+
return true;
333+
}
260334
}

source/VersionHandlerImpl/src/DialogWindow.cs

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ namespace Google {
2626
/// The dialog will not block the editor. When multiple dialogs are triggered, they will be queued
2727
/// and only be shown one at a time based on the triggering order.
2828
/// </summary>
29-
[Serializable]
3029
public class DialogWindow : EditorWindow {
3130
/// <summary>
3231
/// Option selected by the dialog.
@@ -62,9 +61,7 @@ public enum Option {
6261
/// All the data to render the content of the dialog and react to the user interaction.
6362
/// All the context should be serialable/deserialable so that all the context can be reloaded
6463
/// after Unity hot reloads.
65-
/// TODO: Make Action serializable.
6664
/// </summary>
67-
[Serializable]
6865
private class DialogContext {
6966
// Title of the dialog.
7067
internal string Title;
@@ -176,12 +173,14 @@ static private GUIStyle DefaultMessageStyle {
176173
static private GUIStyle defaultMessageStyle;
177174

178175
// Context for this dialog window.
179-
[SerializeField]
180176
private DialogContext dialogContext = new DialogContext();
181177

182178
// The option selected by the user.
183179
private Option selectedOption = Option.SelectedNone;
184180

181+
// Whether this window is terminating due to Unity hot reload.
182+
private bool terminating = false;
183+
185184
/// <summary>
186185
/// Displays a non-blocking modal dialog with up to 3 options.
187186
/// </summary>
@@ -325,6 +324,19 @@ public static void Display(string title, string message, Option defaultOption,
325324
/// Render the dialog according to the context.
326325
/// </summary>
327326
void OnGUI() {
327+
// Close the window if Option0String is empty.
328+
// After Unity reload assemblies, the EditorWindow will remain open but all the content
329+
// in the dialog will be cleared because dialogContext is not serializable. Therefore,
330+
// close the dialog after assembly reload. Close in the next editor frame or it may
331+
// generate error message like "OpenGL Context became invalid during rendering".
332+
// This is for Unity 5.
333+
if (String.IsNullOrEmpty(dialogContext.Option0String) && !terminating) {
334+
terminating = true;
335+
RunOnMainThread.Run(() => {
336+
Close();
337+
}, runNow: false);
338+
}
339+
328340
Rect rect = EditorGUILayout.BeginVertical();
329341

330342
// Render the dialog message.
@@ -418,5 +430,26 @@ void OnDestroy() {
418430
// Complete the current dialog and display the next one if there is any in the queue.
419431
dialogJobQueue.Complete();
420432
}
421-
} // class DialogWindow
433+
434+
// Function called when the dialog window is enabled.
435+
void OnEnable() {
436+
VersionHandler.RegisterBeforeAssemblyReloadEvent(OnBeforeAssemblyReload);
437+
}
438+
439+
// Function called when the dialog window is disabled.
440+
void OnDisable() {
441+
VersionHandler.UnregisterBeforeAssemblyReloadEvent(OnBeforeAssemblyReload);
442+
}
443+
444+
// Function called before assemblies will be reloaded.
445+
private void OnBeforeAssemblyReload() {
446+
// Close the window before assembly will be reloaded.
447+
// After Unity reload assemblies, the EditorWindow will remain open but all the content
448+
// in the dialog will be cleared because all Action are not serialiable. Therefore,
449+
// close the dialog before assembly reload.
450+
// Note that this only works from Unity 2017 since any version below does not have
451+
// the event API.
452+
Close();
453+
}
454+
} // class NonBlockingDialog
422455
} // namespace Google

source/VersionHandlerImpl/src/EditorMeasurement.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@ private set {
154154
}
155155
}
156156

157+
// Whether consent dialog has been displayed.
158+
private bool ConsentRequesting = false;
159+
157160
/// <summary>
158161
/// Project level cookie which enables reporting of unique projects.
159162
/// Empty string if analytics is disabled.
@@ -352,9 +355,10 @@ private static string GetAndCacheUnityRuntimePlatform() {
352355
/// Ask user to enable analytics.
353356
/// </summary>
354357
public void PromptToEnable(Action complete) {
355-
if (ConsentRequested) {
358+
if (ConsentRequesting || ConsentRequested) {
356359
complete();
357360
} else {
361+
ConsentRequesting = true;
358362
displayDialog(
359363
String.Format(EnableAnalytics, PluginName),
360364
String.Format(RequestConsentMessage, PluginName),
@@ -369,6 +373,8 @@ public void PromptToEnable(Action complete) {
369373
Enabled = false;
370374
break;
371375
}
376+
ConsentRequesting = false;
377+
ConsentRequested = true;
372378
complete();
373379
}, renderContent: dialog => {
374380
GUILayout.Label(RequestConsentDataCollection,
@@ -406,7 +412,6 @@ public void PromptToEnable(Action complete) {
406412
}
407413
EditorGUILayout.Space();
408414
});
409-
ConsentRequested = true;
410415
}
411416
}
412417

source/VersionHandlerImpl/src/VersionHandlerImpl.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2423,6 +2423,8 @@ private static void NotifyWhenCompliationComplete(bool forceNotification) {
24232423
setCleanupFilesPending: false);
24242424
}, runNow: false);
24252425
}
2426+
2427+
analytics.Report("enablemostrecentplugins", "Enable Most Recent Plugins");
24262428
NotifyUpdateCompleteMethods();
24272429
}
24282430
return true;
@@ -2824,7 +2826,6 @@ private static void UpdateVersionedAssetsOnMainThread(bool forceUpdate,
28242826
ManifestReferences.FindAndReadManifests(allMetadataSet), allMetadataSet);
28252827
if (metadataSet.EnableMostRecentPlugins(forceUpdate, obsoleteFiles.All)) {
28262828
enabledEditorDlls.UnionWith(metadataSet.EnabledEditorDlls);
2827-
analytics.Report("enablemostrecentplugins", "Enable Most Recent Plugins");
28282829
AssetDatabase.Refresh();
28292830
Refreshing = true;
28302831
}

0 commit comments

Comments
 (0)