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

Recommendations are not sorted by download count #4004

Closed
JonnyOThan opened this issue Jan 23, 2024 · 3 comments · Fixed by #4007
Closed

Recommendations are not sorted by download count #4004

JonnyOThan opened this issue Jan 23, 2024 · 3 comments · Fixed by #4007
Labels
Enhancement New features or functionality GUI Issues affecting the interactive GUI

Comments

@JonnyOThan
Copy link
Contributor

image

If you install EnvironmentalVisualEnhancements (eve redux) directly, you get this window recommending a config. For some reason Whirligig World gets auto-selected, even though it has far fewer downloads than AVP or the stock configs.

@JonnyOThan JonnyOThan added Bug Something is not working as intended GUI Issues affecting the interactive GUI labels Jan 23, 2024
@HebaruSan
Copy link
Member

HebaruSan commented Jan 23, 2024

We applied the sorting to the prompt that chooses mods to satisfy a dependency (#3934), not recommendations, which is a separate screen. (EVE unfortunately recommends the EVE config virtual module instead of depending on it.) Sorting recommendations is questionable since normally they reflect the order specified in the mods' relationships.

I'll look into whether we can control the ordering of providing mods for a given virtual module, at least...

Update, the current ordering is not related to the ordering in the mods' relationships; the recommendations are stored in a dictionary, and the ordering is the pseudorandom ordering of the key value pairs, then sorted by the related mod column.

Sorting by download counts would certainly be an improvement over randomness, but sticking to the original ordering from the mods' relationships might be better, need to consider that for a bit...

Another update, the pseudorandom ordering comes from the HashSet values of Registry.providers, not the dictionary, which seems to preserve the ordering it gets from RelationshipResolver.

@HebaruSan HebaruSan removed the Bug Something is not working as intended label Jan 23, 2024
@HebaruSan HebaruSan changed the title Sorting recommendations & auto-selection seems broken Recommendations are not sorted by download count Jan 23, 2024
@HebaruSan HebaruSan added the Enhancement New features or functionality label Jan 23, 2024
@JonnyOThan
Copy link
Contributor Author

JonnyOThan commented Jan 23, 2024

Ah interesting. I don't have a strong feeling on whether they should be sorted by download count UNLESS it's via virtual module. Proposal: first level sorting is the order given in the netkan, then if any of those are a virtual module that subset of entries is sorted by download count.

What's the logic behind which one gets auto-selected then? Recommendations would always be auto-selected right? So why is only one entry checked here? Is it because checking all of them would be a conflict? Or is the same logic as described in #3934 coming into play where it's auto-selecting the top entry?

@HebaruSan
Copy link
Member

Ah interesting. I don't have a strong feeling on whether they should be sorted by download count UNLESS it's via virtual module. Proposal: first level sorting is the order given in the netkan, then if any of those are a virtual module that subset of entries is sorted by download count.

Yup, agreed. This is what I've tried to move towards in #4007.

What's the logic behind which one gets auto-selected then? Recommendations would always be auto-selected right? So why is only one entry checked here? Is it because checking all of them would be a conflict? Or is the same logic as described in #3934 coming into play where it's auto-selecting the top entry?

Each recommends relationship has one row that's checked by default. If it's a virtual module or an any_of list, it can also have several other rows that are unchecked by default. The KSP-RO folks in particular like this because they can recommend groups of mods that are mutually exclusive without creating confusion. The checked one is the first in the list of candidates, but the candidates for a virtual module were not arranged in any particular order up till now. As of #4007 it will be the one with the highest download count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New features or functionality GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants