6665: Fix ordering of unfinished/unconnected tees r=heinrich5991 a=Learath2

<!-- What is the motivation for the changes of this pull request? -->
I thought there was initially a strict weak ordering issue here but upon further inspection noticed it's fine.
However, people with negative score were getting sorted undeterministically, basically every unfinished tee was treated as equal since `!comp(a, b) && !comp(b, a)`. Sorting them by name was the intention.

I also got rid of the old `INT_MIN` "hack", it was only there to make the code shorter and it's no longer possible with the newly flipped sign without flipping it back on the client which is ugly.

Also made `ScoreKind` `const`, not sure if any compiler optimizes on that but why not? If we ever move to C++20 this could be a generic lambda instead where we can template `ScoreKind` and make sure it gets optimized properly.

Tidbit: I treated score 0 as a normal score, I'm not sure what it implies. 

<!-- Note that builds and other checks will be run for your change. Don't feel intimidated by failures in some of the checks. If you can't resolve them yourself, experienced devs can also resolve them before merging your pull request. -->

## Checklist

- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Learath <learath2@gmail.com>
This commit is contained in:
bors[bot] 2023-05-29 11:02:52 +00:00 committed by GitHub
commit 6c7f6a4494
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -575,7 +575,7 @@ void CServerBrowser::SetInfo(CServerEntry *pEntry, const CServerInfo &Info)
class CPlayerScoreNameLess
{
int ScoreKind;
const int ScoreKind;
public:
CPlayerScoreNameLess(int ClientScoreKind) :
@ -585,6 +585,7 @@ void CServerBrowser::SetInfo(CServerEntry *pEntry, const CServerInfo &Info)
bool operator()(const CServerInfo::CClient &p0, const CServerInfo::CClient &p1)
{
// Sort players before non players
if(p0.m_Player && !p1.m_Player)
return true;
if(!p0.m_Player && p1.m_Player)
@ -593,33 +594,22 @@ void CServerBrowser::SetInfo(CServerEntry *pEntry, const CServerInfo &Info)
int Score0 = p0.m_Score;
int Score1 = p1.m_Score;
if(ScoreKind == CServerInfo::CLIENT_SCORE_KIND_TIME)
if(ScoreKind == CServerInfo::CLIENT_SCORE_KIND_TIME || ScoreKind == CServerInfo::CLIENT_SCORE_KIND_TIME_BACKCOMPAT)
{
// time is sent as a positive value to the http master, counting 0, if there is a time (finished)
// only positive times are meant to represent an actual time.
if(Score0 != Score1)
{
if(Score0 < 0)
// Sort unfinished (-9999) and still connecting players (-1) after others
if(Score0 < 0 && Score1 >= 0)
return false;
if(Score1 < 0)
if(Score0 >= 0 && Score1 < 0)
return true;
return Score0 < Score1;
}
}
else
{
if(ScoreKind == CServerInfo::CLIENT_SCORE_KIND_TIME_BACKCOMPAT)
{
if(Score0 == -9999)
Score0 = INT_MIN;
if(Score1 == -9999)
Score1 = INT_MIN;
}
if(Score0 > Score1)
return true;
if(Score0 < Score1)
return false;
if(Score0 != Score1)
{
// Handle the sign change introduced with CLIENT_SCORE_KIND_TIME
if(ScoreKind == CServerInfo::CLIENT_SCORE_KIND_TIME)
return Score0 < Score1;
else
return Score0 > Score1;
}
return str_comp_nocase(p0.m_aName, p1.m_aName) < 0;