-
-
Notifications
You must be signed in to change notification settings - Fork 71
Feature/#1032 working veto system #1071
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
Changes from 84 commits
b7db5ec
adcc746
5933022
5a3e05b
a2c42a5
3c77c0b
4ac5d49
5bfea4f
998f03c
dc8dc57
ec6a9f6
6f137fd
c050a3c
515125d
1b6a3b6
8a822f7
a33db54
2d22bb7
7dc151f
638c661
1e45324
0de6ae8
dc934d9
af45b0e
cb79089
a76df29
8d7fa14
aba93fa
5e6ebf0
a293b40
416b483
ca425c8
a1d5da2
80933b8
81251be
16a15d2
9018c65
9ef1693
d8aa760
70ff78c
ca61e9d
3872919
8c1cbfe
3e32332
0b42bf1
27b56fc
285851e
e90dc52
bf8db6c
282cf14
620fd12
be19c44
522e5b1
2b91d6b
1c16fd2
0a4ad9d
90292ed
129648f
3058a31
2644646
0c374d2
d002367
bee6f13
73f6ced
fed4882
23bd8ec
3e84ed1
e5c8362
7ef8a9e
a766aa0
23c641a
6ac051a
729e0f4
4de97a3
6b70550
158e4a6
14c2c2b
5401dad
b43a0ce
3decd56
4ee987d
b9c6a85
49eb2f8
3795aff
80d2a46
1f0edc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -295,10 +295,14 @@ | |
|
|
||
| matchmaker_queue_map_pool = Table( | ||
| "matchmaker_queue_map_pool", metadata, | ||
| Column("matchmaker_queue_id", Integer, ForeignKey("matchmaker_queue.id"), nullable=False), | ||
| Column("map_pool_id", Integer, ForeignKey("map_pool.id"), nullable=False), | ||
| Column("min_rating", Integer), | ||
| Column("max_rating", Integer), | ||
| Column("id", Integer, primary_key=True), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding my earlier comment. I just saw that this was not introduced by your PR but instead already existed in the code base. I pasted your version of this definition into claude sonnet and prompted to generate a comment for the review, based on my earlier comment. This is its output:
Code Suggestion: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this already existed before your PR I'd say let's not rename this, but definitely do add the source comment. First check whether it makes sense though. The LLMs are good at generating stuff that sounds convincing but might be subtly or even fundamentally wrong.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://faforever.github.io/db/tables/matchmaker_queue_map_pool.html this is just database entity There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the other database entities have descriptive names. This name is not helpful. Instead of renaming it, I'd suggest simply adding a source comment what this is for. |
||
| Column("matchmaker_queue_id", Integer, ForeignKey("matchmaker_queue.id"), nullable=False), | ||
| Column("map_pool_id", Integer, ForeignKey("map_pool.id"), nullable=False), | ||
| Column("min_rating", Integer), | ||
| Column("max_rating", Integer), | ||
| Column("veto_tokens_per_player", Integer, nullable=False), | ||
| Column("max_tokens_per_map", Integer, nullable=False), | ||
| Column("minimum_maps_after_veto", Float, nullable=False), | ||
| ) | ||
|
|
||
| teamkills = Table( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You said there were issues with this being
2. Add a unit test which: enforces this being 1 until the underlying issue is fixed. Add a short description to the unit test of what the issue was.