meraki-analytics / orianna

A Java framework for the Riot Games League of Legends API (http://developer.riotgames.com/).
MIT License
182 stars 56 forks source link

Add Riot Account support (using Riot Account-V1) #142

Closed Excaliburns closed 11 months ago

Excaliburns commented 1 year ago

Hi there!

I wanted to use Orianna for a small personal project but while I was using it, I noticed there was no way to query Account-V1. I checked the discord for any information on the topic but couldn't find any, and it was pretty quiet.

Riot will be moving away from Summoner names on Nov 20, according to their API docs. Looks like they will support the Summoner-V4 endpoints for awhile after, but they're deprecated at this point. https://developer.riotgames.com/docs/lol#summoner-names-to-riot-ids_summoner-names-post-migration

I was really unfamiliar with the Orianna repo but I got it working as I needed for my personal project. This pull request includes:

I realize this is nowhere near a complete PR but it's working enough for my personal project for now, with no breaking changes to the existing codebase to my knowledge. I figured I'd open it since this seems to be something that will be needed in the future anyway, and if anyone wants to use some of the work here it's here to use. I figure this might need a more comprehensive implementation.

There are some questionable decisions (especially in the APIs) that I made because I don't think the pattern of Riot Ids gameName and tagLine fit in with any existing design patterns. I just wanted to get something working in an afternoon.

Excaliburns commented 11 months ago

Thanks for taking a look! I made some of the changes that were noted above. This is also my first time looking at the internals of Orianna so I don't have the fullest grasp of how everything fits together.

One thing about URL encoding - I didn't see any other endpoints (like summoner name) that did it. I think it's encoded as part of the URLBuilder in the HTTPClient (here).

If this is a special case and needs to be encoded otherwise (or I'm just flat out wrong), I can make the change!

Derpthemeus commented 11 months ago

Good catch, you're correct that URL encoding is handled elsewhere. LGTM, thanks for the contribution!