Skip to content

Conversation

@Jeshibu
Copy link
Contributor

@Jeshibu Jeshibu commented Feb 18, 2024

This fixes #469

I have verified that:

  • These changes work, by building the extension and testing.
  • That the changes comply with the rules indicated in the repository.
  • Pull request is targeting master branch.

@Jeshibu Jeshibu changed the title Fixed 2 games both with empty collections resulting in 0 similarity (#469) Fixed 2 games both with empty collections resulting in 0 similarity Feb 18, 2024
@darklinkpower
Copy link
Owner

Changes like this should be done in the control game matching algorithm and not to that method, since it calculates the Jaccard Similarity and this change would make it inaccurate. I'm also not sure if the best approach is to give a fixed number in such cases but I'm open to suggestions to improve the algorithm and if you've checked that it works fine.

@Jeshibu
Copy link
Contributor Author

Jeshibu commented Feb 25, 2024

I've changed it so that it only uses fields that have values for at least one of the games.
Fields with 0 (non-ignored) items in both lists now do not contribute to the similarity score, but also don't contribute to the minimum similarity score threshold.

I've also tweaked the minimum similarity threshold, but I want to put that in an advanced settings tab, along with the field weights, if you think this is the right way to go.

@Jeshibu
Copy link
Contributor Author

Jeshibu commented Mar 23, 2024

Added settings. If this is about right I'll add localization too.

@Jeshibu Jeshibu requested a review from darklinkpower March 25, 2024 20:58
Comment on lines +198 to +200
settings.SimilarGamesControlSettings.FieldSettings.Add(new SimilarGamesFieldSettings(GameField.TagIds, GetResourceString("LOCTagsLabel"), true, 1));
settings.SimilarGamesControlSettings.FieldSettings.Add(new SimilarGamesFieldSettings(GameField.GenreIds, GetResourceString("LOCGenresLabel"), true, 1.2));
settings.SimilarGamesControlSettings.FieldSettings.Add(new SimilarGamesFieldSettings(GameField.CategoryIds, GetResourceString("LOCCategoriesLabel"), true, 1.3));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the names should be set during their creation. This creates the issue that if language or source string changes, they'll stay the previous incorrect name so it would be better to resolve at runtime.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, if the settings for each field are being made separate here, in my opinion it would make sense to also move the item exclusion functionality there too:

image

<Expander Margin="0,10,0,0" Header="Advanced">
<StackPanel>
<StackPanel Orientation="Horizontal" Margin="0,10,0,0">
<TextBlock VerticalAlignment="Center">Jacard similarity requirement per field:</TextBlock>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing translation string

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the string should remain only as "Minimum similarity to match games" with a more detailed explanation in a tooltip, including saying that it uses Jacard.

Width="200" VerticalAlignment="Center"/>
</StackPanel>

<TextBlock Text="Fields enabled/weights:" Margin="0,10,0,0"/>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like much the text here. Perhaps it should say "Individual fields settings"

@Jeshibu Jeshibu force-pushed the match-empty-collections branch from bac70a3 to 1d40c3b Compare June 12, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Game Relations] Improvements to similar game matching algorithm e.g. when fields are missing

2 participants