From 200576b00f876ecf6687667e006afbec98290379 Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Wed, 30 Aug 2023 23:28:57 +0100 Subject: [PATCH 1/3] Refactor experience and level-up systems Updated the level-up system to appropriately fire events for each level gained and not just the final level. Removed the direct update of the user's level within the levelUp method and instead implemented a function to calculate and update to the highest possible level based on a user's current points. This ensures that levelling up and firing of events occur for each level achieved, rather than skipping to the final level. Additionally, methods within the GiveExperience.php file were rearranged for better readability and maintenance. --- src/Concerns/GiveExperience.php | 44 ++++++++++--------- src/Listeners/PointsIncreasedListener.php | 20 ++++++--- tests/Concerns/GiveExperienceTest.php | 2 +- .../Listeners/PointsIncreasedListenerTest.php | 13 +++++- .../Listeners/UserLevelledUpListenerTest.php | 18 +++++++- 5 files changed, 68 insertions(+), 29 deletions(-) diff --git a/src/Concerns/GiveExperience.php b/src/Concerns/GiveExperience.php index 84e31d9..f605c71 100644 --- a/src/Concerns/GiveExperience.php +++ b/src/Concerns/GiveExperience.php @@ -69,14 +69,15 @@ public function addPoints( 'level_id' => $experience->level_id, ])->save(); - if ($level?->level > config(key: 'level-up.starting_level')) { - Event::dispatch(event: new UserLevelledUp(user: $this, level: $level->level)); + for ($lvl = config(key: 'level-up.starting_level'); $lvl <= $level?->level; $lvl++) { + Event::dispatch(event: new UserLevelledUp(user: $this, level: $lvl)); } $this->dispatchEvent($amount, $type, $reason); return $this->experience; } + /** * If the User does have an Experience record, update it. */ @@ -105,16 +106,6 @@ public function experience(): HasOne return $this->hasOne(related: Experience::class); } - public function experienceHistory(): HasMany - { - return $this->hasMany(related: ExperienceAudit::class); - } - - public function getPoints(): int - { - return $this->experience->experience_points; - } - protected function dispatchEvent(int $amount, string $type, ?string $reason): void { event(new PointsIncreased( @@ -138,6 +129,11 @@ public function getLevel(): int return $this->experience->status->level; } + public function experienceHistory(): HasMany + { + return $this->hasMany(related: ExperienceAudit::class); + } + public function deductPoints(int $amount): Experience { $this->experience->decrement(column: 'experience_points', amount: $amount); @@ -191,22 +187,27 @@ public function nextLevelAt(int $checkAgainst = null, bool $showAsPercentage = f return max(0, ($nextLevel->next_level_experience - $currentLevelExperience) - ($this->getPoints() - $currentLevelExperience)); } - public function levelUp(): void + public function getPoints(): int + { + return $this->experience->experience_points; + } + + public function levelUp(int $to): void { if (config(key: 'level-up.level_cap.enabled') && $this->getLevel() >= config(key: 'level-up.level_cap.level')) { return; } - $nextLevel = Level::firstWhere(column: 'level', operator: $this->getLevel() + 1); - - $this->experience->status()->associate(model: $nextLevel); - $this->experience->save(); + $this->fill(attributes: ['level_id' => $to]) + ->save(); - $this->update(attributes: [ - 'level_id' => $nextLevel->level, - ]); + $this->experience->status()->associate(model: $to); + $this->save(); - event(new UserLevelledUp(user: $this, level: $this->getLevel())); + // Fire an event for each level gained + for ($lvl = $this->getLevel(); $lvl <= $to; $lvl++) { + event(new UserLevelledUp(user: $this, level: $lvl)); + } } public function level(): BelongsTo @@ -214,3 +215,4 @@ public function level(): BelongsTo return $this->belongsTo(related: Level::class); } } + diff --git a/src/Listeners/PointsIncreasedListener.php b/src/Listeners/PointsIncreasedListener.php index bedf3d9..1d7a43f 100644 --- a/src/Listeners/PointsIncreasedListener.php +++ b/src/Listeners/PointsIncreasedListener.php @@ -24,16 +24,26 @@ public function __invoke(PointsIncreased $event): void ]); } - $nextLevel = Level::firstWhere(column: 'level', operator: $event->user->getLevel() + 1); + // Get the next level experience needed for the user's current level + $nextLevel = Level::firstWhere(column: 'level', operator: '=', value: $event->user->getLevel() + 1); if (! $nextLevel) { + // If there is no next level, return return; } - if ($event->user->getPoints() < $nextLevel->next_level_experience) { - return; + // Check if user's points are equal or greater than the next level's required experience + if ($event->user->getPoints() >= $nextLevel->next_level_experience) { + // Find the highest level the user can achieve with current points + $highestAchievableLevel = Level::query() + ->where(column: 'next_level_experience', operator: '<=', value: $event->user->getPoints()) + ->orderByDesc(column: 'level') + ->first(); + + // Update the user level to the highest achievable level + if ($highestAchievableLevel->level > $event->user->getLevel()) { + $event->user->levelUp(to: $highestAchievableLevel->level); + } } - - $event->user->levelUp(); } } diff --git a/tests/Concerns/GiveExperienceTest.php b/tests/Concerns/GiveExperienceTest.php index 45d5704..9b18b78 100644 --- a/tests/Concerns/GiveExperienceTest.php +++ b/tests/Concerns/GiveExperienceTest.php @@ -257,7 +257,7 @@ expect($this->user->experience) ->experience_points->toBe(expected: 400) - ->and($this->user)->getLevel()->toBe(expected: 3); + ->and($this->user)->getLevel()->toBe(expected: 4); }); test('A multiplier can use data that was passed through to it', function () { diff --git a/tests/Listeners/PointsIncreasedListenerTest.php b/tests/Listeners/PointsIncreasedListenerTest.php index 1307f9f..af0dfa7 100644 --- a/tests/Listeners/PointsIncreasedListenerTest.php +++ b/tests/Listeners/PointsIncreasedListenerTest.php @@ -58,7 +58,7 @@ $this->user->addPoints(amount: 100); expect($this->user) - ->experienceHistory()->count()->toBe(expected: 2); + ->experienceHistory()->count()->toBe(expected: 3); $this->assertDatabaseHas(table: 'experience_audits', data: [ 'user_id' => $this->user->id, @@ -110,3 +110,14 @@ 'reason' => 'test', ]); }); + +test(description: 'a User can level up multiple times', closure: function () { + /** + * Example: a new User will start with no experience, can be given + * 300 points, and will level up to level 3 + */ + $this->user->addPoints(amount: 300); + $this->user->addPoints(amount: 300); + + expect($this->user)->getLevel()->toBe(expected: 5); +}); diff --git a/tests/Listeners/UserLevelledUpListenerTest.php b/tests/Listeners/UserLevelledUpListenerTest.php index 2b60413..cce654a 100644 --- a/tests/Listeners/UserLevelledUpListenerTest.php +++ b/tests/Listeners/UserLevelledUpListenerTest.php @@ -12,7 +12,7 @@ it(description: 'adds audit data when a User level\'s up', closure: function () { $this->user->addPoints(100); - expect($this->user)->experienceHistory->count()->toBe(expected: 2); + expect($this->user)->experienceHistory->count()->toBe(expected: 3); $this->assertDatabaseHas(table: 'experience_audits', data: [ 'user_id' => $this->user->id, @@ -31,3 +31,19 @@ Event::assertDispatched(event: UserLevelledUp::class); Event::assertListening(expectedEvent: UserLevelledUp::class, expectedListener: UserLevelledUpListener::class); }); + +test(description: 'when a User levels up more than once, an event runs for each level', closure: function (int $level) { + Event::fake(); + + $this->user->addPoints(300); + + expect($this->user)->experience->level_id->toBe(expected: 3) + ->and($this->user)->level_id->toBe(expected: 3); + + Event::assertDispatchedTimes(event: UserLevelledUp::class, times: 3); + + Event::assertDispatched( + event: UserLevelledUp::class, + callback: fn (UserLevelledUp $event): bool => $event->level === $level + ); +})->with([1, 2, 3]); From 146064327a70a064e63181ea0b26ff105abfb6c1 Mon Sep 17 00:00:00 2001 From: cjmellor Date: Wed, 30 Aug 2023 22:33:24 +0000 Subject: [PATCH 2/3] fix: Files linted with Pint --- src/Concerns/GiveExperience.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Concerns/GiveExperience.php b/src/Concerns/GiveExperience.php index f605c71..a2afcb2 100644 --- a/src/Concerns/GiveExperience.php +++ b/src/Concerns/GiveExperience.php @@ -215,4 +215,3 @@ public function level(): BelongsTo return $this->belongsTo(related: Level::class); } } - From 7b3c4ec942903fa06670603ef6b5d42d536bbb1f Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Thu, 31 Aug 2023 20:28:15 +0100 Subject: [PATCH 3/3] Reduce query --- src/Concerns/GiveExperience.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Concerns/GiveExperience.php b/src/Concerns/GiveExperience.php index a2afcb2..0a8e178 100644 --- a/src/Concerns/GiveExperience.php +++ b/src/Concerns/GiveExperience.php @@ -33,7 +33,7 @@ public function addPoints( $lastLevel = Level::orderByDesc(column: 'level')->first(); throw_if( - condition: isset($lastLevel->next_level_experience) && $amount > Level::orderByDesc(column: 'level')->first()->next_level_experience, + condition: isset($lastLevel->next_level_experience) && $amount > $lastLevel->next_level_experience, message: 'Points exceed the last level\'s experience points.', );