dolejska-daniel / riot-api

Riot League of Legends & DataDragon API wrappers for PHP7 and PHP8.
GNU General Public License v3.0
112 stars 25 forks source link

[Question] Is there a way to get the champion's icon without wrapping it in an image element? #16

Closed zaclummys closed 6 years ago

zaclummys commented 6 years ago

I'd like to get the full address of the static files.

dolejska-daniel commented 6 years ago

Hello @zaclummys,

it is not currently possible to retrieve only image URL, but it is possible to easily extract it from generated Html class instance. Heres how:

Source:

DataDragonAPI::initByCdn();
var_dump(DataDragonAPI::getChampionIcon('Orianna')->getAttribute('src'));

Output:

string(69) "http://ddragon.leagueoflegends.com/cdn/8.9.1/img/champion/Orianna.png"

I will add functions, that will only generate image URLs, also supporting SSL as soon as I get to it. Thank you for your question - if you've got any more questions, feel free to ask, otherwise, mark this issue as closed.

Daniel

dolejska-daniel commented 6 years ago

Hi there again,

you can now refer to v2.0.0-rc.4(299599c), it should contain what you've asked for 😺.

zaclummys commented 6 years ago

@dolejska-daniel, I have an suggestion.

Create a new method called RiotAPI::region to create a new instance with same settings but different region setting.

class RiotAPI {
    // ...
    public static region (RiotAPI $instance, Region $region) {
        $settings = clone $instance->settings; // clone all settings

        $settings[
            RiotAPI::SET_REGION => $region, // change region setting
        ];

        return new RiotAPI(settings); // return a new instance
    }
}

$RiotAPINorthAmerica = new RiotAPI([
    RiotAPI::SET_KEY => '',
    RiotAPI::SET_REGION => Region::NORTH_AMERICA,
]);

$RiotAPIBrazil = RiotAPI::region($RiotAPINorthAmerica, Region::BRAZIL);

// or

$RiotAPI = new RiotAPI([
    RiotAPI::SET_KEY => '',
]);

$RiotAPIBrazil = RiotAPI::region($RiotAPI, Region::BRAZIL);
dolejska-daniel commented 6 years ago

Hi again @zaclummys,

I don't see a reason, why would that be useful. You can easily change current region with RiotAPI::setRegion(...) or change it only temporarily using RiotAPI::setTemporaryRegion(...) with ability to revert it to the previous region setting using RiotAPI::unsetTemporaryRegion().

Tell me when you would need to create completly new instance with only different region settings, even when you've got the ability to change it during its lifetime.

Daniel

zaclummys commented 6 years ago

@dolejska-daniel,

You're right! I hadn't noticed RiotAPI::setTemporaryRegion(...).

That method solves my problem. Thank you!

zaclummys commented 6 years ago

@dolejska-daniel,

I tested some endpoints and got another suggestion.

/lol/static-data/v3/champions/{id} can only be requested 10 times a hour (10:3600). It's not feasible to use it that way.

Some rioters recommend requesting the entire list and caching it instead of requesting items individually. (https://discussion.developer.riotgames.com/questions/3204/why-am-i-getting-statusmessagerate-limit-exceededs.html)

I think a many-to-one caching system can solves that.

/lol/static-data/v3/champions/1 +----+
                                     |----+ /lol/static-data/v3/champions
/lol/static-data/v3/champions/2 +----+

What do you think about it? Have you already created a way to solve this?

dolejska-daniel commented 6 years ago

Hey, @zaclummys,

the idea behind is this great, but this type of change made to the function accessing /lol/static-data/v3/champions/{id} endpoint would result in endpoint being completly inaccessible by the library - it would instead of that endpoint use data from different endpoint.

Because if the functionality of the port would eventually change in the future or if someone would really want to actually use the endpoint, they would not be able to.

People are not dummies, so when using StaticData, they will implement this on their side (eventually 😆, when they notice the low limit on StaticData endpoints). Anyhow, this approach is used in StaticData linking feature, so whatever for would someone want to use the mentioned endpoint, they don't actually really need to, when they can use StaticData linking with any request made to the API.

Also, next time, please make sure to create new issue, this idea is not really related to the current issue topic and since we've solved this, I'll mark it as closed 😺.

Daniel