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

Stablize sorts used by path finders #1805

Merged
merged 8 commits into from
Dec 16, 2024

Conversation

lostsquirrel1
Copy link
Collaborator

std::priority_que doesn't guarantee that the sorts it performs is stable. This could potentially lead to a desync between different platforms or build chains.

@lhog
Copy link
Collaborator

lhog commented Dec 8, 2024

Maybe move these comparisons to a single spaceship operator, since we're now in c++20 land?

@lostsquirrel1
Copy link
Collaborator Author

lostsquirrel1 commented Dec 8, 2024

Maybe move these comparisons to a single spaceship operator, since we're now in c++20 land?

These weak comparisons use different logic to the regular comparisons. I can add three-way comparison (as an addition), but it won't be used in the current code - maybe better to wait until other/future code needs it?

@lostsquirrel1 lostsquirrel1 force-pushed the BAR105-stable_sort_pathing_priority_queues branch from 0fce1b3 to b423e08 Compare December 8, 2024 20:56
rts/Sim/Path/QTPFS/PathThreads.h Outdated Show resolved Hide resolved
rts/Sim/Path/QTPFS/PathThreads.h Outdated Show resolved Hide resolved
@lostsquirrel1 lostsquirrel1 changed the title Stablize sorts used by QTPFS Stablize sorts used by path finders Dec 12, 2024
@jarmani
Copy link

jarmani commented Dec 12, 2024

Thanks you for fixing this guys ❤️

Can't wait to try multi from OpenBSD with non-OpenBSD players & friends

@saurtron saurtron added the status: candidate PRs that should be good to go or important for next release label Dec 16, 2024
@lhog lhog merged commit e4d0e47 into master Dec 16, 2024
2 checks passed
@lhog lhog deleted the BAR105-stable_sort_pathing_priority_queues branch December 16, 2024 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: candidate PRs that should be good to go or important for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants