Merge pull request ppy#11361 from bdach/fix-user-score-aggregate-acco…

…unting-when-equal Fix considering playlist score as user high if total is equal to previous high
nanaya · Jul 25, 2024 · ddf92c8 · ddf92c8
2 parents 44ef777 + bc6ef9b
commit ddf92c8
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 1 deletion.
diff --git a/app/Models/Multiplayer/PlaylistItemUserHighScore.php b/app/Models/Multiplayer/PlaylistItemUserHighScore.php
@@ -114,7 +114,7 @@ public function updateWithScoreLink(ScoreLink $scoreLink): bool
     {
         $score = $scoreLink->score;
 
-        if ($score === null || !$score->passed || $score->total_score < $this->total_score) {
+        if ($score === null || !$score->passed || $score->total_score <= $this->total_score) {
             return false;
         }
 

diff --git a/tests/Models/Multiplayer/UserScoreAggregateTest.php b/tests/Models/Multiplayer/UserScoreAggregateTest.php
@@ -83,6 +83,58 @@ public function testAddingLowerScore(): void
         $this->assertSame($scoreLink->getKey(), $agg->last_score_id);
     }
 
+    public function testAddingEqualScore(): void
+    {
+        $firstUser = User::factory()->create();
+        $secondUser = User::factory()->create();
+        $playlistItem = $this->createPlaylistItem();
+
+        // first user sets play
+        $firstUserPlay = $this->addPlay($firstUser, $playlistItem, [
+            'accuracy' => 0.5,
+            'passed' => true,
+            'total_score' => 10,
+        ]);
+
+        $firstUserAgg = UserScoreAggregate::new($firstUser, $this->room);
+        $this->assertSame(1, $firstUserAgg->completed);
+        $this->assertSame(0.5, $firstUserAgg->accuracy);
+        $this->assertSame(10, $firstUserAgg->total_score);
+        $this->assertSame(1, $firstUserAgg->userRank());
+        $this->assertSame($firstUserPlay->getKey(), $firstUserAgg->last_score_id);
+
+        // second user sets play with same total, so they get second place due to being late
+        $secondUserPlay = $this->addPlay($secondUser, $playlistItem, [
+            'accuracy' => 0.5,
+            'passed' => true,
+            'total_score' => 10,
+        ]);
+
+        $secondUserAgg = UserScoreAggregate::new($secondUser, $this->room);
+        $this->assertSame(1, $secondUserAgg->completed);
+        $this->assertSame(0.5, $secondUserAgg->accuracy);
+        $this->assertSame(10, $secondUserAgg->total_score);
+        $this->assertSame(2, $secondUserAgg->userRank());
+        $this->assertSame($secondUserPlay->getKey(), $secondUserAgg->last_score_id);
+
+        // first user sets play with same total again, but their rank should not move now
+        $this->addPlay($firstUser, $playlistItem, [
+            'accuracy' => 0.5,
+            'passed' => true,
+            'total_score' => 10,
+        ]);
+
+        $firstUserAgg->refresh();
+        $this->assertSame(1, $firstUserAgg->completed);
+        $this->assertSame(0.5, $firstUserAgg->accuracy);
+        $this->assertSame(10, $firstUserAgg->total_score);
+        $this->assertSame(1, $firstUserAgg->userRank());
+        $this->assertSame($firstUserPlay->getKey(), $firstUserAgg->last_score_id);
+
+        $secondUserAgg->refresh();
+        $this->assertSame(2, $secondUserAgg->userRank());
+    }
+
     public function testAddingMultiplePlaylistItems(): void
     {
         $user = User::factory()->create();