Skip to content

Commit a8cbe50

Browse files
author
Stewart Miles
committed
Removed use of EditorUtility.DisplayProgressBar in Android Resolver
EditorUtility.DisplayProgressBar() is very flakey, it can end up in odd states where it can never be cleared. This removes the use of EditorUtility.DisplayProgressBar from the Android Resolver, instead moving the progress bar display to the CommandLineDialog. In addition, long running operations like labeling assets, have been made asynchronous by scheduling each iteration on the main thread. Finally, this leaves the Android Resolver window open if resolution fails, results in one or more warnings or verbose logging is enabled so that it's easier to inspect the failure logs. Bug: 132070706 Change-Id: I31ed32659d3871f501aab7305cbfe4df99613e89
1 parent 7211da4 commit a8cbe50

File tree

8 files changed

+346
-154
lines changed

8 files changed

+346
-154
lines changed

source/PlayServicesResolver/src/CommandLineDialog.cs

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,6 @@ public void Update(CommandLineDialog window)
148148
bodyText = bodyTextHead + bodyText.Substring(carriageReturn + 1);
149149
}
150150
window.bodyText = bodyText;
151-
if (window.autoScrollToBottom)
152-
{
153-
window.scrollPosition.y = Mathf.Infinity;
154-
}
155151
window.Repaint();
156152
}
157153
if (maxProgressLines > 0)
@@ -169,18 +165,20 @@ public void Update(CommandLineDialog window)
169165
public volatile float progress;
170166
public string progressTitle;
171167
public string progressSummary;
172-
public volatile bool autoScrollToBottom;
173168
public Google.Logger logger = new Google.Logger();
174169

170+
/// <summary>
171+
/// Whether a command is currently being executed.
172+
/// </summary>
173+
public bool RunningCommand { protected set; get; }
174+
175175
/// <summary>
176176
/// Event delegate called from the Update() method of the window.
177177
/// </summary>
178178
public delegate void UpdateDelegate(CommandLineDialog window);
179179

180180
public event UpdateDelegate UpdateEvent;
181181

182-
private bool progressBarVisible;
183-
184182
/// <summary>
185183
/// Create a dialog box which can display command line output.
186184
/// </summary>
@@ -212,15 +210,6 @@ public static CommandLineDialog CreateCommandLineDialog(string title)
212210
}
213211
}
214212

215-
/// <summary>
216-
/// Alternative Repaint() method that does not crash in batch mode.
217-
/// </summary>
218-
public new void Repaint() {
219-
if (!ExecutionEnvironment.InBatchMode) {
220-
base.Repaint();
221-
}
222-
}
223-
224213
/// <summary>
225214
/// Alternative Close() method that does not crash in batch mode.
226215
/// </summary>
@@ -240,10 +229,42 @@ public override void Initialize()
240229
progressTitle = "";
241230
progressSummary = "";
242231
UpdateEvent = null;
243-
progressBarVisible = false;
244232
autoScrollToBottom = false;
245233
}
246234

235+
/// <summary>
236+
/// Set the progress bar status.
237+
/// </summary>
238+
/// <param name="title">Text to display before the progress bar.</param>
239+
/// <param name="value">Progress bar value 0..1.</param>
240+
/// <param name="summary">Text to display in the progress bar.</param>
241+
public void SetProgress(string title, float value, string summary) {
242+
progressTitle = title;
243+
progress = value;
244+
progressSummary = summary;
245+
Repaint();
246+
}
247+
248+
// Draw the GUI with an optional status bar.
249+
protected override void OnGUI() {
250+
summaryTextDisplay = true;
251+
if (!String.IsNullOrEmpty(progressTitle)) {
252+
summaryTextDisplay = false;
253+
EditorGUILayout.BeginVertical();
254+
EditorGUILayout.LabelField(progressTitle, EditorStyles.boldLabel);
255+
var progressBarRect = EditorGUILayout.BeginVertical();
256+
EditorGUILayout.LabelField(""); // Creates vertical space for the progress bar.
257+
EditorGUI.ProgressBar(
258+
progressBarRect, progress,
259+
String.IsNullOrEmpty(progressSummary) ?
260+
String.Format("{0}%... ", (int)(progress * 100.0f)) : progressSummary);
261+
EditorGUILayout.EndVertical();
262+
EditorGUILayout.Space();
263+
EditorGUILayout.EndVertical();
264+
}
265+
base.OnGUI();
266+
}
267+
247268
/// <summary>
248269
/// Asynchronously execute a command line tool in this window, showing progress
249270
/// and finally calling the specified delegate on completion from the main / UI thread.
@@ -273,38 +294,29 @@ public void RunAsync(
273294
// Connect the caller's IoHandler delegate to the reporter.
274295
reporter.DataHandler += ioHandler;
275296
// Disconnect the reporter when the command completes.
276-
CommandLine.CompletionHandler reporterUpdateDisable =
277-
(CommandLine.Result unusedResult) => { this.UpdateEvent -= reporter.Update; };
297+
CommandLine.CompletionHandler reporterUpdateDisable = (unusedResult) => {
298+
RunningCommand = false;
299+
this.UpdateEvent -= reporter.Update;
300+
};
278301
reporter.Complete += reporterUpdateDisable;
279302
logger.Log(String.Format(
280303
"Executing command: {0} {1}", toolPath, arguments), level: LogLevel.Verbose);
304+
RunningCommand = true;
281305
CommandLine.RunAsync(toolPath, arguments, reporter.CommandLineToolCompletion,
282306
workingDirectory: workingDirectory, envVars: envVars,
283307
ioHandler: reporter.AggregateLine);
284308
}
285309

286310
/// <summary>
287-
/// Call the update event from the UI thread, optionally display / hide the progress bar.
311+
/// Call the update event from the UI thread.
288312
/// </summary>
289313
protected virtual void Update()
290314
{
291315
if (UpdateEvent != null) UpdateEvent(this);
292-
if (progressTitle != "")
293-
{
294-
progressBarVisible = true;
295-
EditorUtility.DisplayProgressBar(progressTitle, progressSummary,
296-
progress);
297-
}
298-
else if (progressBarVisible)
299-
{
300-
progressBarVisible = false;
301-
EditorUtility.ClearProgressBar();
302-
}
303316
}
304317

305318
// Hide the progress bar if the window is closed.
306319
protected override void OnDestroy() {
307-
if (progressBarVisible) EditorUtility.ClearProgressBar();
308320
base.OnDestroy();
309321
}
310322
}

source/PlayServicesResolver/src/DefaultResolver.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ internal AndroidAbis AarDirectoryFindAbis(string aarDirectory) {
262262
/// to repack the processed AAR as a new AAR.</param>
263263
/// <param name="abis">ABIs in the AAR or null if it's universal.</param>
264264
/// <returns>true if successful, false otherwise.</returns>
265-
internal virtual bool ProcessAar(string dir, string aarFile, bool antProject,
266-
out AndroidAbis abis) {
265+
internal bool ProcessAar(string dir, string aarFile, bool antProject,
266+
out AndroidAbis abis) {
267267
PlayServicesResolver.Log(String.Format("ProcessAar {0} {1} antProject={2}",
268268
dir, aarFile, antProject),
269269
level: LogLevel.Verbose);

source/PlayServicesResolver/src/GradleTemplateResolver.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,15 @@ private static bool CopySrcAars(ICollection<Dependency> dependencies) {
108108
aarFiles.Add(targetFilename);
109109
if (!File.Exists(targetFilename)) {
110110
bool configuredAar = false;
111-
if (AssetDatabase.CopyAsset(srcaar, targetFilename) &&
111+
bool copiedAndLabeledAar = AssetDatabase.CopyAsset(srcaar, targetFilename);
112+
if (copiedAndLabeledAar) {
113+
var unlabeledAssets = new HashSet<string>();
112114
PlayServicesResolver.LabelAssets(
113-
new [] { targetFilename }).Count == 0) {
115+
new [] { targetFilename },
116+
complete: (unlabeled) => { unlabeledAssets.UnionWith(unlabeled); });
117+
copiedAndLabeledAar = unlabeledAssets.Count == 0;
118+
}
119+
if (copiedAndLabeledAar) {
114120
try {
115121
PluginImporter importer = (PluginImporter)AssetImporter.GetAtPath(
116122
targetFilename);

source/PlayServicesResolver/src/PlayServicesResolver.cs

Lines changed: 73 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,61 +1718,87 @@ internal static void OnSettingsChanged() {
17181718
/// Label a set of assets that should be managed by this plugin.
17191719
/// </summary>
17201720
/// <param name="assetPaths">Set of assets to label.</param>
1721+
/// <param name="complete">Called when the operation is complete with the set of assets
1722+
/// that could not be labeled.</param>
1723+
/// <param name="synchronous">Whether to block until asset labeling is complete.</param>
1724+
/// <param name="progressUpdate">Called with the progress (0..1) and message that indicates
1725+
/// processing progress.</param>
17211726
/// <param name="displayWarning">Whether to display a warning if assets can't be
17221727
/// labeled.</param>
17231728
/// <param name="recursive">Whether to label assets in subdirectories of the specified
17241729
/// assetPaths.</param>
1725-
/// <returns>List of assets that could not be labeled.</returns>
1726-
internal static HashSet<string> LabelAssets(IEnumerable<string> assetPaths,
1727-
bool displayWarning = true,
1728-
bool recursive = false) {
1729-
var assetsWithoutAssetImporter = new HashSet<string>(assetPaths);
1730-
if (assetsWithoutAssetImporter.Count == 0) return assetsWithoutAssetImporter;
1730+
/// <returns></returns>
1731+
internal static void LabelAssets(IEnumerable<string> assetPaths,
1732+
Action<HashSet<string>> complete = null,
1733+
bool synchronous = true,
1734+
Action<float, string> progressUpdate = null,
1735+
bool displayWarning = true,
1736+
bool recursive = false) {
1737+
var assetsWithoutAssetImporter = new HashSet<string>();
17311738
var projectDataFolder = Path.GetFullPath(Application.dataPath);
1732-
foreach (var assetPath in new List<string>(assetsWithoutAssetImporter)) {
1733-
// Ignore asset meta files which are used to store the labels and files that
1734-
// are not in the project.
1735-
var fullAssetPath = Path.GetFullPath(assetPath);
1736-
if (assetPath.EndsWith(".meta") || !fullAssetPath.StartsWith(projectDataFolder)) {
1737-
assetsWithoutAssetImporter.Remove(assetPath);
1738-
continue;
1739-
}
1739+
var assetsToProcess = new List<string>(assetPaths);
1740+
int totalAssets = assetsToProcess.Count;
1741+
RunOnMainThread.PollOnUpdateUntilComplete(() => {
1742+
var remainingAssets = assetsToProcess.Count;
1743+
// Display summary of processing and call the completion function.
1744+
if (remainingAssets == 0) {
1745+
if (assetsWithoutAssetImporter.Count > 0 && displayWarning) {
1746+
Log(String.Format(
1747+
"Failed to add tracking label {0} to some assets.\n\n" +
1748+
"The following files will not be managed by this module:\n" +
1749+
"{1}\n", ManagedAssetLabel,
1750+
String.Join(
1751+
"\n", new List<string>(assetsWithoutAssetImporter).ToArray())),
1752+
level: LogLevel.Warning);
1753+
}
1754+
if (complete != null) complete(assetsWithoutAssetImporter);
1755+
return true;
1756+
}
1757+
var assetPath = assetsToProcess[0];
1758+
assetsToProcess.RemoveAt(0);
1759+
// Ignore asset meta files which are used to store the labels and files that
1760+
// are not in the project.
1761+
var fullAssetPath = Path.GetFullPath(assetPath);
1762+
if (assetPath.EndsWith(".meta") ||
1763+
!fullAssetPath.StartsWith(projectDataFolder)) {
1764+
return false;
1765+
}
17401766

1741-
// Get the relative path of this asset.
1742-
var relativeAssetPath = Path.Combine(
1743-
Path.GetFileName(projectDataFolder),
1744-
fullAssetPath.Substring(projectDataFolder.Length +1));
1767+
// Get the relative path of this asset.
1768+
var relativeAssetPath = Path.Combine(
1769+
Path.GetFileName(projectDataFolder),
1770+
fullAssetPath.Substring(projectDataFolder.Length +1));
17451771

1746-
// If the asset is a directory, add labels to the contents.
1747-
if (recursive && Directory.Exists(relativeAssetPath)) {
1748-
assetsWithoutAssetImporter.UnionWith(
1749-
LabelAssets(Directory.GetFileSystemEntries(relativeAssetPath),
1750-
displayWarning: false));
1751-
}
1772+
if (progressUpdate != null) {
1773+
progressUpdate((float)(totalAssets - remainingAssets) /
1774+
(float)totalAssets, relativeAssetPath);
1775+
}
17521776

1753-
// It's likely files have been added or removed without using AssetDatabase methods
1754-
// so (re)import the asset to make sure it's in the AssetDatabase.
1755-
AssetDatabase.ImportAsset(relativeAssetPath,
1756-
options: ImportAssetOptions.ForceSynchronousImport);
1757-
1758-
// Add the label to the asset.
1759-
AssetImporter importer = AssetImporter.GetAtPath(relativeAssetPath);
1760-
if (importer != null) {
1761-
var labels = new HashSet<string>(AssetDatabase.GetLabels(importer));
1762-
labels.Add(ManagedAssetLabel);
1763-
AssetDatabase.SetLabels(importer, (new List<string>(labels)).ToArray());
1764-
assetsWithoutAssetImporter.Remove(assetPath);
1765-
}
1766-
}
1767-
if (assetsWithoutAssetImporter.Count > 0 && displayWarning) {
1768-
Log(String.Format(
1769-
"Failed to add tracking label {0} to some assets.\n\n" +
1770-
"The following files will not be managed by this module:\n" +
1771-
"{1}\n", ManagedAssetLabel,
1772-
String.Join("\n", new List<string>(assetsWithoutAssetImporter).ToArray())),
1773-
level: LogLevel.Warning);
1774-
}
1775-
return assetsWithoutAssetImporter;
1777+
// If the asset is a directory, add labels to the contents.
1778+
if (recursive && Directory.Exists(relativeAssetPath)) {
1779+
var contents = new List<string>(
1780+
Directory.GetFileSystemEntries(relativeAssetPath));
1781+
totalAssets += contents.Count;
1782+
assetsToProcess.AddRange(contents);
1783+
return false;
1784+
}
1785+
1786+
// It's likely files have been added or removed without using AssetDatabase
1787+
// methods so (re)import the asset to make sure it's in the AssetDatabase.
1788+
AssetDatabase.ImportAsset(relativeAssetPath,
1789+
options: ImportAssetOptions.ForceSynchronousImport);
1790+
1791+
// Add the label to the asset.
1792+
AssetImporter importer = AssetImporter.GetAtPath(relativeAssetPath);
1793+
if (importer != null) {
1794+
var labels = new HashSet<string>(AssetDatabase.GetLabels(importer));
1795+
labels.Add(ManagedAssetLabel);
1796+
AssetDatabase.SetLabels(importer, (new List<string>(labels)).ToArray());
1797+
} else {
1798+
assetsWithoutAssetImporter.Add(assetPath);
1799+
}
1800+
return false;
1801+
}, synchronous: synchronous);
17761802
}
17771803

17781804
/// <summary>

0 commit comments

Comments
 (0)