Skip to content

fix: Fix crash when deleting a folder of assets #2673

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

Jump to bottom
Merged
merged 5 commits into from
Mar 11, 2025
Merged

fix: Fix crash when deleting a folder of assets #2673

merged 5 commits into from
Mar 11, 2025

Conversation

NicusorN5
Copy link
Contributor

@NicusorN5 NicusorN5 commented Mar 10, 2025 •
edited
Loading

PR Details

Fixes a crash (specifically Null Reference exceptions) when deleting a folder filled with model assets, specifically "BlocksPrefabs" folder in a new project using asset packs.

This is a duct-tape-y fix, I'm not fully aware if this could break other things - despite just null checks and validation being written.

Related Issue

None, didn't bother to create a issue linked to this.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Stacktrace of solved exception

Exception: NullReferenceException: Object reference not set to an instance of an object.
   at Stride.Core.Assets.AssetReferenceDataSerializer.Serialize(AssetReference& assetReference, ArchiveMode mode, SerializationStream stream) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\assets\Stride.Core.Assets\AssetReferenceDataSerializer.cs:line 16
   at Stride.Core.Assets.BasePartDataSerializer.Serialize(BasePart& basePart, ArchiveMode mode, SerializationStream stream) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\assets\Stride.Core.Assets\BasePart.cs:line 85
   at Stride.Core.Serialization.MemberReuseSerializer`1.Serialize(T& obj, ArchiveMode mode, SerializationStream stream) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core\Serialization\MemberSerializerGenerated.cs:line 1225
   at Stride.Core.DataSerializers.StrideAssetsEntities_EntityDesignSerializer.Serialize(EntityDesign& obj, ArchiveMode mode, SerializationStream stream)
   at Stride.Core.Serialization.MemberReuseSerializer`1.Serialize(T& obj, ArchiveMode mode, SerializationStream stream) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core\Serialization\MemberSerializerGenerated.cs:line 1225
   at Stride.Core.Assets.AssetPartCollectionSerializer`2.Serialize(AssetPartCollection`2& obj, ArchiveMode mode, SerializationStream stream) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\assets\Stride.Core.Assets\AssetPartCollection.cs:line 85
   at Stride.Core.Serialization.MemberReuseSerializer`1.Serialize(T& obj, ArchiveMode mode, SerializationStream stream) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core\Serialization\MemberSerializerGenerated.cs:line 1225
   at Stride.Core.DataSerializers.StrideCoreAssets_AssetCompositeHierarchyDataSerializer`2.Serialize(AssetCompositeHierarchyData`2& obj, ArchiveMode mode, SerializationStream stream)
   at Stride.Core.Serialization.MemberReuseSerializer`1.Serialize(T& obj, ArchiveMode mode, SerializationStream stream) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core\Serialization\MemberSerializerGenerated.cs:line 1225
   at Stride.Core.DataSerializers.StrideCoreAssets_AssetCompositeHierarchySerializer`2.Serialize(AssetCompositeHierarchy`2& obj, ArchiveMode mode, SerializationStream stream)
   at Stride.Core.DataSerializers.StrideAssetsEntities_SceneAssetSerializer.Serialize(SceneAsset& obj, ArchiveMode mode, SerializationStream stream)
   at Stride.Core.Serialization.DataSerializer`1.Serialize(Object& obj, ArchiveMode mode, SerializationStream stream) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core\Serialization\DataSerializer.cs:line 80
   at Stride.Core.Serialization.MemberReuseSerializer`1.SerializeExtended(T& obj, ArchiveMode mode, SerializationStream stream, DataSerializer`1 dataSerializer) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core\Serialization\MemberSerializerGenerated.cs:line 1429
   at Stride.Core.Assets.AssetCloner..ctor(Object value, AssetClonerFlags flags, IEnumerable`1 externalIdentifiables) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\assets\Stride.Core.Assets\AssetCloner.cs:line 73
   at Stride.Core.Assets.AssetCloner.Clone(Object asset, AssetClonerFlags flags, HashSet`1 externalIdentifiable, Dictionary`2& idRemapping) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\assets\Stride.Core.Assets\AssetCloner.cs:line 225
   at Stride.Core.Assets.Analysis.AssetDependencyManager.Session_AssetDirtyChanged(AssetItem asset, Boolean oldValue, Boolean newValue) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\assets\Stride.Core.Assets\Analysis\AssetDependencyManager.cs:line 525
   at Stride.Core.Assets.Editor.ViewModel.AssetViewModel.OnDirtyFlagSet() in C:\BuildAgent\work\b5f46e3c4829a09e\sources\editor\Stride.Core.Assets.Editor\ViewModel\AssetViewModel.cs:line 349
   at Stride.Core.Presentation.ViewModels.DirtiableEditableViewModel.set_IsDirty(Boolean value) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\presentation\Stride.Core.Presentation\ViewModels\DirtiableEditableViewModel.cs:line 33
   at Stride.Core.Presentation.ViewModels.DirtiableEditableViewModel.UpdateDirtiness(Boolean value) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\presentation\Stride.Core.Presentation\ViewModels\DirtiableEditableViewModel.cs:line 47
   at Stride.Core.Presentation.ViewModels.DirtiableEditableViewModel.Stride.Core.Presentation.Dirtiables.IDirtiable.UpdateDirtiness(Boolean value) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\presentation\Stride.Core.Presentation\ViewModels\DirtiableEditableViewModel.cs:line 52
   at Stride.Core.Presentation.Dirtiables.DirtiableManager.UpdateDirtiables(HashSet`1 dirtiables) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\presentation\Stride.Core.Presentation\Dirtiables\DirtiableManager.cs:line 121
   at Stride.Core.Presentation.Dirtiables.DirtiableManager.TransactionCompleted(Object sender, TransactionEventArgs e) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\presentation\Stride.Core.Presentation\Dirtiables\DirtiableManager.cs:line 138
   at Stride.Core.Transactions.TransactionStack.CompleteTransaction(Transaction transaction) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core.Design\Transactions\TransactionStack.cs:line 202
   at Stride.Core.Transactions.Transaction.Complete() in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core.Design\Transactions\Transaction.cs:line 75
   at Stride.Core.Transactions.Transaction.Dispose() in C:\BuildAgent\work\b5f46e3c4829a09e\sources\core\Stride.Core.Design\Transactions\Transaction.cs:line 45
   at Stride.Core.Assets.Editor.ViewModel.SessionViewModel.DeleteItems(IReadOnlyCollection`1 locations, Boolean skipConfirmation) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\editor\Stride.Core.Assets.Editor\ViewModel\SessionViewModel.cs:line 1770
   at Stride.Core.Assets.Editor.ViewModel.AssetCollectionViewModel.DeleteContent(IReadOnlyCollection`1 locations, Boolean skipConfirmation) in C:\BuildAgent\work\b5f46e3c4829a09e\sources\editor\Stride.Core.Assets.Editor\ViewModel\AssetCollectionViewModel.cs:line 1350
   at Stride.Core.Presentation.Commands.AnonymousTaskCommand.<>c__DisplayClass0_0.<<-ctor>b__0>d.MoveNext() in C:\BuildAgent\work\b5f46e3c4829a09e\sources\presentation\Stride.Core.Presentation\Commands\AnonymousCommand.cs:line 84
--- End of stack trace from previous location ---
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.DispatcherOperation.InvokeImpl()
   at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(Object obj)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at MS.Internal.CulturePreservingExecutionContext.Run(CulturePreservingExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Windows.Threading.DispatcherOperation.Invoke()
   at System.Windows.Threading.Dispatcher.ProcessQueue()
   at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
   at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
   at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
   at System.Windows.Application.RunDispatcher(Object ignore)
   at System.Windows.Application.RunInternal(Window window)
   at Stride.GameStudio.Program.Main() in C:\BuildAgent\work\b5f46e3c4829a09e\sources\editor\Stride.GameStudio\Program.cs:line 148

How to reproduce:

  • Use latest Stride release (as of writing this).
  • Create a new project.
  • Include asset packs.
  • Remove "BlocksPrefabs" folder.
  • Click "Clear all references"
  • Crash! (this PR fixes this)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

@NicusorN5 NicusorN5 changed the title []Fix crash when deleting a folder of assets [fix] Fix crash when deleting a folder of assets Mar 10, 2025
@NicusorN5 NicusorN5 changed the title [fix] Fix crash when deleting a folder of assets fix: Fix crash when deleting a folder of assets Mar 10, 2025
@roku674
Copy link

roku674 commented Mar 10, 2025 •
edited
Loading

LGTM!

@net2cn

Thoughts?

Kryptos-FR
Kryptos-FR reviewed Mar 10, 2025
View reviewed changes
Copy link
Member

@Kryptos-FR Kryptos-FR left a comment

Choose a reason for hiding this comment

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

Probably a good fix, but uncertain about one particular line.

@NicusorN5
Copy link
Contributor Author

NicusorN5 commented Mar 10, 2025 •
edited
Loading

Further experimentation showed that only a single changed null check inside AssetCompositeHierarchyPropertyGraph was needed.

It would be pretty (for my eyes) to squash all the commits into one when merging.

IT'S BROKEN AGAIN, DO NOT MERGE!

@NicusorN5 NicusorN5 requested a review from Kryptos-FR March 10, 2025 20:56
@NicusorN5
Copy link
Contributor Author

Removing the null check in AssetReferenceDataSerializer.cs made an exception appear when removing a asset folder - but that only happened once, so I added it back. I forgot to check the stack trace, to check if it was something different.

I checked inside Stride by removing different default assets folders, adding assets, et cetera. Seems to be working now.

Ready for an other review.

Kryptos-FR
Kryptos-FR approved these changes Mar 10, 2025
View reviewed changes
Copy link
Member

@Kryptos-FR Kryptos-FR left a comment

Choose a reason for hiding this comment

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

It's likely there is a bug somewhere else as I don't think enumerating the parts should give one without a base.

Anyway, as it is the fix is a good double-check even if we manage to find the source of the problem later.

@Kryptos-FR Kryptos-FR merged commit 7b77b00 into stride3d:master Mar 11, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kryptos-FR Kryptos-FR Kryptos-FR approved these changes

Assignees
No one assigned
Labels
area-Asset
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@NicusorN5 @roku674 @Kryptos-FR @VaclavElias