Add leaderboard -> targets Issue Leaderboard command #5

Gregory-Nitch commented 6 months ago

Targets Issue Leaderboard command #5

Unfortunately, GitHub is showing 1k plus additions for this pull request once I switched to the dev branch from the main branch, this is my first pull request ever; forgive me if I've done something silly.

Below you can find the code that is added and where.

Implements

leaderboard() function in game_commands.py create_leaderboard() method in core.py

The default values for the leaderboard command are 'debt' and 5 therefore sending a bot message with the top 5 users based on debt, but users can input cranes and any other length.

Achieved functionality:

>.leaderboard
1. user1 - 10,000 debt - 0 cranes
2. user2 - 8,989 debt - 2 cranes

>.leaderboard cranes
1. user2 - 8,989 debt - 2 cranes
2. user1 - 10,000 debt - 0 cranes

>.leaderboard debt 1
1. user1 - 10,000 debt - 0 cranes

Though with this solution it requires the user to enter a sorting type if they wish to enter a leaderboard length requirement.

Code

leaderboard() function in game_commands.py -> Lines 71-83

    @commands.command(name="leaderboard")
    async def leaderboard(self, ctx, sort_by: str = "debt", lb_len: int = 5):
        if not self.game:
            await ctx.send("Game not loaded. (Yell at Toast more!)")
            return
        try:
            lb = self.game.create_leaderboard(sort_by, lb_len)
            if lb == "":
                await ctx.send("No registered users, a leaderboard could not be made!")
                return
            await ctx.send(lb)
        except ValueError as err:
            await ctx.send(f"Error: {str(err)}")

create_leaderboard() method in core.py -> Lines 59-79

    def create_leaderboard(self, sort_by: str, lb_len: int):
        if sort_by not in ["debt", "cranes"]:
            raise ValueError("ordering is done by 'debt' or 'cranes'")
        if lb_len < 1:
            raise ValueError("requested leaderboard length must be 1 or greater")
        # Make a sort key and sort users
        if sort_by == "debt":
            key = lambda u: u[1].word_debt
        elif sort_by == "cranes":
            key = lambda u: u[1].cranes
        users = sorted(self._state.items(), key=key, reverse=True)

        # Produce leaderboard string to return
        lb = ""
        i = 1
        for u in users:
            lb += f"{i}. {u[1].display_name} - {u[1].word_debt:,} debt - {u[1].cranes:,} cranes\n"
            i += 1
            if i > lb_len:
                break
        return lb

Hope this helps!

Gregory-Nitch commented 6 months ago

Might be a little forward but I took some time to make some unit tests for the leaderboard functionality, should it be merged and should it need unit tests. I've set the pull request as a draft, if you see any required changes I would love to make them. Have a good weekend!

-Greg

test_game.py -> Lines 78-98

Check proper rejection of a requested leaderboard length < 1:

def test_game_rejects_invalid_leaderboard_length(game_state: game.WordDebtGame):
with pytest.raises(ValueError):
    game_state.create_leaderboard("debt", 0)

Check proper rejection of an invalid sorting key, ie != "debt" or "cranes":

def test_game_rejects_invalid_leaderboard_sorting(game_state: game.WordDebtGame):
with pytest.raises(ValueError):
    game_state.create_leaderboard("INVALID", 1)

Check proper leaderboard layout given 1 player and test values:

def test_game_produces_valid_leaderboard(
game_state: game.WordDebtGame, player: game.WordDebtPlayer
):
player.display_name = "test_name"
player.word_debt = 10000
player.cranes = 0
game_state.register_player(player)
assert (
    game_state.create_leaderboard("debt", 1)
    == f"1. {player.display_name} - {player.word_debt:,} debt - {player.cranes:,} cranes\n"
)

test_game_commands_cog.py -> Lines 133-149

Check command properly informs users if no one has registered and a leaderboard cannot be made:

@pytest.mark.asyncio
async def test_request_leaderboard_no_register(game_commands_cog: cogs.GameCommands):
ctx = AsyncMock()

await game_commands_cog.leaderboard(game_commands_cog, ctx)

ctx.send.assert_called_with(String() & Regex("No registered users.*"))

Check command properly informs users when the game has not been properly loaded.

@pytest.mark.asyncio
async def test_request_leaderboard_no_game(game_commands_cog: cogs.GameCommands):
ctx = AsyncMock()
game_commands_cog.game = None

await game_commands_cog.leaderboard(game_commands_cog, ctx)

ctx.send.assert_called_with(String() & Regex("Game not loaded.*"))

Swiddis commented 6 months ago

It looks like these commits were put on the main branch making a large diff / merge conflicts when creating a PR for main -- Can you rebase the new commits to dev to get an accurate diff and resolve the merge conflicts?

Gregory-Nitch commented 6 months ago

Sorry for the headache, obviously I have more learning on git to do. My attempt to rebase to dev is still yielding 22 file changes. I'll see what I can do.

Gregory-Nitch commented 6 months ago

Created new branch & pull request to avoid the above mess:

Swiddis / word-debt-bot