Initialize checkboxes to desired value at creation

[HebaruSan]

This is a possible fix for #2162, and an alternative to #2183.

That issue happens because after a failed install/upgrade attempt, Main.PostInstallMods rebuilds the mod list and then re-applies the ModChanges object's changes to the checkboxes. But one of the checkboxes may no longer be a checkbox, and trying to access it as one causes a null reference exception.

This pull request eliminates the possibility of this happening. Rather than generating the check boxes in a default state and then applying a ModChanges after the fact, we now pass the ModChanges directly to Main.ConstructModList and use it to control the initial checkbox state, so the row object starts out consistent with the ModChanges.

ConstructModList is cleaned up a bit to make this happen. Previously it created the checkbox cells, set their values, and potentially set them as read-only in three separate statements per checkbox. Each of those lines has to have its own Boolean check for whether to show a checkbox or a "-" for actions that aren't allowed (incompatible mods, for example). If those checks ever got out of sync, we would risk the same kind of exceptions that were happening in this issue by treating a non-checkbox as a checkbox. And they sort of were out of sync! The creation of the Update checkbox depended on mod.HasUpdate && !mod.IsAutodetected, but the setting of its value used !mod.Isinstallable() || !mod.HasUpdate, which, in addition to being confusing, is not exactly the same condition.

Now each checkbox is set up in one single step; we create the checkbox and set its value property simultaneously using the {Property=value} syntax, under one single Boolean check, so there is no risk of them executing out of sync. (ReadOnly still has to be set separately due to .NET raising an exception, see review comments for details.)

As for the reason why the update checkbox was no longer a checkbox, @mwerle reports that AvailableModule.module_versions sometimes flipped its sort order, which caused it to consider the wrong mod version as the most recent, and therefore miss an available upgrade. This object is a SortedDictionary initialized to sort in descending order, but it's also an internal [JsonProperty], so it's possible that the deserializer (or some other object) might replace it with a SortedDictionary that uses ascending order instead (which is the default), which could cause such issues.

Since we can't force all objects everywhere to initialize their SortedDictionary objects to descending order before assigning them to AvailableModule.module_versions, AvailableModule is updated to use an ascending order sort instead. This way it will work even if some other object replaces the dictionary.

In addition, some weird or confusing things are cleaned up:

• GUIMod.IsInstalled() was implemented as an extension method in Util.cs, which made it harder to find without doing a find-in-files, and is not necessary because we own the GUIMod class and can do whatever we like with it. This method is now moved to being a normal public instance method of GUIMod.
• GUIMod.IsInstalled() was also fairly hard to understand, consisting of an OR of an OR expression and an AND expression, sharing one term, one of which had a NOT on the outside, and the other had a NOT on the inside. If you could tell what it did by glancing at it, I salute you. Now it's simplified to an equivalent sequence of checks that I think is much easier to read, with explanatory comments and XML documentation.

[mwerle]

This line causes an exception (Windows):

Unhandled exception:
System.InvalidOperationException: ReadOnly property of a cell cannot be set before it is added to a row.
at System.Windows.Forms.DataGridViewCell.set_ReadOnly(Boolean value)
at CKAN.MainModList.ConstructModList(IEnumerable1 modules, List1 mc, Boolean refreshAll, Boolean hideEpochs) in S:\KSP\CKAN\CKAN\GUI\MainModList.cs:line 537
at CKAN.Main._UpdateModsList(Boolean repo_updated, List1 mc) in S:\KSP\CKAN\CKAN\GUI\MainModList.cs:line 186 at CKAN.Main.<>c__DisplayClass233_0.<UpdateModsList>b__0() in S:\KSP\CKAN\CKAN\GUI\MainModList.cs:line 143 at CKAN.Util.Invoke[T](T obj, Action action) in S:\KSP\CKAN\CKAN\GUI\Util.cs:line 28 at CKAN.Main.UpdateModsList(Boolean repo_updated, List1 mc) in S:\KSP\CKAN\CKAN\GUI\MainModList.cs:line 143
at CKAN.Main.CurrentInstanceUpdated() in S:\KSP\CKAN\CKAN\GUI\Main.cs:line 441
at CKAN.Main.OnLoad(EventArgs e) in S:\KSP\CKAN\CKAN\GUI\Main.cs:line 347
at System.Windows.Forms.Form.OnCreateControl()
at System.Windows.Forms.Control.CreateControl(Boolean fIgnoreVisible)
at System.Windows.Forms.Control.CreateControl()
at System.Windows.Forms.Control.WmShowWindow(Message& m)
at System.Windows.Forms.Control.WndProc(Message& m)
at System.Windows.Forms.Form.WmShowWindow(Message& m)
at System.Windows.Forms.NativeWindow.DebuggableCallback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

[mwerle]

This line causes an exception (Windows) - see details in previous comment.

[mwerle]

These lines are still required given the exceptions thrown above.

[mwerle]

UPDATE
After a recompile I can no longer reproduce the below.

---

Also, I'm not sure why we set "ReadOnly" at all - if you try double-clicking on the cell, it ends up throwing a core exception (not in managed code), regardless of whether or not it's set as ReadOnly.

Unhandled exception:
System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'Main'.
at System.Windows.Forms.Control.CreateHandle()
at System.Windows.Forms.Form.CreateHandle()
at System.Windows.Forms.Control.get_Handle()
at System.Windows.Forms.Control.SetVisibleCore(Boolean value)
at System.Windows.Forms.Form.SetVisibleCore(Boolean value)
at System.Windows.Forms.Application.ThreadContext.RunMessageLoopInner(Int32 reason, ApplicationContext context)
at System.Windows.Forms.Application.ThreadContext.RunMessageLoop(Int32 reason, ApplicationContext context)
at CKAN.Main..ctor(String[] cmdlineArgs, GUIUser user, Boolean showConsole) in S:\KSP\CKAN\CKAN\GUI\Main.cs:line 249
at CKAN.GUI.Main_(String[] args, Boolean showConsole) in S:\KSP\CKAN\CKAN\GUI\Program.cs:line 37
at CKAN.GUI.Main(String[] args) in S:\KSP\CKAN\CKAN\GUI\Program.cs:line 17
at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ThreadHelper.ThreadStart()

[HebaruSan]

Yeah, on further inspection I don't think we need to set ReadOnly. The entire rest of the row consists of DataGridViewTextBoxCell objects without ReadOnly set, so that should be fine for these cells as well.

[mwerle]

Uh, just re-tested without setting ReadOnly, and yes we do. Not sure why the other cells don't enter text-edit mode (perhaps it's a column-setting?) on double-click, but these ones do and it causes issues if you start typing into them.

[HebaruSan]

Ahh, good point. The columns probably have to be editable to allow the user to change the checkboxes. I just deleted that commit, so now it's back to how you had it.

[mwerle]

And here we go, in Main.Designer.cs:469-534 all columns except the first two are set to read-only.