Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flag Best Country ties better #283

Open
UEWBot opened this issue Feb 9, 2024 · 9 comments
Open

Flag Best Country ties better #283

UEWBot opened this issue Feb 9, 2024 · 9 comments

Comments

@UEWBot
Copy link
Owner

UEWBot commented Feb 9, 2024

Where two players have the same score as the same power, need to make that clearer on the "Best Countries" page. May want to support automatic tie-breaks. We do now have the ability for the TD to pick who gets the actual award, so that should be reflected in the view.

@UEWBot
Copy link
Owner Author

UEWBot commented Feb 10, 2024

Tournament.best_countries(whole_list=False) does correctly handle ties.
Tournament.update_scores() currently gives best country awards to every player of each power with the top score (so it allows ties, and shared awards).
The Best Countries view definitely needs tweaking.
It would be good if the awards page said "tied" or "shared" or something, because it's keyed by player rather than by award.
Similarly, it's not obvious when there are multiple recipients in the awards form, but maybe that's ok if we clearly show them as shared on the awards view.

@UEWBot
Copy link
Owner Author

UEWBot commented Feb 10, 2024

The awards view doesn't have its own code at the moment - it uses tournament_views.tournament_simple(). I don't see an easy way to detect a shared award from within the template.

@UEWBot
Copy link
Owner Author

UEWBot commented Feb 19, 2024

I wonder if we should be storing the player's rank in their RoundPlayer and TournamentPlayer (and potentially GamePlayer, too). It only changes when Game scores change, and having it there makes it accessible to the template code. We could also add a rank_tied attribute...

@UEWBot
Copy link
Owner Author

UEWBot commented Feb 19, 2024

In which case we could also add a best_power_rank to GamePlayer (and potentially best_power_rank_tied).

@UEWBot
Copy link
Owner Author

UEWBot commented Feb 19, 2024

Of course none of that helps with awards. I think the only way to deal with that is to have an actual class linking the Award to the TournamentPlayer, which can then have an is_shared attribute or method.

@UEWBot
Copy link
Owner Author

UEWBot commented Feb 19, 2024

This now has a bunch of potential improvements, so let's try to capture them all:

  1. The best_countries view needs to better flag ties. Tournament.best_countries(whole_list=False) does handle ties correctly.
  2. We need a new class to associate an Award with a TournamentPlayer so we can add an is_shared attribute or method. Then we can use that in the awards view to better flag shared awards.
  3. Do we need to do anything in the best_countries view after the TD has resolved a tie? Probably not if we do both the above.
  4. It may be useful to add GamePlayer.best_power_rank and TournamentPlayer.rank (and possibly a flag for shared rank) rather than calculating them every time they are needed.

@UEWBot
Copy link
Owner Author

UEWBot commented Feb 19, 2024

tournament_best_countries() calls Tournament.best_countries(whole_list=True), which doesn't flag ties. For each power, it returns an ordered list of GamePlayers, but some of those may be ties. It can identify ties by looking at the scores.

UEWBot added a commit that referenced this issue Feb 19, 2024
Change the list returned by Tournament.best_countries(hole_list=True)
to be a list of lists, with each sub-list containing the GamePlayers
at a given rank, thus clearly identifying ties.
Change the corresponding view code and template to process the list
accordingly.

Issue #283
@UEWBot UEWBot self-assigned this Feb 19, 2024
@UEWBot
Copy link
Owner Author

UEWBot commented Feb 23, 2024

The commit fixes (1).
(2) needs doing, but is tricky, particularly as we have existing data using the current schema.
I think (3) is fine as-is.
(4) should be a separate Issue.

@UEWBot UEWBot removed their assignment Feb 23, 2024
@UEWBot
Copy link
Owner Author

UEWBot commented Feb 23, 2024

Created issue #288 to cover (4)

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

No branches or pull requests

1 participant