Skip to content

Commit 5da6a5b

Browse files
authored
DetectAssetChangeProcessor bug fix (#182)
## Description - Skip scene assets when doing `LoadAssetAtPath` in DetectAssetChangePostProcessor - Add documentation URL to package.json - Update home page to github pages ## Related Issue <!-- Link to the issue this PR addresses --> Fixes #180 Fixes #181 ## Type of Change <!-- Check all that apply --> - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Documentation update - [ ] Refactor (code change that neither fixes a bug nor adds a feature) ## Checklist <!-- Ensure all items are completed before requesting review --> - [x] I have added tests that prove my fix is effective or my feature works - [x] I have updated the documentation accordingly - [x] I have updated the [CHANGELOG](../CHANGELOG.md) - [x] My changes do not introduce breaking changes, or breaking changes are documented
1 parent 12561e4 commit 5da6a5b

File tree

5 files changed

+104
-11
lines changed

5 files changed

+104
-11
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- **DetectAssetChanged scene file crash**: Fixed Unity crash ("Do not use ReadObjectThreaded on scene objects!") when `.unity` or `.scenetemplate` files were processed by the asset change detection system
13+
1014
See [the roadmap](./docs/overview/roadmap.md) for details
1115

1216
## [3.1.6]

Editor/AssetProcessors/DetectAssetChangeProcessor.cs

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -554,15 +554,19 @@ IReadOnlyList<string> createdPaths
554554
}
555555

556556
// If main asset doesn't match, check sub-assets (e.g., Sprites in a Texture2D)
557-
UnityEngine.Object[] allAssets = AssetDatabase.LoadAllAssetsAtPath(path);
558-
if (allAssets != null)
557+
// Scene files crash LoadAllAssetsAtPath (ReadObjectThreaded not allowed)
558+
if (!IsScenePath(path))
559559
{
560-
for (int j = 0; j < allAssets.Length; j++)
560+
UnityEngine.Object[] allAssets = AssetDatabase.LoadAllAssetsAtPath(path);
561+
if (allAssets != null)
561562
{
562-
UnityEngine.Object subAsset = allAssets[j];
563-
if (subAsset != null && assetType.IsInstanceOfType(subAsset))
563+
for (int j = 0; j < allAssets.Length; j++)
564564
{
565-
instances.Add(subAsset);
565+
UnityEngine.Object subAsset = allAssets[j];
566+
if (subAsset != null && assetType.IsInstanceOfType(subAsset))
567+
{
568+
instances.Add(subAsset);
569+
}
566570
}
567571
}
568572
}
@@ -974,6 +978,12 @@ List<string> buffer
974978

975979
private static bool HasMatchingSubAsset(string path, Type assetType)
976980
{
981+
// Scene files crash LoadAllAssetsAtPath (ReadObjectThreaded not allowed)
982+
if (IsScenePath(path))
983+
{
984+
return false;
985+
}
986+
977987
UnityEngine.Object[] allAssets = AssetDatabase.LoadAllAssetsAtPath(path);
978988
if (allAssets == null || allAssets.Length <= 1)
979989
{
@@ -1492,5 +1502,14 @@ private static bool ShouldSkipPath(string assetPath)
14921502

14931503
return false;
14941504
}
1505+
1506+
private static bool IsScenePath(string assetPath)
1507+
{
1508+
return assetPath != null
1509+
&& (
1510+
assetPath.EndsWith(".unity", StringComparison.OrdinalIgnoreCase)
1511+
|| assetPath.EndsWith(".scenetemplate", StringComparison.OrdinalIgnoreCase)
1512+
);
1513+
}
14951514
}
14961515
}

Tests/Editor/AssetProcessors/DetectAssetChangeProcessorTests.cs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,74 @@ public void MixedHandlerTypesInSingleEventBatchProcessCorrectly()
826826
);
827827
}
828828

829+
[TestCase(".unity", TestName = "SceneFile.LowerCase.DoesNotCrash")]
830+
[TestCase(".Unity", TestName = "SceneFile.PascalCase.DoesNotCrash")]
831+
[TestCase(".UNITY", TestName = "SceneFile.UpperCase.DoesNotCrash")]
832+
[TestCase(".scenetemplate", TestName = "SceneFile.SceneTemplate.DoesNotCrash")]
833+
public void SceneFileImportDoesNotCrash(string extension)
834+
{
835+
// Regression test: Importing a .unity scene file should not crash.
836+
// Previously, HasMatchingSubAsset called AssetDatabase.LoadAllAssetsAtPath on
837+
// scene files, which triggers Unity's "Do not use ReadObjectThreaded on scene objects!" error.
838+
string fakeScenePath = TestRoot + "/TestScene" + extension;
839+
840+
ClearTestState();
841+
842+
Assert.DoesNotThrow(
843+
() =>
844+
DetectAssetChangeProcessor.ProcessChangesForTesting(
845+
new[] { fakeScenePath },
846+
null,
847+
null,
848+
null
849+
),
850+
$"Processing a '{extension}' scene file as an imported asset should not throw or crash"
851+
);
852+
853+
// Scene files are not TestDetectableAsset, so no handler should fire for created assets
854+
foreach (AssetChangeContext context in TestDetectAssetChangeHandler.RecordedContexts)
855+
{
856+
CollectionAssert.DoesNotContain(
857+
context.CreatedAssetPaths,
858+
fakeScenePath,
859+
$"Scene file '{fakeScenePath}' should not appear in created asset paths for a ScriptableObject watcher"
860+
);
861+
}
862+
}
863+
864+
[Test]
865+
public void SceneFileInMixedBatchDoesNotInterfereWithNormalAssets()
866+
{
867+
// Verify that a scene file in the same import batch does not prevent normal asset processing.
868+
string fakeScenePath = TestRoot + "/TestScene.unity";
869+
870+
CreatePayloadAssetAt(PayloadPath);
871+
ClearTestState();
872+
873+
DetectAssetChangeProcessor.ProcessChangesForTesting(
874+
new[] { PayloadPath, fakeScenePath },
875+
null,
876+
null,
877+
null
878+
);
879+
880+
Assert.GreaterOrEqual(
881+
TestDetectAssetChangeHandler.RecordedContexts.Count,
882+
1,
883+
$"Expected at least 1 handler invocation for valid asset '{PayloadPath}' in mixed batch with scene file, "
884+
+ $"but got {TestDetectAssetChangeHandler.RecordedContexts.Count}"
885+
);
886+
887+
foreach (AssetChangeContext context in TestDetectAssetChangeHandler.RecordedContexts)
888+
{
889+
CollectionAssert.DoesNotContain(
890+
context.CreatedAssetPaths,
891+
fakeScenePath,
892+
$"Scene file '{fakeScenePath}' should not appear in created asset paths when mixed with normal assets"
893+
);
894+
}
895+
}
896+
829897
[TestCase(false, false, true, true, TestName = "ChangeFlags.MovedOnly.HandlesMovedAsset")]
830898
public void MovedAssetFlagsDataDriven(
831899
bool hasCreated,

docs/images/unity-helpers-banner.svg

Lines changed: 1 addition & 1 deletion
Loading

package.json

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
22
"name": "com.wallstop-studios.unity-helpers",
3-
"version": "3.1.6",
3+
"version": "3.1.7",
44
"displayName": "Unity Helpers",
5-
"description": "Treasure chest of Unity developer tools ",
5+
"description": "Treasure chest of Unity developer tools",
66
"unity": "2021.3",
77
"keywords": [
88
"inspector-attributes",
@@ -40,8 +40,10 @@
4040
"url": "https://github.com/wallstop/unity-helpers/issues"
4141
},
4242
"author": "wallstop studios <[email protected]> (https://wallstopstudios.com)",
43-
"homepage": "https://github.com/wallstop/unity-helpers#readme",
44-
"main": "README.md",
43+
"homepage": "https://wallstop.github.io/unity-helpers",
44+
"documentationUrl": "https://wallstop.github.io/unity-helpers",
45+
"changelogUrl": "https://github.com/wallstop/unity-helpers/blob/main/CHANGELOG.md",
46+
"licensesUrl": "https://wallstop.github.io/unity-helpers/project/license/",
4547
"samples": [
4648
{
4749
"displayName": "DI – VContainer",

0 commit comments

Comments
 (0)