Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

Worker Inspector window & Entity List #1375

Merged
merged 7 commits into from
May 29, 2020
Merged
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion workers/unity/Packages/io.improbable.gdk.core/EntityId.cs
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ namespace Improbable.Gdk.Core
/// Instances of this type should be treated as transient identifiers that will not be
/// consistent between different runs of the same simulation.
/// </remarks>
public readonly struct EntityId : IEquatable<EntityId>
public readonly struct EntityId : IEquatable<EntityId>, IComparable<EntityId>, IComparable
{
/// <summary>
/// The value of the EntityId.
@@ -82,5 +82,42 @@ public override string ToString()
{
return Id.ToString();
}

public int CompareTo(EntityId other)
{
return Id.CompareTo(other.Id);
}

public int CompareTo(object obj)
{
if (ReferenceEquals(null, obj))
{
return 1;
}

return obj is EntityId other
? CompareTo(other)
: throw new ArgumentException($"Object must be of type {nameof(EntityId)}");
}

public static bool operator <(EntityId left, EntityId right)
{
return left.CompareTo(right) < 0;
}

public static bool operator >(EntityId left, EntityId right)
{
return left.CompareTo(right) > 0;
}

public static bool operator <=(EntityId left, EntityId right)
{
return left.CompareTo(right) <= 0;
}

public static bool operator >=(EntityId left, EntityId right)
{
return left.CompareTo(right) >= 0;
}
}
}
Original file line number Diff line number Diff line change
@@ -10,6 +10,8 @@ namespace Improbable.Gdk.Core
[UpdateBefore(typeof(SpatialOSReceiveSystem))]
public class EntitySystem : ComponentSystem
{
public int ViewVersion { get; private set; }

private readonly List<EntityId> entitiesAdded = new List<EntityId>();
private readonly List<EntityId> entitiesRemoved = new List<EntityId>();

@@ -49,6 +51,11 @@ internal void ApplyDiff(ViewDiff diff)
{
entitiesRemoved.Add(entityId);
}

if (entitiesAdded.Count != 0 || entitiesRemoved.Count != 0)
jamiebrynes7 marked this conversation as resolved.
Show resolved Hide resolved
{
ViewVersion += 1;
}
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo("Improbable.Gdk.Debug.EditmodeTests")]

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
@@ -4,7 +4,8 @@
"Improbable.Gdk.Core",
"Unity.Entities",
"Sirenix.OdinInspector.Editor",
"Improbable.Gdk.Core.Editor"
"Improbable.Gdk.Core.Editor",
"Improbable.Gdk.Generated"
jamiebrynes7 marked this conversation as resolved.
Show resolved Hide resolved
],
"includePlatforms": [
"Editor"
@@ -15,5 +16,6 @@
"precompiledReferences": [],
"autoReferenced": true,
"defineConstraints": [],
"versionDefines": []
"versionDefines": [],
"noEngineReferences": false
}
8 changes: 8 additions & 0 deletions workers/unity/Packages/io.improbable.gdk.debug/Tests.meta

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,28 @@
{
"name": "Improbable.Gdk.Debug.EditmodeTests",
"references": [
"GUID:40425aeb5a2b3f74797745fff21bc766",
"GUID:c5f2ea2f695256346af9ccd76c6f055d",
"GUID:edb3612c44ad0d24988d387581fd5fbe",
"GUID:0acc523941302664db1f4e527237feb3",
"GUID:27619889b8ba8c24980f49ee34dbb44a",
"GUID:734d92eba21c94caba915361bd5ac177",
"GUID:bd66e1825421d9041a8f2c4fa9ca562a"
],
"includePlatforms": [
"Editor"
],
"excludePlatforms": [],
"allowUnsafeCode": false,
"overrideReferences": true,
"precompiledReferences": [
"nunit.framework.dll",
"Improbable.Worker.CInterop.dll"
],
"autoReferenced": false,
"defineConstraints": [
"UNITY_INCLUDE_TESTS"
],
"versionDefines": [],
"noEngineReferences": false
}

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,146 @@
using Improbable.Gdk.Core;
using Improbable.Gdk.Debug.WorkerInspector;
using Improbable.Gdk.TestUtils;
using NUnit.Framework;

