Skip to content

Various fixes #176

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.

Sign up for GitHub

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

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Various fixes #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
da7789b
Use TypeCache instead of GetAllAssignableClasses
lgarczyn Aug 27, 2025
1d9f0b5
Don't crash if collection in CollectionRegistry is null
lgarczyn Aug 27, 2025
72089b8
Fixed error when refreshing a graph where the last item was deleted
lgarczyn Aug 27, 2025
82b43cf
Added sort command for collection editor
lgarczyn Aug 27, 2025
6d3d97e
fixed collection move not updating collection parent
lgarczyn Aug 27, 2025
c6257f2
Don't move file if target folder is same
lgarczyn Aug 27, 2025
8608bf5
Fixed whitespace
lgarczyn Aug 27, 2025
c372719
Allow multiple collection namespace
lgarczyn Aug 27, 2025
f419fa8
Merge remote-tracking branch 'origin/master'
lgarczyn Aug 27, 2025
fe9c8ef
Fixed absurdly long error message going off screen
lgarczyn Aug 28, 2025
9b94b69
Fixed moving asset from "Path/Asset.asset" to "Path/Asset1.asset"
lgarczyn Aug 28, 2025
c1aac39
Try to make asset moving faster by using AssetDatabase.StartAssetEditing
lgarczyn Aug 28, 2025
c5a4df5
Fixed stack overflow
lgarczyn Aug 28, 2025
8da50d7
Fixed path compare
lgarczyn Aug 28, 2025
4484f5c
Don't fight with neighbor collections
lgarczyn Aug 28, 2025
2869c2d
Fixed exception
lgarczyn Aug 28, 2025
e45787f
Improved log
lgarczyn Aug 28, 2025
31f569d
Fixed compare
lgarczyn Aug 28, 2025
003b01e
Removed log
lgarczyn Aug 28, 2025
4c83758
FUCKING FIXED LIST ORDERING WOW
lgarczyn Aug 28, 2025
b106540
Removed autoclean of ExtensionOld
lgarczyn Aug 29, 2025
40df7c7
Full removed ExtensionOld
lgarczyn Aug 29, 2025
e1138eb
Added checks for accessors in CollectionsRegistry.cs
lgarczyn Aug 29, 2025
b43af7a
Update Scripts/Runtime/Core/ScriptableObjectCollection.cs
lgarczyn Sep 1, 2025
07f002c
Update Scripts/Runtime/Core/ScriptableObjectCollection.cs
lgarczyn Sep 1, 2025
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
60 changes: 26 additions & 34 deletions Scripts/Editor/Core/CodeGenerationUtility.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
private const string PrivateValuesName = "cachedValues";
private const string PublicValuesName = "Values";
private const string HasCachedValuesName = "hasCachedValues";
private const string ExtensionOld = ".cs";
private const string ExtensionNew = ".g.cs";


Expand Down Expand Up @@ -343,18 +342,6 @@
AssetDatabaseUtils.CreatePathIfDoesntExist(targetFolder);

string targetFileName = Path.Combine(targetFolder, fileName);

// Delete any existing files that have the old deprecated extension.
string deprecatedFileName = targetFileName + ExtensionOld;
#if UNITY_2023_1_OR_NEWER
if (AssetDatabase.AssetPathExists(deprecatedFileName))
#else
if (AssetDatabase.LoadAssetAtPath<UnityEngine.Object>(deprecatedFileName) != null)
#endif
{
Debug.LogWarning($"Deleting deprecated Indirect Access file '{deprecatedFileName}'.");
AssetDatabase.DeleteAsset(deprecatedFileName);
}

targetFileName += ExtensionNew;
using (StreamWriter writer = new StreamWriter(targetFileName))
Expand Down Expand Up @@ -406,14 +393,6 @@

string finalFileName = Path.Combine(finalFolder, fileName);

// Delete any existing files that have the old deprecated extension.
string deprecatedFileName = finalFileName + ExtensionOld;
if (File.Exists(deprecatedFileName))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove this exactly? It is useful when migrating from an older version of SOC that it doesn't leave the old .cs file but automatically removes it, otherwise you get compile errors and you have to manually find where it generated the code and delete the old .cs file

Copy link
Author

Choose a reason for hiding this comment

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

Our collections are in the same folder as the generated code, so this code deletes them. Not sure how we ended up with a different structure than recommended, but we have a 100+ collections now.

This wasn't meant to be sent with the pull request, but since I start the pr with master, it got updated

{
Debug.LogWarning($"Deleting deprecated Static Access file '{deprecatedFileName}'.");
AssetDatabase.DeleteAsset(deprecatedFileName);
}

finalFileName += ExtensionNew;
using (StreamWriter writer = new StreamWriter(finalFileName))
{
Expand Down Expand Up @@ -519,8 +498,12 @@
private static void WriteDirectAccessCollectionStatic(ScriptableObjectCollection collection, StreamWriter writer,
ref int indentation, bool useBaseClass)
{
AppendLine(writer, indentation, $"private static bool {HasCachedValuesName};");
AppendLine(writer, indentation, $"private static {collection.GetType().Name} {PrivateValuesName};");
string privateValuesName = GetCollectionSpecificVariableName(PrivateValuesName, collection.name);
string publicValuesName = GetCollectionSpecificVariableName(PublicValuesName, collection.name);
string hasCachedValuesName = GetCollectionSpecificVariableName(HasCachedValuesName, collection.name);

AppendLine(writer, indentation, $"private static bool {hasCachedValuesName};");
AppendLine(writer, indentation, $"private static {collection.GetType().Name} {privateValuesName};");

AppendLine(writer, indentation);

Expand All @@ -538,20 +521,20 @@


AppendLine(writer, indentation,
$"public static {collection.GetType().FullName} {PublicValuesName}");
$"public static {collection.GetType().FullName} {publicValuesName}");

AppendLine(writer, indentation, "{");
indentation++;
AppendLine(writer, indentation, "get");
AppendLine(writer, indentation, "{");
indentation++;
AppendLine(writer, indentation, $"if (!{HasCachedValuesName})");
AppendLine(writer, indentation, $"if (!{hasCachedValuesName})");
indentation++;
(long, long) collectionGUIDValues = collection.GUID.GetRawValues();
AppendLine(writer, indentation,
$"{HasCachedValuesName} = CollectionsRegistry.Instance.TryGetCollectionByGUID(new LongGuid({collectionGUIDValues.Item1}, {collectionGUIDValues.Item2}), out {PrivateValuesName});");
$"{hasCachedValuesName} = CollectionsRegistry.Instance.TryGetCollectionByGUID(new LongGuid({collectionGUIDValues.Item1}, {collectionGUIDValues.Item2}), out {privateValuesName});");
indentation--;
AppendLine(writer, indentation, $"return {PrivateValuesName};");
AppendLine(writer, indentation, $"return {privateValuesName};");
indentation--;
AppendLine(writer, indentation, "}");
indentation--;
Expand Down Expand Up @@ -583,7 +566,7 @@
indentation++;
(long, long) collectionItemGUIDValues = socItem.GUID.GetRawValues();
AppendLine(writer, indentation,
$"{privateHasCachedName} = {PublicValuesName}.TryGetItemByGUID(new LongGuid({collectionItemGUIDValues.Item1}, {collectionItemGUIDValues.Item2}), out {privateStaticCachedName});");
$"{privateHasCachedName} = {publicValuesName}.TryGetItemByGUID(new LongGuid({collectionItemGUIDValues.Item1}, {collectionItemGUIDValues.Item2}), out {privateStaticCachedName});");
indentation--;
AppendLine(writer, indentation, $"return {privateStaticCachedName};");
indentation--;
Expand All @@ -598,12 +581,16 @@

private static void WriteNonAutomaticallyLoadedCollectionItems(ScriptableObjectCollection collection, StreamWriter writer, ref int indentation, bool useBaseClass)
{
string privateValuesName = GetCollectionSpecificVariableName(PrivateValuesName, collection.name);

Check warning on line 584 in Scripts/Editor/Core/CodeGenerationUtility.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Scripts/Editor/Core/CodeGenerationUtility.cs#L584 

Remove the unused local variable 'privateValuesName'.
string publicValuesName = GetCollectionSpecificVariableName(PublicValuesName, collection.name);
string hasCachedValuesName = GetCollectionSpecificVariableName(HasCachedValuesName, collection.name);

Check warning on line 586 in Scripts/Editor/Core/CodeGenerationUtility.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Scripts/Editor/Core/CodeGenerationUtility.cs#L586 

Remove the unused local variable 'hasCachedValuesName'.

AppendLine(writer, indentation,
$"public static bool IsCollectionLoaded()");

AppendLine(writer, indentation, "{");
indentation++;
AppendLine(writer, indentation, $"return {PublicValuesName} != null;");
AppendLine(writer, indentation, $"return {publicValuesName} != null;");
indentation--;
AppendLine(writer, indentation, "}");

Expand All @@ -630,8 +617,8 @@
AppendLine(writer, indentation, "{");
indentation++;
AppendLine(writer, indentation, "CollectionsRegistry.Instance.RegisterCollection(operation.Result);");
AppendLine(writer, indentation, $"{HasCachedValuesName} = true;");
AppendLine(writer, indentation, $"{PrivateValuesName} = operation.Result;");
AppendLine(writer, indentation, $"{hasCachedValuesName} = true;");
AppendLine(writer, indentation, $"{privateValuesName} = operation.Result;");
indentation--;
AppendLine(writer, indentation, "};");
AppendLine(writer, indentation, "return collectionHandle;");
Expand All @@ -642,9 +629,9 @@
AppendLine(writer, indentation, "public static void UnloadCollection()");
AppendLine(writer, indentation, "{");
indentation++;
AppendLine(writer, indentation, $"CollectionsRegistry.Instance.UnregisterCollection({PublicValuesName});");
AppendLine(writer, indentation, $"{HasCachedValuesName} = false;");
AppendLine(writer, indentation, $"{PrivateValuesName} = null;");
AppendLine(writer, indentation, $"CollectionsRegistry.Instance.UnregisterCollection({publicValuesName});");
AppendLine(writer, indentation, $"{hasCachedValuesName} = false;");
AppendLine(writer, indentation, $"{privateValuesName} = null;");

AppendLine(writer, indentation, "Addressables.Release(collectionHandle);");
indentation--;
Expand All @@ -661,5 +648,10 @@
AssetDatabase.GetAssetPath(SOCSettings.Instance.GetParentDefaultAssetScriptsFolderForCollection(collection)),
$"{SOCSettings.Instance.GetStaticFilenameForCollection(collection)}{ExtensionNew}"));
}

private static string GetCollectionSpecificVariableName(string variableName, string collectionName)
{
return $"{variableName}_{collectionName}";
}
}
}
65 changes: 65 additions & 0 deletions Scripts/Editor/CustomEditors/CollectionCustomEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@

private void OnCollectionItemOrderChanged(int fromIndex, int toIndex)
{
ReloadFilteredItems();
ScriptableObject sourceItem = filteredItems[fromIndex];
ScriptableObject targetItem = filteredItems[toIndex];

Expand Down Expand Up @@ -735,12 +736,32 @@
false,
() => { CopyCollectionItemUtility.SetSource(scriptableObject); }
);

menu.AddDisabledItem(new GUIContent("Sort Items"), false);
}
else
{
menu.AddDisabledItem(
new GUIContent("Copy Values"),
false);

menu.AddItem(
new GUIContent("Sort Items"),
false,
() =>
{
Undo.RecordObject(collection, "Sort Items");
List<ScriptableObject> itemsToBeSorted = new();
foreach (int selectedIndex in collectionItemListView.selectedIndices)
{
itemsToBeSorted.Add(filteredItems[selectedIndex]);
}

collection.Sort(new LimitedComparer(itemsToBeSorted));

ReloadFilteredItems();
}
);
}
if (CopyCollectionItemUtility.CanPasteToTarget(scriptableObject))
{
Expand Down Expand Up @@ -1085,4 +1106,48 @@
}
}
}

internal struct LimitedComparer : IComparer<ScriptableObject>

Check warning on line 1110 in Scripts/Editor/CustomEditors/CollectionCustomEditor.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Scripts/Editor/CustomEditors/CollectionCustomEditor.cs#L1110 

