Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Right after #4171, we got a report of an index out of bounds exception in the manage game instances dialog, which was fixed in 2ddf740. Today another came up in a different method of the same class.
Cause
Lines like this cause warnings (which we promote to errors) with nullable reference enabled:
CKAN/GUI/Dialogs/ManageGameInstancesDialog.cs
Line 352 in 82e5c35
We need to check whether
GameInstancesListView.SelectedItems[0].Tag
is actually a string rather than null before we cast it, so I did that:CKAN/GUI/Dialogs/ManageGameInstancesDialog.cs
Lines 361 to 366 in c13930f
But now we're evaluating
GameInstancesListView.SelectedItems[0]
beforeHasSelections
runs to confirm that list is non-empty, so when it is empty, it's an array bounds error.Ideal solution and why we can't do it
C# 11 provides a great way to simplify checking array bounds and eliminate hard coded indexes:
Initially I did a code search to find every instance of a hard coded array index and worked up a set of changes to replace them with list patterns, but I hit a snag when I tried to build it with Mono:
So we need a solution that works on C# 9.
Changes
Now every hard coded array index access is replaced by:
If we are ever able to migrate to C# 11, we will have the option of switching to the list patterns that are already there in these comments.