namespace Improbable.Gdk.Debug.EditmodeTests.WorkerInspector
{
[TestFixture]
public class EntityListDataTests : MockBase
{
[TestCase]
public void Data_is_empty_intially()
{
var data = new EntityListData();
Assert.IsEmpty(data.Data);
}

[TestCase]
public void SetNewWorld_should_not_throw()
{
var data = new EntityListData();
Assert.DoesNotThrow(() => data.SetNewWorld(null));
}

[TestCase]
public void RefreshData_does_not_throw_if_no_world()
{
var data = new EntityListData();
Assert.DoesNotThrow(() => data.RefreshData());
}

[TestCase]
public void ApplySearch_does_not_throw_if_no_world()
{
var data = new EntityListData();
Assert.DoesNotThrow(() => data.ApplySearch(EntitySearchParameters.FromSearchString("")));
}

[TestCase]
public void RefreshData_finds_entities_after_setting_world()
{
World
.Step(world =>
{
world.Connection.CreateEntity(1, GetTemplate("some-entity"));
})
.Step(world =>
{
var data = new EntityListData();
data.SetNewWorld(world.Worker.World); // Yikes
return data;
})
.Step((world, data) =>
{
data.RefreshData();
Assert.AreEqual(1, data.Data.Count);

var entityData = data.Data[0];
Assert.AreEqual(1, entityData.EntityId.Id);
Assert.AreEqual("some-entity", entityData.Metadata);
});
}

[TestCase]
public void ApplySearchFilter_immediately_filters_data()
{
World
.Step(world =>
{
world.Connection.CreateEntity(1, GetTemplate("some-entity"));
})
.Step(world =>
{
var data = new EntityListData();
data.SetNewWorld(world.Worker.World); // Yikes
return data;
})
.Step((world, data) =>
{
data.RefreshData();
Assert.AreEqual(1, data.Data.Count);
})
.Step((world, data) =>
{
data.ApplySearch(EntitySearchParameters.FromSearchString("2")); // Entity ID = 2
Assert.IsEmpty(data.Data);
});
}

[TestCase]
public void SetNewWorld_resets_data()
{
World
.Step(world =>
{
world.Connection.CreateEntity(1, GetTemplate("some-entity"));
})
.Step(world =>
{
var data = new EntityListData();
data.SetNewWorld(world.Worker.World); // Yikes
return data;
})
.Step((world, data) =>
{
data.RefreshData();
Assert.AreEqual(1, data.Data.Count);
})
.Step((world, data) =>
{
data.SetNewWorld(null);
Assert.IsEmpty(data.Data);
});
}

[TestCase]
public void SearchFilter_persists_through_RefreshData()
{
World
.Step(world =>
{
world.Connection.CreateEntity(1, GetTemplate("some-entity"));
})
.Step(world =>
{
var data = new EntityListData();
data.ApplySearch(EntitySearchParameters.FromSearchString("2")); // Entity ID = 2
data.SetNewWorld(world.Worker.World); // Yikes
return data;
})
.Step((world, data) =>
{
data.RefreshData();
Assert.IsEmpty(data.Data);
});
}

private EntityTemplate GetTemplate(string metadata)
{
var template = new EntityTemplate();
template.AddComponent(new Position.Snapshot());
template.AddComponent(new Metadata.Snapshot(metadata));
return template;
}
}
}

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,48 @@
using Improbable.Gdk.Debug.WorkerInspector;
using NUnit.Framework;

