Skip to content

Added achievement for playing multiple games with different players#81

Open
PhilipTzannis wants to merge 4 commits intobeeracademy:masterfrom
PhilipTzannis:moreTheMerrier
Open

Added achievement for playing multiple games with different players#81
PhilipTzannis wants to merge 4 commits intobeeracademy:masterfrom
PhilipTzannis:moreTheMerrier

Conversation

@PhilipTzannis
Copy link
Copy Markdown
Contributor

No description provided.

@PhilipTzannis
Copy link
Copy Markdown
Contributor Author

Forslag til nyt achievement, for at have spillet flere spil med flere forskellige mennesker

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something is wrong with the icon

Comment thread games/achievements.py
top20 = sorted(
({"x": k, "y": v} for k, v in played_with_count.items()),
key=lambda x: -x["y"],
)[:30]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 30?

Comment thread games/achievements.py
if player != user:
played_with_count[player.username] += 1

top20 = sorted(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use played_with_count.most_common()

Comment thread games/achievements.py
({"x": k, "y": v} for k, v in played_with_count.items()),
key=lambda x: -x["y"],
)[:30]
if len(top20) < 20:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is wrong. If a player has only ever played with the same 5 players, they should be able to get the base level.

Copy link
Copy Markdown
Member

@tyilo tyilo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants