Fix NullReferenceException when importing UST with unregistered flags #1909
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a NullReferenceException that occurs when importing UST files containing flags that are not registered in the project's expression list. The fix adds a null check before attempting to clone expression descriptors.
Key changes:
- Added null check for expression lookup result before cloning
- Unregistered flags are now silently ignored instead of causing a crash
- Added explanatory comments about the behavior
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // otherwise expression not found, often when importing UST into a new project | ||
| // ignore it. |
There was a problem hiding this comment.
The comment is grammatically incomplete. It should be a complete sentence with proper capitalization and punctuation.
| // otherwise expression not found, often when importing UST into a new project | |
| // ignore it. | |
| // Otherwise expression not found, often when importing UST into a new project. | |
| // Ignore it. |
| foreach (var flag in parser.Parse(parts[1].Trim())) { | ||
| var descriptor = project.expressions.Values.FirstOrDefault(exp => exp.flag == flag.Key).Clone(); | ||
| descriptor.CustomDefaultValue = flag.Value; | ||
| track.TrackExpressions.Add(descriptor); | ||
| var item = project.expressions.Values.FirstOrDefault(exp => exp.flag == flag.Key); | ||
| if (item != null) { | ||
| var descriptor = item.Clone(); | ||
| descriptor.CustomDefaultValue = flag.Value; | ||
| track.TrackExpressions.Add(descriptor); | ||
| } | ||
| // otherwise expression not found, often when importing UST into a new project | ||
| // ignore it. | ||
| } |
There was a problem hiding this comment.
This bug fix lacks test coverage. Consider adding a test case that verifies the handling of unregistered flags when importing UST files, ensuring that no NullReferenceException is thrown and that unknown flags are silently ignored.
|
This was already fixed in #1864 |
2 participants
Summary
This PR fixes a
System.NullReferenceExceptionthat occurs when parsing UST flags that are not registered in the project's expressions list.The Bug
When importing UST with flag into a new UProject, the parser identifies flags correctly, but the
FirstOrDefaultalways returnsnullsince no expressions are registered to the project.The Fix
Added a null check before cloning the expression descriptor. If an expression/flag is registered, it is now safely ignored.
Future Improvements / Suggestions
Since I am not very familiar with C#, I only fixed the crash with minimum modification. However, silently ignoring unknown flags is not ideal. A better long-term solution would be to implement a Flag Mapping Popup . When importing a UST with unknown flags, the user could be prompted to manually map them to valid/registered USTX flags.