Skip to content
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.

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

Fix index -1 exception in Manage Instances #3800

Merged

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Mar 3, 2023

Problem

  1. Open the manage game instances dialog
  2. "Forget" all instances that have hours played (see Play Time #3543), if any; the Hours Played column will disappear if it was visible
  3. Now add an instance that has hours played (again see Play Time #3543 for how to set this up), you'll get an exception and the instance list will fail to populate:

image

image

Noticed during dev of #3797 (in relation to the Game column instead), but I think I recall sporadic reports of this without those changes that I couldn't pin down at the time, so it seemed worth splitting into its own fix.

Cause

The list view has two columns that are sometimes visible and sometimes hidden: Game and Hours Played. To hide them, we remove them from the list view. If they become visible again, we insert them using their Column.Index property, which unfortunately is -1 for any column not already in the list:

If the ColumnHeader is not contained within a ListView control this property returns a value of -1.

Inserting the column with an index of -1 throws an exception. This happens anytime a previously hidden column is supposed to become visible (including adding a KSP2 instance to a list of KSP1 instances in #3797).

Changes

Now instead of using the column's own index property, the show/hide helper function has an index parameter, into which we pass the index of the next column in the normal layout that is always visible. This makes the columns hide and reappear properly without exceptions.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix GUI Issues affecting the interactive GUI Pull request labels Mar 3, 2023
@HebaruSan HebaruSan mentioned this pull request Mar 3, 2023
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

Aha, yeah that makes sense. LGTM!

@HebaruSan HebaruSan merged commit 483bb28 into KSP-CKAN:master Mar 3, 2023
@HebaruSan HebaruSan deleted the fix/manage-instances-index-exception branch March 3, 2023 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Easy This is easy to fix GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants