Skip to content

Conversation

@FFFFFFFXXXXXXX
Copy link
Contributor

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?

@FFFFFFFXXXXXXX FFFFFFFXXXXXXX mentioned this pull request Jul 11, 2025
@Leastrio
Copy link
Owner

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.

@FFFFFFFXXXXXXX
Copy link
Contributor Author

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 IngameClient::active_game().

────────────────────────────────────────────────┐
• 18: pub struct IngameClient(reqwest::Client); │
────────────────────────────────────────────────┘
  181819impl IngameClient {                                      19impl IngameClient {
  21pub fn new() -> Result<Self, IngameClientError> {    21pub fn new() -> Self {
  22Ok(Self(build_reqwest_client(None)))             22Self(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 Player fields an Option<> even though they should always be there - or we filter out the fake "players".

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)
}

@FFFFFFFXXXXXXX
Copy link
Contributor Author

see #16 for the new patchset

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants