fix: add null check for oldNode in GameEditorChangePropagator

[laske185]

PR Details

Added a null check for oldNode before invoking visitor.Visit(oldNode).
This change prevents potential null reference exceptions during execution.

The bug was introduced in the "User-defined assets support" feature in 5da9ae3 and is not in the release version.

Related Issue

Closes #2850

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)

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.

[Copilot]

Pull Request Overview

This PR fixes a potential null reference exception in the GameEditorChangePropagator by adding a null check before calling visitor.Visit(oldNode). The fix addresses a bug introduced in the "User-defined assets support" feature that could cause crashes during game editor operations.

• Added null check for oldNode parameter before visitor invocation
• Prevents null reference exceptions in the change propagation logic

Comments suppressed due to low confidence (1)

[tebjan]

Looks good, thanks for the detailed description!

[Eideren]

That part was authored by xen afair so I'm not 100% certain what this change entails, do either of you know why oldNode may be null when its value is not ? Not super likely but this could just be caused by another issue upstream and this change is just silencing it further.

If it is safe, shouldn't line 195 to 203 be included inside the if as well ?

[laske185]

In Line 193 the oldNode is returned by the Editor.NodeContainer for the OldValue from the change event. This value was in my cases, the old row or column of the UI element in the grid. For these values, the NodeContainers do not have any IObjectNode.

var oldNode = Editor.NodeContainer.GetNode(e.OldValue);

Maybe you are right, and it is safer when the complete code is inside the if statement.