add _substr_[pos]_[len] to picture download templates

[ebbit1q]

Related Ticket(s)

• PictureLoader: Replace downloadedPics cache with QNetworkCache #4756

Short roundup of the initial problem

this comment mentions the option for us to skip using the scryfall api by going for the card image location directly: #4756 (comment)

scryfall card cdn uses urls with this format: cards.scryfall.io/[card size]/[card face]/[first hex of id]/[second hex of id]/[complete id].jpg?[unix timestamp of update]

this format is mostly compatible with our url formatting except for the option to get parts of the id instead of the full id. if we were to allow getting parts of the id in our formatting template we'd be able to use this template to download directly from the cdn without going through the api: https://cards.scryfall.io/large/!prop:side!/!set:uuid_substr_0_1!/!set:uuid_substr_1_1!/!set:uuid!.jpg?latest

scryfall's policy is a bit unclear on this, on one hand they don't seem to encourage doing this in case the url is inaccurate but on the other hand they wouldn't mind not spending bandwidth on a request I'd guess.

getting the timestamp of the last update is not possible for us without using the api, storing this information seems a bit pointless as it can change after oracle has run. the timestamp is optional and afaik is used to force the cdn to fetch the most recently updated version as their aws will have cached versions of the image on multiple worldwide instances, changing the url forces it to update the cache, if this is the case the effects of not including the timestamp are limited. randomising this extra bit to the url (or using the current time for it) is a bad idea and will pretty much break #4756 as well. instead we could use some other identifier that is unique to that client, like the clientid, it doesn't seem worth it though. if this is a concern we should instead focus on the client having this exact same issue first, as the client will not download new images if it's on its own disk cache.

What will change with this Pull Request?

• add the template modifier _substr_[pos]_[len] to the picture loader, note that it applies before _fill_with
• after this is merged the wiki on https://github.com/Cockatrice/Cockatrice/wiki/Custom-Picture-Download-URLs#filling-values should be updated to include this

[ebbit1q]

in my testing using this template url works great and is 3 times faster because it avoids any redirects: https://cards.scryfall.io/large/!prop:side!/!set:uuid_substr_0_1!/!set:uuid_substr_1_1!/!set:uuid!.jpg?latest

[ZeldaZach]

SF discourages direct CDN links as they're not guaranteed to be consistent over time. I just worry this could cause us headaches in the future if they change their folder structures

[ebbit1q]

I'm not advocating setting this as the default option, I'm just enabling the option by adding a broadly applicable template option.

[Elarnon]

in my testing using this template url works great and is 3 times faster because it avoids any redirects: https://cards.scryfall.io/large/!prop:side!/!set:uuid_substr_0_1!/!set:uuid_substr_1_1!/!set:uuid!.jpg?latest

What I was thinking in #4756 (comment) was rather along the lines to add picURL entries to cards.xml like there is in tokens.xml (maybe changing the format a little bit to have different picURL values depending on quality & language settings). Then we would get the CDN URLs from ScryFall (or rather mtgjson, I assume they are there?) in bulk on DB update.

[ebbit1q]

that would overrule users' configured download url templates, I can see something like that implemented with #4109 instead (this would replace oracle)

[Elarnon]

Right, picURL is tried first, not last, I got confused.

[ebbit1q]

the goal of this pr isn't the subversion of scyfall's api, it simply expands the url template language

[tooomm]

SF discourages direct CDN links as they're not guaranteed to be consistent over time. I just worry this could cause us headaches in the future if they change their folder structures

I'm not advocating setting this as the default option, I'm just enabling the option by adding a broadly applicable template option.
the goal of this pr isn't the subversion of scyfall's api, it simply expands the url template language

Would it be an option to actually make this the default to prevent any issues with rate limits, but leave the API still in there in case that they change something on their end?

1. Try direct Scryfall link
2. Try to get link via Scryfall API
3. Fallback to Gatherer in case Scryfall fails completly

[ebbit1q]

changing the default url templates is out of scope for this pr.

this is possible but would mean we'd need to do an update to the application if they would change this template, we could do a similar approach to the default server list though and have this default be downloaded.

[ZeldaZach]

This PR accomplishes its goal of expanding the ops of the language. I agree with no making this a default. Seems fine to go.

[ebbit1q]

https://github.com/Cockatrice/Cockatrice/wiki/Custom-Picture-Download-URLs#substrings-of-values has been updated