-
Notifications
You must be signed in to change notification settings - Fork 8
RiotId + Arena fixes #15
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
Conversation
…tead of waiting for one
…r String if that's not possible
e.g. to avoid 'temporary value dropped while borrowed' and similar inconveniences
|
Hi! Thanks for the pr, I havent had much time to go over it but from skimming it ive noticed one case where you just have it panic when league client isnt running. Id prefer to leave it to the old behavior where the user handles the unwrapping so that they can handle whether the league client is running or not. I think for now if possible just do the riot id changes. And may I ask what the special case for arena are? If I had to be honest I quit league months ago so ive forgotten a bit about it. |
|
The Result<> was just wrapping a function that never fails so removing it should be fine. I think the doc-comment was wrong. The function always just created a reqwest client and didnt check if there is an ingame-client running. The method for checking if an ingame client api is running should be ────────────────────────────────────────────────┐
• 18: pub struct IngameClient(reqwest::Client); │
────────────────────────────────────────────────┘
18┊ 18│
19┊impl IngameClient { 19│impl IngameClient {
21┊ pub fn new() -> Result<Self, IngameClientError> { 21│ pub fn new() -> Self {
22┊ Ok(Self(build_reqwest_client(None))) 22│ Self(build_reqwest_client(None))
23┊ } 23│ }As for the Arena special cases there are "Pyke", "Sett", "Thresh" and some others running around the map as obstacles to play around and they show up as players when you request the list of players from the ingame api. The problem is they have some fields missing. So either we have to make a bunch of I chose to try and deserialize each player and filter out all that fail to deserialize correctly. #[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct AllGameData {
/// only available in live game - None in spectator mode
#[serde(deserialize_with = "treat_error_as_none")]
pub active_player: Option<ActivePlayer>,
#[serde(deserialize_with = "deserialize_players")]
pub all_players: Vec<Player>,
#[serde(deserialize_with = "deserialize_events")]
pub events: Vec<GameEvent>,
pub game_data: GameStats,
}
/// in Arena mode the special events (random Thresh lanterns, Pyke jumping across the map, Jhin shooting bullets)
/// are coded as normal champions, which show up in the API response as champions but with an error message
/// instead of proper stats
///
/// as a workaround deserialize players to `PlayerOpt` and only return Players that were successfully deserialized
pub(crate) fn deserialize_players<'de, D>(deserializer: D) -> Result<Vec<Player>, D::Error>
where
D: Deserializer<'de>,
{
#[derive(Deserialize)]
struct PlayerOpt(#[serde(deserialize_with = "treat_error_as_none")] Option<Player>);
let players = Vec::<PlayerOpt>::deserialize(deserializer)?
.into_iter()
.filter_map(|player| player.0)
.collect::<Vec<_>>();
Ok(players)
} |
|
see #16 for the new patchset |
I have some changes for the RiotId changes and some special cases for Arena. But mixed in there are some changes I did to adjust stuff for one of my projects. How much of this do you want me to upstream? What do you want me to leave out?