Implement 'IEquatable<T>' in value type 'LimitedComparer'.
{
private readonly HashSet<ScriptableObjectCollectionItem> m_ItemsToBeSorted;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The m_ prefix is not consistent with the rest of the package. According to the style of this package this field should be called itemsToBeSorted. That goes for m_FirstItemIndex as well

private readonly int m_FirstItemIndex;
public LimitedComparer(List<ScriptableObject> itemsToBeSorted)
{
m_ItemsToBeSorted = itemsToBeSorted.OfType<ScriptableObjectCollectionItem>().ToHashSet();
m_FirstItemIndex = m_ItemsToBeSorted.Select(i => i.Index).Min();
}

public int Compare(ScriptableObject xObject, ScriptableObject yObject)
{
if (ReferenceEquals(xObject, yObject)) return 0;
if (yObject is null) return 1;
if (xObject is null) return -1;

if (xObject is not ScriptableObjectCollectionItem x || yObject is not ScriptableObjectCollectionItem y)
{
return 0; // or throw an exception if you want to enforce that both are of type ScriptableObjectCollectionItem
}

bool toSortX = m_ItemsToBeSorted.Contains(x);
bool toSortY = m_ItemsToBeSorted.Contains(y);

if (toSortX && toSortY)
{
return String.Compare(x.name, y.name, StringComparison.Ordinal);
}

if (toSortX)
{
// compare y with the first item in the list to sort
return x.Index - m_FirstItemIndex;
}
if (toSortY)
{
// compare x with the first item in the list to sort
return m_FirstItemIndex - y.Index;
}
Comment on lines +1139 to +1148
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are the comments in these if statements perhaps flipped?

// neither x nor y are in the list to sort, compare by Index
return x.Index.CompareTo(y.Index);
}
}
}
2 changes: 1 addition & 1 deletion Scripts/Editor/Generators/CollectionGenerators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ private static void GetGeneratorTypes(Type generatorType, out Type collectionTyp

private static Type[] GetAllGeneratorTypes()
{
return InterfaceType.GetAllAssignableClasses();
return TypeCache.GetTypesDerivedFrom(InterfaceType).ToArray();
}

public static Type GetGeneratorTypeForCollection(Type collectionType, bool allowSubclasses = true)
Expand Down
23 changes: 16 additions & 7 deletions Scripts/Runtime/Core/CollectionsRegistry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,13 @@ public void SetAutoSearchForCollections(bool isOn)

public void UpdateAutoSearchForCollections()
{
for (int i = 0; i < Collections.Count; i++)
foreach (ScriptableObjectCollection collection in collections)
{
ScriptableObjectCollection collection = Collections[i];
if (!collection.AutomaticallyLoaded)
if (!collection)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's fine to omit braces for one line statements after an if condition. It's not done 100% of the time so I wouldn't say it's a rule, just know that it's okay to do so. In cases like this where it's just a continue, I'd say it's simpler / snappier.

{
continue;
}
if (collection != null && !collection.AutomaticallyLoaded)
{
SetAutoSearchForCollections(true);
return;
Expand All @@ -525,9 +528,12 @@ public void UpdateAutoSearchForCollections()

public bool HasUniqueGUID(ISOCItem targetItem)
{
for (int i = 0; i < collections.Count; i++)
foreach (ScriptableObjectCollection collection in collections)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a particular reason you've changed these for loops to foreach loops?

Copy link
Author

Choose a reason for hiding this comment

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

They're faster and I find them more readable

{
ScriptableObjectCollection collection = collections[i];
if (!collection)
{
continue;
}
foreach (ScriptableObject scriptableObject in collection)
{
if (scriptableObject is ISOCItem socItem)
Expand All @@ -543,9 +549,12 @@ public bool HasUniqueGUID(ISOCItem targetItem)

public bool HasUniqueGUID(ScriptableObjectCollection targetCollection)
{
for (int i = 0; i < collections.Count; i++)
foreach (ScriptableObjectCollection collection in collections)
{
ScriptableObjectCollection collection = collections[i];
if (!collection)
{
continue;
}
if (collection != targetCollection && collection.GUID == targetCollection.GUID)
return false;
}
Expand Down
Loading