-
-
Notifications
You must be signed in to change notification settings - Fork 345
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
Once again allow users to select dependencies when multiple provides exist #1002
Conversation
a5fa0a4
to
90c751a
Compare
@@ -9,12 +9,15 @@ | |||
<AssemblyName>CmdLine</AssemblyName> | |||
<StartupObject>CKAN.CmdLine.MainClass</StartupObject> | |||
<ApplicationIcon>..\GUI\assets\ckan.ico</ApplicationIcon> | |||
<TargetFrameworkVersion>v4.5</TargetFrameworkVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing we didn't have a target framework before? What would it default to without this? (My mind says that we were targeting dotnet40, but I've no idea if that's actually true.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the VS default is v4.0 but I'm not 100%
63054d2
to
21d2b98
Compare
@@ -333,6 +334,15 @@ private void Add(CkanModule module, Relationship reason) | |||
} | |||
} | |||
|
|||
private bool MightBeInstallable(CkanModule module) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Some docs on this would be great.
It looks like we recursively chase all dependencies and make sure we can potentially install them all in some iteration. (Although we don't check against conflicts against currently installed mods, since MightBeInstallable
vs AbsolutelyInstallable
. :)
Under Linux with mono 3.2.8 and 21d2b98, I get a hard lock whenever I select anything that requires a choice to be made. The GUI completely stops responding, the window can't be closed, and I have to kill it from the command-line. :( |
I can also reproduce the same lock-up under Mono 3.12.1 under Linux. |
21d2b98
to
6f1fcef
Compare
Fixes KSP-CKAN#976 Pops up the moment the user chooses a mod instead of waiting until the install. This way there can be accurate change sets and conflict detection.
Allows scrolling the error message and easier copying.
Now works. It deselects the mod which caused the screen to pop up. Also adds a test.
Windows seems to fire ItemChecked on creation, Linux does not. As such all checkbox where selected on Linux leading to errors. Fixes some of KSP-CKAN/CKAN-GUI#137 (comment)
When a selected provide mod it self triggered to many provides the next screen's cancel button was incorrectly cancelling the original mod.
Select mod for install was working on the filtered list instead of the whole. ModuleNotFound occurs because the registry's availablewithprovides happy returns mods which are not available. Long term fixing that would be the correct solution
Otherwise if a provides throws a InconsistentKraken the contents would leak.
Seems await in catch is breaking things.
See KSP-CKAN/CKAN-GUI#137 (comment) for description
6f1fcef
to
4b5d3e4
Compare
Testing 4b5d3e4:
This isn't a show-stopper (we're way better than we were before), so I'm continuing with review. |
Same commit, but more serious bug:
Selecting the cloud pack directly in the mod list does the right thing (gives me a change set with dependencies listed). A more minor behavioural surprise (and also not a merge-blocker) is that if we untick the cloud pack we immediately get asked to choose a cloud pack. That's kind of cool (because it keeps consistency), but users may be surprised by it (eg: either expecting either for the dependant package to be unchosen, or for that to be an option that's presented). |
Outerplanetsmod shows a similar behaviour. ----- Ursprüngliche Nachricht ----- Same commit, but more serious bug: |
Fixed in 2cbcdf1
Long term idea is to continue towards making RelationshipResolver work with multiple calls instead of the current one-shot everything in ctor method. Once this and adding mods to the provider with reasons are in it should be possible to use the reasonFor functionality to extract which mod is the dependant package. At which point it would be easy to change the GUI behaviour.
:worksOnMyMachine:. I'll break out the linux VM and check if it is a mono/.Net difference and investigate. |
Functioning on mono 4.0.1 from a empty install. |
@pjf Can you replicate the missing changeset tab with a empty install? |
Apologies for my absence over the last two days; life threw me a real curveball.
I cannot. And after some tracking, I discovered that I have This is a fault in my testing. Hooray! :D |
Merging every zig for great justice. :) |
Once again allow users to select dependencies when multiple provides exist
Fixes #976.
See: KSP-CKAN/CKAN-GUI#137 for some discussion.