namespace Improbable.Gdk.Debug.EditmodeTests.WorkerInspector
{
[TestFixture]
public class EntitySearchParametersTests
{
[TestCase("1", 1)]
[TestCase(" 1 ", 1)]
[TestCase("1 ", 1)]
public void FromSearchString_parses_out_numbers(string searchString, long resultingId)
{
var searchParams = EntitySearchParameters.FromSearchString(searchString);
Assert.IsTrue(searchParams.EntityId.HasValue);
Assert.AreEqual(resultingId, searchParams.EntityId.Value.Id);
}

[TestCase("0")]
[TestCase("-1")]
[TestCase("300 spartans")]
[TestCase("9223372036854775808")] // Max long + 1
paulbalaji marked this conversation as resolved.
Show resolved Hide resolved
public void FromSearchString_rejects_non_entity_id_values(string searchString)
{
var searchParams = EntitySearchParameters.FromSearchString(searchString);
Assert.IsFalse(searchParams.EntityId.HasValue);
}

[TestCase("some value", "some value")]
[TestCase(" with leading space", "with leading space")]
[TestCase("with trailing space ", "with trailing space")]
[TestCase(" with both! ", "with both!")]
public void FromSearchString_ignores_leading_and_trailing_whitespace(string searchString, string expectedFragment)
{
var searchParams = EntitySearchParameters.FromSearchString(searchString);
Assert.IsNotNull(searchParams.SearchFragment);
Assert.AreEqual(expectedFragment, searchParams.SearchFragment);
}

[TestCase]
public void FromSearchString_converts_all_to_lower()
{
var searchParams = EntitySearchParameters.FromSearchString("CAPS");
Assert.IsNotNull(searchParams.SearchFragment);
Assert.AreEqual("caps", searchParams.SearchFragment);
}
}
}

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,122 @@
using System;
using System.Collections.Generic;
using Improbable.Gdk.Core;
using Unity.Entities;
using UnityEditor;
using UnityEditor.UIElements;
using UnityEngine.UIElements;

