From ea201b617326949d26b310b61ab56872a77ecca7 Mon Sep 17 00:00:00 2001 From: Ari Vuollet Date: Mon, 25 Sep 2023 22:05:37 +0300 Subject: [PATCH 1/3] Fix null check in SceneAnimationWindow The null-conditional operator checks for reference equality of the Object, but doesn't check the unmanaged pointer validity. --- Source/Editor/Windows/Assets/SceneAnimationWindow.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Editor/Windows/Assets/SceneAnimationWindow.cs b/Source/Editor/Windows/Assets/SceneAnimationWindow.cs index 824928743..0d7fa1c78 100644 --- a/Source/Editor/Windows/Assets/SceneAnimationWindow.cs +++ b/Source/Editor/Windows/Assets/SceneAnimationWindow.cs @@ -821,7 +821,7 @@ namespace FlaxEditor.Windows.Assets if (_timeline.IsModified) { var time = _timeline.CurrentTime; - var isPlaying = _previewPlayer?.IsPlaying ?? false; + var isPlaying = _previewPlayer != null ? _previewPlayer.IsPlaying : false; _timeline.Save(_asset); if (_previewButton.Checked && _previewPlayer != null) { From 58445f04c4aa3cd6d06e5212d895287c03e8bc1b Mon Sep 17 00:00:00 2001 From: Ari Vuollet Date: Mon, 25 Sep 2023 23:06:14 +0300 Subject: [PATCH 2/3] Fix potential incorrect null checks in `FlaxEngine.Object`s The null-conditional operator checks for reference equality of the Object, but doesn't check the validity of the unmanaged pointer. This check is corrected in cases where the object was not immediately returned from the bindings layer and may have been destroyed earlier. --- .../CustomEditors/Dedicated/ScriptsEditor.cs | 4 ++-- Source/Editor/EditorAssets.cs | 4 +++- .../Editor/GUI/Timeline/SceneAnimationTimeline.cs | 4 ++-- Source/Editor/Tools/Foliage/FoliageTypesTab.cs | 2 +- Source/Editor/Tools/Foliage/PaintTab.cs | 2 +- Source/Editor/Tools/VertexPainting.cs | 2 +- Source/Editor/Undo/Actions/PasteActorsAction.cs | 2 +- .../Viewport/Previews/ParticleSystemPreview.cs | 4 ++-- Source/Editor/Viewport/Previews/TexturePreview.cs | 8 ++++---- .../Windows/Assets/MaterialInstanceWindow.cs | 14 +++++++++----- Source/Editor/Windows/Assets/ModelWindow.cs | 6 +++--- .../Editor/Windows/Assets/ParticleSystemWindow.cs | 2 +- .../Editor/Windows/Assets/SceneAnimationWindow.cs | 2 +- Source/Editor/Windows/Assets/SkinnedModelWindow.cs | 6 +++--- Source/Engine/UI/GUI/Brushes/GPUTextureBrush.cs | 2 +- Source/Engine/UI/GUI/Brushes/TextureBrush.cs | 2 +- Source/Engine/UI/UICanvas.cs | 3 ++- Source/Engine/UI/UIControl.cs | 8 ++++---- 18 files changed, 42 insertions(+), 35 deletions(-) diff --git a/Source/Editor/CustomEditors/Dedicated/ScriptsEditor.cs b/Source/Editor/CustomEditors/Dedicated/ScriptsEditor.cs index bf82e19da..acd27d7ef 100644 --- a/Source/Editor/CustomEditors/Dedicated/ScriptsEditor.cs +++ b/Source/Editor/CustomEditors/Dedicated/ScriptsEditor.cs @@ -576,8 +576,8 @@ namespace FlaxEditor.CustomEditors.Dedicated return; for (int j = 0; j < e.Length; j++) { - var t1 = scripts[j]?.TypeName; - var t2 = e[j]?.TypeName; + var t1 = scripts[j] != null ? scripts[j].TypeName : null; + var t2 = e[j] != null ? e[j].TypeName : null; if (t1 != t2) return; } diff --git a/Source/Editor/EditorAssets.cs b/Source/Editor/EditorAssets.cs index 7fa920ca6..eb2f21356 100644 --- a/Source/Editor/EditorAssets.cs +++ b/Source/Editor/EditorAssets.cs @@ -36,7 +36,9 @@ namespace FlaxEditor public static void OnEditorOptionsChanged(Options.EditorOptions options) { - var param = _highlightMaterial?.GetParameter("Color"); + if (!_highlightMaterial) + return; + var param = _highlightMaterial.GetParameter("Color"); if (param != null) param.Value = options.Visual.HighlightColor; } diff --git a/Source/Editor/GUI/Timeline/SceneAnimationTimeline.cs b/Source/Editor/GUI/Timeline/SceneAnimationTimeline.cs index c5989cca5..76b6ca246 100644 --- a/Source/Editor/GUI/Timeline/SceneAnimationTimeline.cs +++ b/Source/Editor/GUI/Timeline/SceneAnimationTimeline.cs @@ -33,7 +33,7 @@ namespace FlaxEditor.GUI.Timeline /// public SceneAnimationPlayer Player { - get => _player; + get => _player != null ? _player : null; set { if (_player == value) @@ -268,7 +268,7 @@ namespace FlaxEditor.GUI.Timeline /// public override void OnSeek(int frame) { - if (_player?.Animation) + if (_player != null && _player.Animation) { _player.Animation.WaitForLoaded(); _player.Time = frame / _player.Animation.FramesPerSecond; diff --git a/Source/Editor/Tools/Foliage/FoliageTypesTab.cs b/Source/Editor/Tools/Foliage/FoliageTypesTab.cs index 2a51d972c..8934f5058 100644 --- a/Source/Editor/Tools/Foliage/FoliageTypesTab.cs +++ b/Source/Editor/Tools/Foliage/FoliageTypesTab.cs @@ -375,7 +375,7 @@ namespace FlaxEditor.Tools.Foliage private void OnModified() { - Editor.Instance.Scene.MarkSceneEdited(_proxy.Foliage?.Scene); + Editor.Instance.Scene.MarkSceneEdited(_proxy.Foliage != null ? _proxy.Foliage.Scene : null); } private void OnSelectedFoliageChanged() diff --git a/Source/Editor/Tools/Foliage/PaintTab.cs b/Source/Editor/Tools/Foliage/PaintTab.cs index e4f01661b..ba31ca18b 100644 --- a/Source/Editor/Tools/Foliage/PaintTab.cs +++ b/Source/Editor/Tools/Foliage/PaintTab.cs @@ -218,7 +218,7 @@ namespace FlaxEditor.Tools.Foliage private void OnModified() { - Editor.Instance.Scene.MarkSceneEdited(_proxy.Foliage?.Scene); + Editor.Instance.Scene.MarkSceneEdited(_proxy.Foliage != null ? _proxy.Foliage.Scene : null); } private void OnSelectedFoliageChanged() diff --git a/Source/Editor/Tools/VertexPainting.cs b/Source/Editor/Tools/VertexPainting.cs index 0863569c2..6934a754b 100644 --- a/Source/Editor/Tools/VertexPainting.cs +++ b/Source/Editor/Tools/VertexPainting.cs @@ -544,7 +544,7 @@ namespace FlaxEditor.Tools public override bool IsControllingMouse => IsPainting; /// - public override BoundingSphere FocusBounds => _selectedModel?.Sphere ?? base.FocusBounds; + public override BoundingSphere FocusBounds => _selectedModel != null ? _selectedModel.Sphere : base.FocusBounds; /// public override void Update(float dt) diff --git a/Source/Editor/Undo/Actions/PasteActorsAction.cs b/Source/Editor/Undo/Actions/PasteActorsAction.cs index 8b0d040d8..7aee40878 100644 --- a/Source/Editor/Undo/Actions/PasteActorsAction.cs +++ b/Source/Editor/Undo/Actions/PasteActorsAction.cs @@ -144,7 +144,7 @@ namespace FlaxEditor.Actions { var node = nodeParents[i]; var actor = node.Actor; - var parent = actor?.Parent; + var parent = actor != null ? actor.Parent : null; if (parent != null) { bool IsNameValid(string name) diff --git a/Source/Editor/Viewport/Previews/ParticleSystemPreview.cs b/Source/Editor/Viewport/Previews/ParticleSystemPreview.cs index fc7fcdbe1..c6ce5a7b5 100644 --- a/Source/Editor/Viewport/Previews/ParticleSystemPreview.cs +++ b/Source/Editor/Viewport/Previews/ParticleSystemPreview.cs @@ -68,7 +68,7 @@ namespace FlaxEditor.Viewport.Previews /// public bool ShowBounds { - get => _boundsModel?.IsActive ?? false; + get => _boundsModel != null ? _boundsModel.IsActive : false; set { if (value == ShowBounds) @@ -110,7 +110,7 @@ namespace FlaxEditor.Viewport.Previews /// public bool ShowOrigin { - get => _originModel?.IsActive ?? false; + get => _originModel != null ? _originModel.IsActive : false; set { if (value == ShowOrigin) diff --git a/Source/Editor/Viewport/Previews/TexturePreview.cs b/Source/Editor/Viewport/Previews/TexturePreview.cs index a33a61f86..ec10bb019 100644 --- a/Source/Editor/Viewport/Previews/TexturePreview.cs +++ b/Source/Editor/Viewport/Previews/TexturePreview.cs @@ -500,7 +500,7 @@ namespace FlaxEditor.Viewport.Previews /// protected override void CalculateTextureRect(out Rectangle rect) { - CalculateTextureRect(_asset?.Size ?? new Float2(100), Size, out rect); + CalculateTextureRect(_asset != null ? _asset.Size : new Float2(100), Size, out rect); } /// @@ -549,7 +549,7 @@ namespace FlaxEditor.Viewport.Previews /// protected override void CalculateTextureRect(out Rectangle rect) { - CalculateTextureRect(_asset?.Size ?? new Float2(100), Size, out rect); + CalculateTextureRect(_asset != null ? _asset.Size : new Float2(100), Size, out rect); } /// @@ -604,7 +604,7 @@ namespace FlaxEditor.Viewport.Previews /// protected override void CalculateTextureRect(out Rectangle rect) { - CalculateTextureRect(_asset?.Size ?? new Float2(100), Size, out rect); + CalculateTextureRect(_asset != null ? _asset.Size : new Float2(100), Size, out rect); } /// @@ -659,7 +659,7 @@ namespace FlaxEditor.Viewport.Previews /// protected override void CalculateTextureRect(out Rectangle rect) { - CalculateTextureRect(_asset?.Size ?? new Float2(100), Size, out rect); + CalculateTextureRect(_asset != null ? _asset.Size : new Float2(100), Size, out rect); } /// diff --git a/Source/Editor/Windows/Assets/MaterialInstanceWindow.cs b/Source/Editor/Windows/Assets/MaterialInstanceWindow.cs index d5ab2ae8a..5f1273999 100644 --- a/Source/Editor/Windows/Assets/MaterialInstanceWindow.cs +++ b/Source/Editor/Windows/Assets/MaterialInstanceWindow.cs @@ -69,7 +69,7 @@ namespace FlaxEditor.Windows.Assets [EditorDisplay("General"), Tooltip("The base material used to override it's properties")] public MaterialBase BaseMaterial { - get => Window?.Asset?.BaseMaterial; + get => Window?.Asset != null ? Window?.Asset.BaseMaterial : null; set { var asset = Window?.Asset; @@ -101,10 +101,12 @@ namespace FlaxEditor.Windows.Assets [HideInEditor] public object[] Values { - get => Window?.Asset?.Parameters.Select(x => x.Value).ToArray(); + get => Window?.Asset != null ? Window?.Asset.Parameters.Select(x => x.Value).ToArray() : null; set { - var parameters = Window?.Asset?.Parameters; + if (Window?.Asset == null) + return; + var parameters = Window?.Asset.Parameters; if (value != null && parameters != null) { if (value.Length != parameters.Length) @@ -131,9 +133,11 @@ namespace FlaxEditor.Windows.Assets [HideInEditor] public FlaxEngine.Object[] ValuesRef { - get => Window?.Asset?.Parameters.Select(x => x.Value as FlaxEngine.Object).ToArray(); + get => Window?.Asset != null ? Window?.Asset.Parameters.Select(x => x.Value as FlaxEngine.Object).ToArray() : null; set { + if (Window?.Asset == null) + return; var parameters = Window?.Asset?.Parameters; if (value != null && parameters != null) { @@ -293,7 +297,7 @@ namespace FlaxEditor.Windows.Assets var p = (MaterialParameter)e.Tag; // Try to get default value (from the base material) - var pBase = baseMaterial?.GetParameter(p.Name); + var pBase = baseMaterial != null ? baseMaterial.GetParameter(p.Name) : null; if (pBase != null && pBase.ParameterType == p.ParameterType) { valueContainer.SetDefaultValue(pBase.Value); diff --git a/Source/Editor/Windows/Assets/ModelWindow.cs b/Source/Editor/Windows/Assets/ModelWindow.cs index f70f6a3b5..c2764a5a6 100644 --- a/Source/Editor/Windows/Assets/ModelWindow.cs +++ b/Source/Editor/Windows/Assets/ModelWindow.cs @@ -134,7 +134,7 @@ namespace FlaxEditor.Windows.Assets if (Window._skipEffectsGuiEvents) return; - Window._isolateIndex = mesh?.MaterialSlotIndex ?? -1; + Window._isolateIndex = mesh != null ? mesh.MaterialSlotIndex : -1; Window.UpdateEffectsOnAsset(); UpdateEffectsOnUI(); } @@ -144,7 +144,7 @@ namespace FlaxEditor.Windows.Assets if (Window._skipEffectsGuiEvents) return; - Window._highlightIndex = mesh?.MaterialSlotIndex ?? -1; + Window._highlightIndex = mesh != null ? mesh.MaterialSlotIndex : -1; Window.UpdateEffectsOnAsset(); UpdateEffectsOnUI(); } @@ -326,7 +326,7 @@ namespace FlaxEditor.Windows.Assets [EditorOrder(10), EditorDisplay("Materials", EditorDisplayAttribute.InlineStyle)] public MaterialSlot[] MaterialSlots { - get => Asset?.MaterialSlots; + get => Asset != null ? Asset.MaterialSlots : null; set { if (Asset != null) diff --git a/Source/Editor/Windows/Assets/ParticleSystemWindow.cs b/Source/Editor/Windows/Assets/ParticleSystemWindow.cs index 328369680..17eda1358 100644 --- a/Source/Editor/Windows/Assets/ParticleSystemWindow.cs +++ b/Source/Editor/Windows/Assets/ParticleSystemWindow.cs @@ -187,7 +187,7 @@ namespace FlaxEditor.Windows.Assets base.Initialize(layout); var emitterTrack = Values[0] as EmitterTrackProxy; - if (emitterTrack?._effect?.Parameters == null) + if (emitterTrack?._effect == null || emitterTrack?._effect.Parameters == null) return; var group = layout.Group("Parameters"); diff --git a/Source/Editor/Windows/Assets/SceneAnimationWindow.cs b/Source/Editor/Windows/Assets/SceneAnimationWindow.cs index 0d7fa1c78..162944144 100644 --- a/Source/Editor/Windows/Assets/SceneAnimationWindow.cs +++ b/Source/Editor/Windows/Assets/SceneAnimationWindow.cs @@ -792,7 +792,7 @@ namespace FlaxEditor.Windows.Assets { if (_previewButton.Checked) return; - _previewPlayerPicker.Value = _timeline.Player; + _previewPlayerPicker.Value = _timeline.Player != null ? _timeline.Player : null; _cachedPlayerId = _timeline.Player?.ID ?? Guid.Empty; } diff --git a/Source/Editor/Windows/Assets/SkinnedModelWindow.cs b/Source/Editor/Windows/Assets/SkinnedModelWindow.cs index a7ee6e767..d360e5570 100644 --- a/Source/Editor/Windows/Assets/SkinnedModelWindow.cs +++ b/Source/Editor/Windows/Assets/SkinnedModelWindow.cs @@ -151,7 +151,7 @@ namespace FlaxEditor.Windows.Assets if (Window._skipEffectsGuiEvents) return; - Window._isolateIndex = mesh?.MaterialSlotIndex ?? -1; + Window._isolateIndex = mesh != null ? mesh.MaterialSlotIndex : -1; Window.UpdateEffectsOnAsset(); UpdateEffectsOnUI(); } @@ -165,7 +165,7 @@ namespace FlaxEditor.Windows.Assets if (Window._skipEffectsGuiEvents) return; - Window._highlightIndex = mesh?.MaterialSlotIndex ?? -1; + Window._highlightIndex = mesh != null ? mesh.MaterialSlotIndex : -1; Window.UpdateEffectsOnAsset(); UpdateEffectsOnUI(); } @@ -415,7 +415,7 @@ namespace FlaxEditor.Windows.Assets [EditorOrder(10), EditorDisplay("Materials", EditorDisplayAttribute.InlineStyle)] public MaterialSlot[] MaterialSlots { - get => Asset?.MaterialSlots; + get => Asset != null ? Asset.MaterialSlots : null; set { if (Asset != null) diff --git a/Source/Engine/UI/GUI/Brushes/GPUTextureBrush.cs b/Source/Engine/UI/GUI/Brushes/GPUTextureBrush.cs index 9ad9db1d3..d3469670a 100644 --- a/Source/Engine/UI/GUI/Brushes/GPUTextureBrush.cs +++ b/Source/Engine/UI/GUI/Brushes/GPUTextureBrush.cs @@ -37,7 +37,7 @@ namespace FlaxEngine.GUI } /// - public Float2 Size => Texture?.Size ?? Float2.Zero; + public Float2 Size => Texture != null ? Texture.Size : Float2.Zero; /// public void Draw(Rectangle rect, Color color) diff --git a/Source/Engine/UI/GUI/Brushes/TextureBrush.cs b/Source/Engine/UI/GUI/Brushes/TextureBrush.cs index d49e5519b..755f7527b 100644 --- a/Source/Engine/UI/GUI/Brushes/TextureBrush.cs +++ b/Source/Engine/UI/GUI/Brushes/TextureBrush.cs @@ -104,7 +104,7 @@ namespace FlaxEngine.GUI } /// - public Float2 Size => Texture?.Size ?? Float2.Zero; + public Float2 Size => Texture != null ? Texture.Size : Float2.Zero; /// public unsafe void Draw(Rectangle rect, Color color) diff --git a/Source/Engine/UI/UICanvas.cs b/Source/Engine/UI/UICanvas.cs index 8cb43af62..edbec7f7d 100644 --- a/Source/Engine/UI/UICanvas.cs +++ b/Source/Engine/UI/UICanvas.cs @@ -493,7 +493,8 @@ namespace FlaxEngine if (_renderer) { #if FLAX_EDITOR - _editorTask?.RemoveCustomPostFx(_renderer); + if (_editorTask != null) + _editorTask.RemoveCustomPostFx(_renderer); #endif SceneRenderTask.RemoveGlobalCustomPostFx(_renderer); _renderer.Canvas = null; diff --git a/Source/Engine/UI/UIControl.cs b/Source/Engine/UI/UIControl.cs index dc76122c0..219a4fa4e 100644 --- a/Source/Engine/UI/UIControl.cs +++ b/Source/Engine/UI/UIControl.cs @@ -204,7 +204,7 @@ namespace FlaxEngine up = value; Internal_SetNavTargets(__unmanagedPtr, GetUnmanagedPtr(up), GetUnmanagedPtr(down), GetUnmanagedPtr(left), GetUnmanagedPtr(right)); if (_control != null) - _control.NavTargetUp = value?.Control; + _control.NavTargetUp = value != null ? value.Control : null; } } @@ -228,7 +228,7 @@ namespace FlaxEngine down = value; Internal_SetNavTargets(__unmanagedPtr, GetUnmanagedPtr(up), GetUnmanagedPtr(down), GetUnmanagedPtr(left), GetUnmanagedPtr(right)); if (_control != null) - _control.NavTargetDown = value?.Control; + _control.NavTargetDown = value != null ? value.Control : null; } } @@ -252,7 +252,7 @@ namespace FlaxEngine left = value; Internal_SetNavTargets(__unmanagedPtr, GetUnmanagedPtr(up), GetUnmanagedPtr(down), GetUnmanagedPtr(left), GetUnmanagedPtr(right)); if (_control != null) - _control.NavTargetLeft = value?.Control; + _control.NavTargetLeft = value != null ? value.Control : null; } } @@ -276,7 +276,7 @@ namespace FlaxEngine right = value; Internal_SetNavTargets(__unmanagedPtr, GetUnmanagedPtr(up), GetUnmanagedPtr(down), GetUnmanagedPtr(left), GetUnmanagedPtr(right)); if (_control != null) - _control.NavTargetRight = value?.Control; + _control.NavTargetRight = value != null ? value.Control : null; } } From 96d880df6a9d8c95cbaf7de056f99f3c88fea0c7 Mon Sep 17 00:00:00 2001 From: Ari Vuollet Date: Thu, 28 Sep 2023 21:49:39 +0300 Subject: [PATCH 3/3] Fix crash in SceneAnimationPlayer --- .../Engine/Animations/SceneAnimations/SceneAnimationPlayer.cpp | 2 +- Source/Engine/Engine/NativeInterop.Unmanaged.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Engine/Animations/SceneAnimations/SceneAnimationPlayer.cpp b/Source/Engine/Animations/SceneAnimations/SceneAnimationPlayer.cpp index fa7999ce0..8fef8e6d3 100644 --- a/Source/Engine/Animations/SceneAnimations/SceneAnimationPlayer.cpp +++ b/Source/Engine/Animations/SceneAnimations/SceneAnimationPlayer.cpp @@ -905,7 +905,7 @@ void SceneAnimationPlayer::Tick(SceneAnimation* anim, float time, float dt, int3 MException ex(exception); ex.Log(LogType::Error, TEXT("Property")); } - else if (!MCore::Type::IsPointer(valueType)) + else if (!MCore::Type::IsPointer(valueType) && !MCore::Type::IsReference(valueType)) { if (boxed) Platform::MemoryCopy(value, MCore::Object::Unbox(boxed), valueSize); diff --git a/Source/Engine/Engine/NativeInterop.Unmanaged.cs b/Source/Engine/Engine/NativeInterop.Unmanaged.cs index a2647dd10..54c1c21ec 100644 --- a/Source/Engine/Engine/NativeInterop.Unmanaged.cs +++ b/Source/Engine/Engine/NativeInterop.Unmanaged.cs @@ -1230,7 +1230,7 @@ namespace FlaxEngine.Interop internal static bool GetTypeIsReference(ManagedHandle typeHandle) { Type type = Unsafe.As(typeHandle.Target); - return type.IsByRef; + return !type.IsValueType; // Maybe also type.IsByRef? } [UnmanagedCallersOnly]