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 crash when creating collections named "All beatmaps" or "Manage collections..." #25839

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Dec 18, 2023

Closes #25834

The name fallback that was there previously since #11892 was half broken. This way should be a lot less prone to failure.

…ollections..."

Closes ppy#25834

The name fallback that was there previously since
ppy#11892 was half broken. This way should
be a lot less prone to failure.
@bdach bdach added type:reliability next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! labels Dec 18, 2023

public override bool Equals(CollectionFilterMenuItem? other) => other is AllBeatmapsCollectionFilterMenuItem;

public override int GetHashCode() => 1;
Copy link
Member

Choose a reason for hiding this comment

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

scary but i'll assume there's some theory behind this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The way to think about it is as such: if two objects are equal, they must have the same hash code, but two objects with the same hash code are not necessarily equal.

So a constant hash code spec such as this ensures that all instances of the class have the same hash code, and since the equality member only checks type, this satisfies the first part.

There is no actual data to base the hash code implementation on. If anything it'll just be a little inefficient in a dictionary, but the assumption is that there's only ever going to be one of these per dropdown.

@peppy peppy self-requested a review December 18, 2023 15:23
Copy link
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

Can't crash at least

@peppy peppy merged commit da658bb into ppy:master Dec 18, 2023
@bdach bdach deleted the collection-name-clashes-crashes branch December 18, 2023 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! size/M type:reliability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash after creating collection called "All beatmaps"
2 participants