-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Add custom package settings #5027
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
Changes from all commits
adcb61c
a1b9fce
f63be9d
87e3a11
4771f68
f6d6003
28e561a
19193f4
5fe5365
d04a5d4
6effd7b
38cbadb
df9aaed
525a5d4
87ed122
3a2a46b
1857a44
2a4c25d
73339fa
5deba49
e1da84f
430e253
53d59fe
52b2297
4a2a5b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,171 @@ | ||
using System; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Reflection; | ||
using NUnit.Framework; | ||
using UnityEditor; | ||
using UnityEngine; | ||
using Unity.MLAgents; | ||
using Unity.MLAgents.Editor; | ||
|
||
|
||
namespace MLAgentsExamples.Tests.Settings | ||
{ | ||
[TestFixture] | ||
public class MLAgentsSettingsTests | ||
{ | ||
string EditorBuildSettingsConfigKey = MLAgentsSettingsManager.EditorBuildSettingsConfigKey; | ||
string tempSettingsRootPath = "Assets/ML-Agents/Scripts/Tests/Editor/MLAgentsSettings"; | ||
MLAgentsSettings storedConfigObject; | ||
[SetUp] | ||
public void SetUp() | ||
{ | ||
if (EditorBuildSettings.TryGetConfigObject(EditorBuildSettingsConfigKey, | ||
out MLAgentsSettings settingsAsset)) | ||
{ | ||
if (settingsAsset != null) | ||
{ | ||
storedConfigObject = settingsAsset; | ||
EditorBuildSettings.RemoveConfigObject(EditorBuildSettingsConfigKey); | ||
} | ||
} | ||
MLAgentsSettingsManager.Destroy(); | ||
ClearSettingsAssets(); | ||
} | ||
|
||
[TearDown] | ||
public void TearDown() | ||
{ | ||
if (storedConfigObject != null) | ||
{ | ||
EditorBuildSettings.AddConfigObject(EditorBuildSettingsConfigKey, storedConfigObject, true); | ||
storedConfigObject = null; | ||
} | ||
MLAgentsSettingsManager.Destroy(); | ||
ClearSettingsAssets(); | ||
} | ||
|
||
internal void ClearSettingsAssets() | ||
{ | ||
var assetsGuids = AssetDatabase.FindAssets("t:MLAgentsSettings", new string[] { tempSettingsRootPath }); | ||
foreach (var guid in assetsGuids) | ||
{ | ||
var path = AssetDatabase.GUIDToAssetPath(guid); | ||
AssetDatabase.DeleteAsset(path); | ||
} | ||
} | ||
|
||
[Test] | ||
public void TestMLAgentsSettingsManager() | ||
{ | ||
Assert.AreNotEqual(null, MLAgentsSettingsManager.Settings); | ||
Assert.AreEqual(5004, MLAgentsSettingsManager.Settings.EditorPort); // default port | ||
MLAgentsSettingsManager.Settings.EditorPort = 6000; | ||
Assert.AreEqual(6000, MLAgentsSettingsManager.Settings.EditorPort); | ||
|
||
var settingsObject = ScriptableObject.CreateInstance<MLAgentsSettings>(); | ||
settingsObject.EditorPort = 7000; | ||
var tempSettingsAssetPath = tempSettingsRootPath + "/test.mlagents.settings.asset"; | ||
AssetDatabase.CreateAsset(settingsObject, tempSettingsAssetPath); | ||
EditorBuildSettings.AddConfigObject(EditorBuildSettingsConfigKey, settingsObject, true); | ||
// destroy manager instantiated as a side effect by accessing MLAgentsSettings directly without manager | ||
MLAgentsSettingsManager.Destroy(); | ||
Assert.AreEqual(7000, MLAgentsSettingsManager.Settings.EditorPort); | ||
} | ||
|
||
// A mock class that can invoke private methods/fields in MLAgentsSettingsProvider | ||
internal class MockSettingsProvider | ||
{ | ||
public MLAgentsSettingsProvider Instance | ||
{ | ||
get | ||
{ | ||
return (MLAgentsSettingsProvider)typeof(MLAgentsSettingsProvider).GetField("s_Instance", | ||
BindingFlags.Static | BindingFlags.NonPublic).GetValue(null); | ||
} | ||
} | ||
|
||
public MLAgentsSettings Settings | ||
{ | ||
get | ||
{ | ||
return (MLAgentsSettings)typeof(MLAgentsSettingsProvider).GetField("m_Settings", | ||
BindingFlags.Instance | BindingFlags.NonPublic).GetValue(Instance); | ||
} | ||
} | ||
|
||
public void CreateMLAgentsSettingsProvider() | ||
{ | ||
MLAgentsSettingsProvider.CreateMLAgentsSettingsProvider(); | ||
} | ||
|
||
public void Reinitialize() | ||
{ | ||
var method = typeof(MLAgentsSettingsProvider).GetMethod("Reinitialize", | ||
BindingFlags.Instance | BindingFlags.NonPublic); | ||
method.Invoke(Instance, null); | ||
} | ||
|
||
public string[] FindSettingsInProject() | ||
{ | ||
var method = typeof(MLAgentsSettingsProvider).GetMethod("FindSettingsInProject", | ||
BindingFlags.Static | BindingFlags.NonPublic); | ||
return (string[])method.Invoke(null, null); | ||
} | ||
|
||
public void CreateNewSettingsAsset(string relativePath) | ||
{ | ||
var method = typeof(MLAgentsSettingsProvider).GetMethod("CreateNewSettingsAsset", | ||
BindingFlags.Static | BindingFlags.NonPublic); | ||
method.Invoke(null, new object[] { relativePath }); | ||
} | ||
} | ||
|
||
[Test] | ||
public void TestMLAgentsSettingsProviderCreateAsset() | ||
{ | ||
var mockProvider = new MockSettingsProvider(); | ||
mockProvider.CreateMLAgentsSettingsProvider(); | ||
Assert.AreNotEqual(null, mockProvider.Instance); | ||
|
||
// mimic MLAgentsSettingsProvider.OnActivate() | ||
MLAgentsSettingsManager.OnSettingsChange += mockProvider.Reinitialize; | ||
|
||
mockProvider.Instance.InitializeWithCurrentSettings(); | ||
Assert.AreEqual(0, mockProvider.FindSettingsInProject().Length); | ||
|
||
var tempSettingsAssetPath1 = tempSettingsRootPath + "/test.mlagents.settings.asset"; | ||
mockProvider.CreateNewSettingsAsset(tempSettingsAssetPath1); | ||
Assert.AreEqual(1, mockProvider.FindSettingsInProject().Length); | ||
Assert.AreEqual(5004, mockProvider.Settings.EditorPort); | ||
MLAgentsSettingsManager.Settings.EditorPort = 6000; // change to something not default | ||
// callback should update the field in provider | ||
Assert.AreEqual(6000, mockProvider.Settings.EditorPort); | ||
|
||
var tempSettingsAssetPath2 = tempSettingsRootPath + "/test2.mlagents.settings.asset"; | ||
mockProvider.CreateNewSettingsAsset(tempSettingsAssetPath2); | ||
Assert.AreEqual(2, mockProvider.FindSettingsInProject().Length); | ||
// manager should set to the new (default) one, not the previous modified one | ||
Assert.AreEqual(5004, MLAgentsSettingsManager.Settings.EditorPort); | ||
|
||
// mimic MLAgentsSettingsProvider.OnDeactivate() | ||
MLAgentsSettingsManager.OnSettingsChange -= mockProvider.Reinitialize; | ||
mockProvider.Instance.Dispose(); | ||
} | ||
|
||
[Test] | ||
public void TestMLAgentsSettingsProviderLoadAsset() | ||
{ | ||
var mockProvider = new MockSettingsProvider(); | ||
var tempSettingsAssetPath1 = tempSettingsRootPath + "/test.mlagents.settings.asset"; | ||
mockProvider.CreateNewSettingsAsset(tempSettingsAssetPath1); | ||
MLAgentsSettingsManager.Settings.EditorPort = 8000; // change to something not default | ||
|
||
mockProvider.Instance?.Dispose(); | ||
MLAgentsSettingsManager.Destroy(); | ||
|
||
mockProvider.CreateMLAgentsSettingsProvider(); | ||
Assert.AreEqual(8000, MLAgentsSettingsManager.Settings.EditorPort); | ||
} | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
{ | ||
"name": "Runtime", | ||
"name": "Unity.ML-Agents.DevTests.Runtime", | ||
"references": [ | ||
"Unity.ML-Agents" | ||
], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
using System.Linq; | ||
using UnityEngine; | ||
using UnityEditor; | ||
using UnityEditor.Build; | ||
using UnityEditor.Build.Reporting; | ||
|
||
|
||
namespace Unity.MLAgents.Editor | ||
{ | ||
internal class MLAgentsSettingsBuildProvider : IPreprocessBuildWithReport, IPostprocessBuildWithReport | ||
{ | ||
private MLAgentsSettings m_SettingsAddedToPreloadedAssets; | ||
|
||
public int callbackOrder => 0; | ||
|
||
public void OnPreprocessBuild(BuildReport report) | ||
{ | ||
var wasDirty = IsPlayerSettingsDirty(); | ||
m_SettingsAddedToPreloadedAssets = null; | ||
|
||
var preloadedAssets = PlayerSettings.GetPreloadedAssets().ToList(); | ||
if (!preloadedAssets.Contains(MLAgentsSettingsManager.Settings)) | ||
{ | ||
m_SettingsAddedToPreloadedAssets = MLAgentsSettingsManager.Settings; | ||
preloadedAssets.Add(m_SettingsAddedToPreloadedAssets); | ||
PlayerSettings.SetPreloadedAssets(preloadedAssets.ToArray()); | ||
} | ||
|
||
if (!wasDirty) | ||
ClearPlayerSettingsDirtyFlag(); | ||
} | ||
|
||
public void OnPostprocessBuild(BuildReport report) | ||
{ | ||
if (m_SettingsAddedToPreloadedAssets == null) | ||
return; | ||
|
||
var wasDirty = IsPlayerSettingsDirty(); | ||
|
||
var preloadedAssets = PlayerSettings.GetPreloadedAssets().ToList(); | ||
if (preloadedAssets.Contains(m_SettingsAddedToPreloadedAssets)) | ||
{ | ||
preloadedAssets.Remove(m_SettingsAddedToPreloadedAssets); | ||
PlayerSettings.SetPreloadedAssets(preloadedAssets.ToArray()); | ||
} | ||
|
||
m_SettingsAddedToPreloadedAssets = null; | ||
|
||
if (!wasDirty) | ||
ClearPlayerSettingsDirtyFlag(); | ||
} | ||
|
||
|
||
private static bool IsPlayerSettingsDirty() | ||
{ | ||
#if UNITY_2019_OR_NEWER | ||
var settings = Resources.FindObjectsOfTypeAll<PlayerSettings>(); | ||
if (settings != null && settings.Length > 0) | ||
return EditorUtility.IsDirty(settings[0]); | ||
return false; | ||
#else | ||
dongruoping marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return false; | ||
#endif | ||
} | ||
|
||
private static void ClearPlayerSettingsDirtyFlag() | ||
{ | ||
#if UNITY_2019_OR_NEWER | ||
var settings = Resources.FindObjectsOfTypeAll<PlayerSettings>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PlayerSettings? I thought this would be an MLAgentsSettings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're injecting our settings into the preload assets in PlayerSettings and these method is to detect/clear the dirty flag of PlayerSettings before/after we put in our settings. So PlayerSettings here is correct. |
||
if (settings != null && settings.Length > 0) | ||
EditorUtility.ClearDirty(settings[0]); | ||
#endif | ||
} | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be a dumb question, but why do we have a BuildSettingsProvider if none of the settings are used in a build? (They are all Editor only settings?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are used in builds, if you don't want to train or even try to connect to a trainer in a game that's being shipped you'd want to know that in your build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are used in builds. If you don't want to train or even try to connect to a trainer in a game that's being shipped, you'd want to know that in your build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean the use case it to remove the command line argument listeners on the build? Because a build will not try to listen to a trainer unless a specific command line argument is passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. Originally I made a player training port but that doesn't seem to make much sense so I removed that.
But I still think there's chance that we'll add settings that is used in player (analytics? academy stepping?)