Skip to content

Fix exception at startup w/o default game inst#3863

Merged
HebaruSan merged 1 commit into
KSP-CKAN:masterfrom
HebaruSan:fix/no-default-startup-crash
Jul 21, 2023
Merged

Fix exception at startup w/o default game inst#3863
HebaruSan merged 1 commit into
KSP-CKAN:masterfrom
HebaruSan:fix/no-default-startup-crash

Conversation

@HebaruSan
Copy link
Copy Markdown
Member

@HebaruSan HebaruSan commented Jul 21, 2023

Problems

If you have multiple game instances and no default game instance, the Manage Game Instances window appears at startup. In v1.33.0, this exception is thrown after you select an instance:

System.ArgumentException: Empty path name is not legal.
   at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding, Int32 bufferSize, Boolean checkHost)
   at System.IO.StreamWriter..ctor(String path)
   at CKAN.GUI.GUIConfiguration.SaveConfiguration(GUIConfiguration configuration)
   at CKAN.GUI.Main.CurrentInstanceUpdated(Boolean allowRepoUpdate)
   at System.ComponentModel.BackgroundWorker.OnRunWorkerCompleted(RunWorkerCompletedEventArgs e)

There's also a "Check for updates" prompt that will happen every time.

(Everything works fine if "Set as default" is checked for some instance.)

Initially reported by a Discord who prefers to remain anonymous, then Poppa Wheelie on the forum.

Causes

As of #3829, when you have no default instance, we use GUIConfiguration's constructor to get the window geometry defaults from an instance of the class that doesn't correspond to a real file on disk, meaning its path field is null.

  • Main.CurrentInstanceUpdated always saves the current GUIConfiguration object before loading the next one, but this causes an attempt to write to a null file path with the special instance described above
  • The "Check for updates" prompt also uses the special GUIConfiguration instance, which can't save

Changes

  • Now GUIConfiguration.Save will abort if GUIConfiguration.path is null or empty, which will prevent the exception from occurring
  • Now the "Check for updates" prompt is moved to Main.CurrentInstanceUpdated, where it will have a valid GUIConfiguration object and therefore always be able to save your choice

Fixes #3862.

We should do a hotfix release after this merged. I'm thinking v1.33.2, "Laplace" Δ.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix GUI Issues affecting the interactive GUI labels Jul 21, 2023
@HebaruSan HebaruSan requested a review from techman83 July 21, 2023 19:55
@HebaruSan
Copy link
Copy Markdown
Member Author

HebaruSan commented Jul 21, 2023

@LunarHackett003 and @Kronosiesium, you can try out a dev build under the Artifacts dropdown here to confirm the fix works:

https://github.com/KSP-CKAN/CKAN/pull/3863/checks

@HebaruSan
Copy link
Copy Markdown
Member Author

I would like to ship a hotfix release before this impacts more users; self-reviewing...

@HebaruSan HebaruSan merged commit 57f786e into KSP-CKAN:master Jul 21, 2023
@HebaruSan HebaruSan deleted the fix/no-default-startup-crash branch July 21, 2023 21:11
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.

[Bug]: Empty path name is not legal with no default game instance

1 participant