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 exception at startup w/o default game inst #3863

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

HebaruSan
Copy link
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
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
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