Open atomflunder opened 5 months ago
I want to give this a go, tomorrow perhaps? Or the day after tomorrow.
I want to give this a go, tomorrow perhaps? Or the day after
Feel free to do that, as I said I don't think it will be that difficult to implement from a technical standpoint.
But how to pass the weights into the function in a clean way is something that isn't straightforward. The function parameters are already a lot (Maybe type aliases for teams would help?). I've been thinking about it yesterday and I am not sure what's the most user-friendly approach. Other libraries just have the weights as a separate argument, like this:
// ts-trueskill library
export function rate(
ratingGroups: Rating[][] | any[],
ranks?: any[] | null,
weights?: any[] | null,
minDelta = 0.0001,
env: TrueSkill = new TrueSkill(),
): Rating[][] {
// ...
}
Which I do think is a bit cleaner than the proposed stuff from me in the original comment, but then you have to validate those separately.
It also has the advantage of not breaking the MultiTeamRatingSystem Trait, since the Weng-Lin algorithm does not support weights. I know that the OpenSkill library supports weights, but it's more of a bonus and not something supported in the original proposal of the algorithm.
But it seems like a hassle for the user to do add the weights regardless, especially since most users would probably leave all of the weights at 1.0 at all times. Maybe a helper function would be neat?
fn get_default_weights(teams: &[&[TrueSkillRating]]) -> Vec<Vec<f64>> {
// Just returns a Vec of Vec of 1.0 for every player
}
Generally speaking, I am quite unhappy about the API choices in this library that I made in the past. On my other libraries that no one uses I would not mind dropping massive breaking changes, but this is the first project on mine that is kind of in use, so I'm very careful with that.
Sorry if this is a long ramble, if you have an idea of how to go about this please let me know @asyncth .
By the way, I really appreciate your contributions to the library 🙂
Perhaps weights could be added into TrueSkillRating
? I really don't know, if I was designing this API from the scratch I'd probably make some kind of builder struct which has a method to add a team and a method to calculate the ratings. The teams themselves would also be constructed using some other builder struct. I don't think that's a good design though, I just don't know how to do it better.
Perhaps weights could be added into TrueSkillRating?
Nah, weights aren't tied to your rating, but the match itself.
I really don't know, if I was designing this API from the scratch I'd probably make some kind of builder struct which has a method to add a team and a method to calculate the ratings. The teams themselves would also be constructed using some other builder struct. I don't think that's a good design though, I just don't know how to do it better.
I don't know either. I originally wanted to keep it simple, but I guess TrueSkill just is not simple 😅
Maybe we could create 2 functions, one with weights and one without, and the one without just calls the weighted function with default weights?
But then again the API already has so many functions, I'm not sure if I wanna add another. Just kind of thinking out loud here.
I think generally a good "roadmap" would be to add this feature, and any other stuff that is missing to call this crate feature-complete and then only introduce a major API change with an eventual 1.0 release, if at all.
TrueSkill has the capability for partial play weights, accounting for players who have not played the whole match. These can range from
0.0
(no participation) to1.0
(full match).This crate should probably implement these, although I am undecided on how to best do it to be as user-friendly as possible. Passing a partial play weight in every time could be pretty annoying to the user, since in 99.9% of cases, it will be set to
1.0
.Rust doesn't use default values for arguments unfortunately, I think those would come in useful here.
Maybe it's useful to offer 2 different functions, one with and one without weights? Does seem a bit "bloaty" to me though.
Also, I think this only makes sense to implement for the full
trueskill_multi_team
andmatch_quality_multi_team
functions, and not the 1-vs-1 and Team-vs-Team shortcuts.For the match quality, the Matrix implementation also has to be updated in two places in the
create_rotated_a_matrix
function.Not sure about the
expected_score_multi_team
function at the moment.I think the technical implementation will be fairly straight-forward, but the design choices will be more difficult here. I also wouldn't call this issue urgent, but it would be nice to have eventually, before an eventual 1.0 release.