Send ratings of search parties in queue #1020
Conversation
| @@ -288,8 +290,7 @@ def to_dict(self): | |||
| ndigits=2 | |||
| ), | |||
| "num_players": self.num_players, | |||
| "boundary_80s": [search.boundary_80 for search in self._queue.keys()], | |||
| "boundary_75s": [search.boundary_75 for search in self._queue.keys()], | |||
| "boundaries": boundaries, | |||
There was a problem hiding this comment.
We can't just remove the old keys as that would be a breaking change in the protocol. The best thing to do would be to leave them, add a # DEPRECATED: comment indicating that they will be removed eventually, and add the new boundaries key in addition to the existing ones.
Somehow I thought they were already marked as deprecated though.
There was a problem hiding this comment.
@Askaholic @BlackYps What do you think about sending a list (set) of means instead of boundaries? Then the client can decide for itself what range it wants to use
There was a problem hiding this comment.
That sounds like a good idea
There was a problem hiding this comment.
Yea that sounds better to me too. I guess another thing to consider is should we be sending the party information in some way or just sending a flat list of means?
Or another idea would be to send something similar to a histogram to prevent the message from scaling with the number of players in queue. Something like this where the means are just rounded to the nearest 100:
{
"summary_means": {
400: 1,
600: 2,
1000: 4,
1400: 1,
}
}There was a problem hiding this comment.
You're right we should probably send the party mean instead of each players rating as the partys are matched by their average resting only. If a 1k and a 2k form a party they need other 1500s in the queue to match.
We don't need the number of players in a chunk, only if a chunk is populated at all. We could do it like this but 100 is probably a bit too coarse. How big of a deal is it anyway when the message size varies?
There was a problem hiding this comment.
Does not removing the old keys to stay protocol compatible also imply that they should still send the same info?
Breaking changes were planned for v2 of the server, right?
There was a problem hiding this comment.
Message size varying is fine, I want to make sure however that we think about the different ways that we could be encoding the information that the client needs and sending it in the most byte efficient way. More bytes means more CPU load on the server and more potential to run into bandwidth issues. If we can pass the same information in a way that avoids the possibility of scaling infinitely then I say we should go with that.
The protocol versioning is more about the structure of messages and not so much about the content. If certain messages start sending empty lists that used to be non-empty, that should be fine as the client should be able to handle that case anyway. Some feature of the client might stop working by showing empty results or just not appearing anymore, but it doesn't cause the client to break (crash) due to serialization errors or trying to perform operations on an incorrect type.
BlackYps
changed the title
BlackYps
changed the title
|
I'm not sure why I broke a seemingly unrelated test |
| @@ -288,9 +292,12 @@ def to_dict(self): | |||
| ndigits=2 | |||
| ), | |||
| "num_players": self.num_players, | |||
| "ratings": sorted(ratings), | |||
There was a problem hiding this comment.
Another thing that we've learned from experience over the years is that sending info in the form of a dict with descriptive key names is preferable over sending a list where you need to know what the data in the list means. This is because with dicts you can easily add more keys if you need to extend the message without breaking the existing functionality.
This might be fine in this case. I think we should rename ratings to something a bit more specific though as there is also a key in the player_info message called ratings which has a different structure from this. Maybe something like rating_buckets, rating_groups or something? Also would it be useful to send the granularity?
There was a problem hiding this comment.
maybe active_rating_groups? when I hear rating_buckets my first thought is that the list contains all eligible rating buckets and all buckets not listed are invalid, whatever that means.
I don't think it's useful to send the granularity. As long as the granularity is reasonably high it doesn't really matter for the client. And if we rounded to something unreasonably coarse like nearest 200 rating then the entire list would be useless anyway.
| @@ -278,6 +278,10 @@ def to_dict(self): | |||
| """ | |||
| Return a fuzzy representation of the searches currently in the queue | |||
| """ | |||
| if self.team_size == 1: | |||
| ratings = {round(search.ratings[0].mean / 25) * 25 for search in self._queue.keys()} | |||
There was a problem hiding this comment.
Would it make sense to add a config value for the granularity?
There was a problem hiding this comment.
I don't see a reason for this. We should choose a reasonable value once and then there is no real reason to change it on the fly. I will extract the number into a variable though to make it less of a magical number.
BlackYps
force-pushed
the
search-boundaries
branch
from
e592d87
to
c0d1ac0
Compare
|
I just realized something: The 1v1 rating gets adjusted for new players. So the client needs to know how this adjustment works, so it can calculate the adjusted rating of the logged in player to determine if that player is actually close to the active rating ranges. |
|
I agree, we should be sending whatever number the matchmaker is actually using if possible. |
|
I opened a new issue to discuss this. #1024 |
Sends the ratings of search parties in a queue. This is a step to bring back the indicator that the python client had, but this time extended for all matchmaker queues.
We send just the rating, so the client can decide on its own how big of a range it wants to allow to display the indicator as a successful match also depends on the deviation of the player seeing the notification