namespace Improbable.Gdk.Debug.WorkerInspector
{
public class EntityList : VisualElement
{
public delegate void EntitySelected(EntityId entityId);

public EntitySelected OnEntitySelected;

private readonly EntityListData entities = new EntityListData();
private readonly ListView listView;

private EntitySystem entitySystem;
private int lastViewVersion;
private EntityData? selectedEntity;

public EntityList()
{
const string uxmlPath = "Packages/io.improbable.gdk.debug/WorkerInspector/Templates/EntityList.uxml";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any kind of system or central place to put hard coded references to other packages/assets? Not saying we need to do that here if it's a one-off, but seeing this made me think of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope we don't. Tbh I'd be against a central place since its only referenced here and the locality is nice.

var template = AssetDatabase.LoadAssetAtPath<VisualTreeAsset>(uxmlPath);
template.CloneTree(this);

listView = this.Q<ListView>();
listView.itemHeight = 24;
jamiebrynes7 marked this conversation as resolved.
Show resolved Hide resolved
listView.makeItem = () => new Label();
listView.bindItem = BindItem;
listView.onSelectionChanged += OnSelectionChanged;
listView.itemsSource = entities.Data;

var searchField = this.Q<ToolbarSearchField>();
searchField.RegisterCallback<ChangeEvent<string>>(OnSearchFieldChanged);
}

public void Update()
jamiebrynes7 marked this conversation as resolved.
Show resolved Hide resolved
{
if (entitySystem == null)
{
listView.itemsSource = entities.Data;
jamiebrynes7 marked this conversation as resolved.
Show resolved Hide resolved
return;
}

if (entitySystem.ViewVersion == lastViewVersion)
{
return;
}

entities.RefreshData();
listView.itemsSource = entities.Data;

// Attempt to continue focusing the previously selected value.
if (selectedEntity != null)
{
var index = entities.Data.IndexOf(selectedEntity.Value);

if (index != -1)
{
listView.selectedIndex = index;
}
}

lastViewVersion = entitySystem.ViewVersion;
}

public void SetWorld(World world)
{
entitySystem = world?.GetExistingSystem<EntitySystem>();
lastViewVersion = 0;
selectedEntity = null;

entities.SetNewWorld(world);
}

private void BindItem(VisualElement element, int index)
{
var entity = entities.Data[index];

if (!(element is Label label))
{
return;
jamiebrynes7 marked this conversation as resolved.
Show resolved Hide resolved
}

label.text = entity.ToString();
}

private void OnSelectionChanged(List<object> selections)
{
if (selections.Count != 1)
{
throw new InvalidOperationException("Unexpectedly selected more than one entity.");
}

if (!(selections[0] is EntityData entityData))
{
throw new InvalidOperationException($"Unexpected type for selection: {selections[0].GetType()}");
}

if (!selectedEntity.HasValue || selectedEntity.Value != entityData)
{
OnEntitySelected?.Invoke(entityData.EntityId);
selectedEntity = entityData;
}
}

private void OnSearchFieldChanged(ChangeEvent<string> changeEvent)
{
var searchValue = changeEvent.newValue.Trim();
entities.ApplySearch(EntitySearchParameters.FromSearchString(searchValue));
listView.itemsSource = entities.Data;
}

public new class UxmlFactory : UxmlFactory<EntityList>
{
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
using System;
using System.Collections.Generic;
using Improbable.Gdk.Core;
using Unity.Collections;
using Unity.Entities;
using Unity.Profiling;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, there are ProfilerMarkers on L12-13.

SonarCloud will flag unused imports now as a code smell 😁


namespace Improbable.Gdk.Debug.WorkerInspector
{
internal class EntityListData
{
private static ProfilerMarker refreshDataMarker = new ProfilerMarker("EntityList.RefreshData");
private static ProfilerMarker applySearchMarker = new ProfilerMarker("EntityList.ApplySearch");

public readonly List<EntityData> Data = new List<EntityData>();
jamiebrynes7 marked this conversation as resolved.
Show resolved Hide resolved

private readonly List<EntityData> fullData = new List<EntityData>();
private World world;
private EntityQuery query;
private EntitySearchParameters searchParameters;

public void ApplySearch(EntitySearchParameters searchParameters)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApplyFilter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer ApplySearch since the parameter is of type EntitySearchParameters

{
this.searchParameters = searchParameters;

using (applySearchMarker.Auto())
{
Data.Clear();

foreach (var datum in fullData)
{
if (datum.Matches(searchParameters))
{
Data.Add(datum);
}
}
}
}

public void SetNewWorld(World newWorld)
{
fullData.Clear();
Data.Clear();
query?.Dispose();

if (newWorld == null)
{
world = null;
query = null;
return;
}

// Need to refresh the query.
world = newWorld;
query = world.EntityManager.CreateEntityQuery(ComponentType.ReadOnly<SpatialEntityId>());
}

public void RefreshData()
{
if (world == null)
{
return;
}

using (refreshDataMarker.Auto())
{
fullData.Clear();
var spatialOSComponentType = world.EntityManager.GetArchetypeChunkComponentType<SpatialEntityId>(true);
var metadataComponentType =
world.EntityManager.GetArchetypeChunkComponentType<Metadata.Component>(true);
var ecsEntityType = world.EntityManager.GetArchetypeChunkEntityType();

using (var chunks = query.CreateArchetypeChunkArray(Allocator.TempJob))
{
foreach (var chunk in chunks)
{
NativeArray<Metadata.Component>? metadataArray = null;

if (chunk.Has(metadataComponentType))
{
metadataArray = chunk.GetNativeArray(metadataComponentType);
}

var entityIdArray = chunk.GetNativeArray(spatialOSComponentType);
var entities = chunk.GetNativeArray(ecsEntityType);

for (var i = 0; i < entities.Length; i++)
{
var data = new EntityData(entityIdArray[i].EntityId, metadataArray?[i].EntityType);
fullData.Add(data);
}
}
}

fullData.Sort();
}

ApplySearch(searchParameters);
}
}

internal readonly struct EntityData : IComparable<EntityData>, IComparable, IEquatable<EntityData>
{
public readonly EntityId EntityId;
public readonly string Metadata;

public EntityData(EntityId entityId, string metadata)
{
EntityId = entityId;
Metadata = metadata;
}

public bool Matches(EntitySearchParameters searchParameters)
{
if (searchParameters.EntityId.HasValue)
{
return searchParameters.EntityId.Value == EntityId;
}

if (!string.IsNullOrEmpty(searchParameters.SearchFragment))
{
return Metadata.ToLower().Contains(searchParameters.SearchFragment);
}

return true;
}

public override string ToString()
{
return Metadata != null ? $"{Metadata} ({EntityId})" : EntityId.ToString();
}

public int CompareTo(EntityData other)
{
return EntityId.CompareTo(other.EntityId);
}

public int CompareTo(object obj)
{
if (ReferenceEquals(null, obj))
{
return 1;
}

return obj is EntityData other
? CompareTo(other)
: throw new ArgumentException($"Object must be of type {nameof(EntityData)}");
}

public bool Equals(EntityData other)
{
return EntityId.Equals(other.EntityId) && Metadata == other.Metadata;
}

public override bool Equals(object obj)
{
return obj is EntityData other && Equals(other);
}

public static bool operator ==(EntityData left, EntityData right)
{
return left.Equals(right);
}

public static bool operator !=(EntityData left, EntityData right)
{
return !left.Equals(right);
}

public override int GetHashCode()
{
unchecked
{
return (EntityId.GetHashCode() * 397) ^ (Metadata != null ? Metadata.GetHashCode() : 0);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using Improbable.Gdk.Core;

namespace Improbable.Gdk.Debug.WorkerInspector
{
internal readonly struct EntitySearchParameters
{
public readonly EntityId? EntityId;
public readonly string SearchFragment;

private EntitySearchParameters(EntityId entityId)
{
EntityId = entityId;
SearchFragment = null;
}

private EntitySearchParameters(string stringFragment)
{
EntityId = null;
SearchFragment = stringFragment;
}

public static EntitySearchParameters FromSearchString(string searchValue)
{
// EntityID values are strictly positive
if (long.TryParse(searchValue, out var value) && value > 0)
{
return new EntitySearchParameters(new EntityId(value));
}

return new EntitySearchParameters(searchValue.Trim().ToLower());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<UXML xmlns="UnityEngine.UIElements" xmlns:ue ="UnityEditor.UIElements">
<ue:Toolbar>
<ue:ToolbarSearchField class="entity-list-searchbar"/>
</ue:Toolbar>
<ListView class="entity-list-view" />
</UXML>
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
.entity-list-view {
flex-grow: 1;
flex-direction: column;
}

.unity-list-view__item {
padding-left: 8px;
-unity-text-align: middle-left;
}

.entity-panel {
flex-grow: 1;
flex-direction: column;
max-width: 33%;
background-color: rgba(0, 0, 0, 0.05);
border-right-color: rgba(0, 0, 0, 0.2);
border-right-width: 1px;
border-top-color: rgba(0, 0, 0, 0.2);
border-top-width: 1px;
}

.entity-list-searchbar {
flex-grow: 1;
width: auto;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<UXML
xmlns="UnityEngine.UIElements"
xmlns:gdk="Improbable.Gdk.Core.Editor.UIElements"
xmlns:wi="Improbable.Gdk.Debug.WorkerInspector"
>
<gdk:WorldSelector />
<wi:EntityList class="entity-panel"/>
</UXML>
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using Improbable.Gdk.Core;
using Improbable.Gdk.Core.Editor.UIElements;
using Unity.Entities;
using UnityEditor;
using UnityEngine.UIElements;

namespace Improbable.Gdk.Debug.WorkerInspector
{
internal class WorkerInspectorWindow : EditorWindow
{
private WorldSelector worldSelector;
private EntityList entityList;

[MenuItem("SpatialOS/Window/Worker Inspector", false)]
public static void ShowWindow()
{
var windowType = typeof(EditorWindow).Assembly.GetType("UnityEditor.InspectorWindow");
var window = GetWindow<WorkerInspectorWindow>("Worker Inspector", windowType);
window.Show();
}

private void OnEnable()
{
SetupUI();
worldSelector.UpdateWorldSelection();
}

private void OnInspectorUpdate()
{
worldSelector.UpdateWorldSelection();
entityList.Update();
}

private void SetupUI()
{
const string windowUxmlPath = "Packages/io.improbable.gdk.debug/WorkerInspector/Templates/WorkerInspectorWindow.uxml";
const string windowUssPath = "Packages/io.improbable.gdk.debug/WorkerInspector/Templates/WorkerInspectorWindow.uss";

var windowTemplate = AssetDatabase.LoadAssetAtPath<VisualTreeAsset>(windowUxmlPath);
windowTemplate.CloneTree(rootVisualElement);

var stylesheet = AssetDatabase.LoadAssetAtPath<StyleSheet>(windowUssPath);
rootVisualElement.styleSheets.Add(stylesheet);

worldSelector = rootVisualElement.Q<WorldSelector>();
worldSelector.OnWorldChanged += OnWorldChanged;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to manually remove these callbacks if the window is closed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the window is closed, the world selector element is removed along with it, so its all GC'ed


entityList = rootVisualElement.Q<EntityList>();
entityList.OnEntitySelected += OnEntitySelected;
}

private void OnWorldChanged(World world)
{
entityList.SetWorld(world);
}

private void OnEntitySelected(EntityId entityId)
{
UnityEngine.Debug.Log($"Selected {entityId}");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug stuff that is removed later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just a placeholder for now.

}
}
}