-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
TFT Implementation #80
Comments
Hello @boraguler, I'm pleased to see you are still around! The implementation of APIs for other games than League of Legends comes with necessary changes to library structure... I've been thinking for a while how to tackle the issue and I think I'll have to split this library to separate modules for each game. I'll try to have something to show by the end of the month, but I cannot promise anything, since this will require decent amount of work. |
Hi m8, yeah I'm always around since my platform uses your awesome wrapper <3 Nowadays I'll be adding TFT to the platform with kinda all features. So I thought to go either write my own simplest workaround till you add necessary endpoints, or try to make them within this wrapper. Tho I'm not that much expert as you, but somehow I may manage creating necessary endpoints / models / dtos etc. I tried to examine your new additions for TFT yesterday, and yes I've come to the same conclusion as you that TFT shall be a separate module. Also a side note, I've got an internal rumour that on June-July, new endpoints will be made for TFT, also Valorant is on the way. Guess need to separate them all. How do you want me to proceed, by just cheking-out and trying to modify-pull to this repo? Or shall I fork and let you join on the new one? |
If you're up for the task feel free to implement missing endpoints for TFT in LeagueAPI.php, it honestly should be just copy-pasta of methods already implemented for TFT League resource with result objects changed based on what's on Riot's Dev portal. I will have to separate these changes into their own module later, but it will surely help me if it has already been implemented! Feel free to send PRs to this repo 🥳. |
Aight then, may take few days to find some focused time due to current workload. Soz for my bad code composition, let me tell you that in advance :) |
@dolejska-daniel any ideas how to do this for now? I want to create repo with TFT support. I think it is looks like https://github.com/dolejska-daniel/riot-api-league but I do not understand some code for now, for example how to count resource values and what is it - https://github.com/dolejska-daniel/riot-api-league/blob/e7a6217b70ee93fbeedb68afea80b4e1a27cd07f/src/LeagueAPI/LeagueAPI.php#L157. But I think I can skip it for now and create DTO related to docs and all should works fine. One more question) do you generate this code or write by yourself? I see a lot of docblocks and it needs huge time for create and support all of this. |
@TheMY3 The resources are essential for internal per-resource caching. The identifiers are from https://developer.riotgames.com/apis#tft-league-v1 (the identifier is hidden in the HTML here, for example, You are indeed correct, it will look the same as the LeagueAPI, but I'd like to separate it into a repository of its own (as shown here). The repository can be found here: https://github.com/dolejska-daniel/riot-api-tft. If you wish to contribute, please create an issue (link/mention this issue) and corresponding PR -- I am more than happy to contribute to it as well with some pre-generated API objects (like in the case of League API: https://github.com/dolejska-daniel/riot-api-league/tree/master/src/LeagueAPI/Objects). I already have those for TFT endpoints. |
@TheMY3, by the way, there is a tool (https://github.com/dolejska-daniel/riot-api-docs_parser) to generate DTO classes from the docs automatically -- that is what I use to create them easily. |
@dolejska-daniel this is exactly what I was looking for, thanks |
@dolejska-daniel have few questions)
There were more questions, but for now I forgot about them, I'll be back as soon as I remember, if you don't mind) |
@TheMY3 Glad you are still looking into this 😄
For now, open the PR, and I can help you with some of the parts I know. |
I found it, by problem in some tests for me that I set RU region, but for some endpoints saved to file some of EU regions, when tests with live data - tests passed, when store data - some tests failed. Do not found solution for now and think better to find EU profile and store data for it. Upd. Nvm, found solution) |
Done - dolejska-daniel/riot-api-tft#2 If everything goes ok and it will be possible to reduce the dockblocks, then I think I can then get to valorant in future, maybe 🤔 |
@dolejska-daniel maybe you miss notification, PR waiting for 3 weeks, I almost forgot about it 🙂 |
@dolejska-daniel hello again, maybe I can fork this? If you have no time :( |
"question"
Any ETA on TFT side @dolejska-daniel ?
The text was updated successfully, but these errors were encountered: