Apostrophes are fixed! I'll investigate the plus now...
I was going to send you a pull request, but it seems overkill now...Known Issues
1. Partial card-names no longer work. I'll see if I can fix this, but if it's a lot of complication I'd rather just stop supporting this. It does break older posts though.
To be honest, I have never worked on any open source projects, and the prospect of my first pull request is exciting! This is the kind of thing I couldn't learn about in my job, and I'm keen to now. You are welcome to contribute!I was going to send you a pull request, but it seems overkill now...
Swapping exact card names to fuzzy searching in line 32 of CardService gets you a long way...
I suppose I could still proceed with the other approach I was going to take, which was using the autocomplete service when not matched (would need to think about caching approaches for the API calls though).
Sent you a pull request 2 days ago. Since you haven't made any commits since then, reviewing and accepting the changes if they make sense should be really easy.To be honest, I have never worked on any open source projects, and the prospect of my first pull request is exciting! This is the kind of thing I couldn't learn about in my job, and I'm keen to now. You are welcome to contribute!
I did see it! I haven't worked in a few days, but I looked over the proposed changes this morning. I'm going to merge them, and then I think make 2 commits of tweaks: 1. Style changes; and 2. Change the naming of the cached "fuzzy pointer" to something more explicit. PerhapsSent you a pull request 2 days ago. Since you haven't made any commits since then, reviewing and accepting the changes if they make sense should be really easy.
`fuzzyPointer--${cardName}`
.No issue at all - those are all stylistic and without a style or contributor guide I can't know any differently - I tried to mimic the style that was in place, but I tend to slip back to my own preferred style sometimes. You can pull into a local branch, then make your changes and merge the branch (you also can just commit directly to my branch thanks to the Pull Request), or just add your comments to the request, then I will update the code tomorrow in line with that and add a new commit to that request. Normally you would do the latter - I tried to catch you on Discord to discuss, but we are on opposite sides of the world...I did see it! I haven't worked in a few days, but I looked over the proposed changes this morning. I'm going to merge them, and then I think make 2 commits of tweaks: 1. Style changes; and 2. Change the naming of the cached "fuzzy pointer" to something more explicit. Perhaps`fuzzyPointer--${cardName}`
.
In your opinion, is it rude to accept a pull request and then make these sort of tweaks? Should I reject with comments instead?
Cool, that's all as I figured. I'll get you on discord! I don't use it much, but I think that is the best place for this kind of thing.No issue at all - those are all stylistic and without a style or contributor guide I can't know any differently - I tried to mimic the style that was in place, but I tend to slip back to my own preferred style sometimes. You can pull into a local branch, then make your changes and merge the branch (you also can just commit directly to my branch thanks to the Pull Request), or just add your comments to the request, then I will update the code tomorrow in line with that and add a new commit to that request. Normally you would do the latter - I tried to catch you on Discord to discuss, but we are on opposite sides of the world...
Note that the rate limiting is going to cause some serious issues when loading a page with a bunch of cubedeck tags, as frequently happens in the Cube Blogs section. A typical cube deck has over 20 unique cards, so a single blog post with the four winning decklists from last night's draft will already take over a minute to load.RiptideLab Thing 1.1
Embedded card images are live!
Rate Limiting
The only interesting thing about this upgrade is rate limiting. Scryfall asks that users of their API keep requests to 10 per second. You'll notice as you load a page full of card-images, they'll load in a nice, rhythmic fashion. This is the rate limiting in action. However, once they're loaded, the results are cached in your browser. The next time you load that page, all the images should just appear immediately.
Known Issues
The "More details" button has disappeared from tooltips on mobile.
Unknown Issues
If the rate limiting doesn't pass mustard, Scryfall may cancel some requests with a "Too many requests" error. I'm not sure what will happen in this case. Nothing serious, I think.
Next up
Deck and cubedeck tags! Should be straight forward.
Let me know if anything works.
Your math is off - 20 cards will take about 2 seconds to resolve, so 4 decks should take about 10 seconds. And this is only the first time you load the thread.Note that the rate limiting is going to cause some serious issues when loading a page with a bunch of cubedeck tags, as frequently happens in the Cube Blogs section. A typical cube deck has over 20 unique cards, so a single blog post with the four winning decklists from last night's draft will already take over a minute to load.
Oh, ha! I somehow read that as 10 per 10 seconds Never mind, nothing to see here!Your math is off - 20 cards will take about 2 seconds to resolve, so 4 decks should take about 10 seconds. And this is only the first time you load the thread.
We'll see how it goes. The ultimate solution is for me to build something server-side to store Scryfall results. Browsers would send requests to my server, which can get cards from a database, and connect to Scryfall to fill in anything missing or outdated.
The upside is, I could tweak rate-limiting however I like, and I could implement bulk requests (which Scryfall does not support).
The downside is, it's a lot more complication that I'll need to maintain, and a lot more work for my server to do. This is especially true if I try to build global rate limiting into my server's requests to Scryfall. It's a lot more moving parts, and I think it would end up being much harder to keep running.
Ah, well, what I meant was: here's the way I could do it if it turns out to be necessary. But I much prefer to stick with totally in-browser solution if I can.Sweet plans for the ultimate solution though, but yeah, that's going to be tough. Also requires quite a bit of storage, potentially.
No, it‘s not only you. I got the same problem.I am not sure if it is just me but I cannot get into ‘General Discussions’ because an ad is covering the bolded thread title.
I have tried different things but I cannot remove it. (I can’t upload a picture to show because “The uploaded file is too large.”)
Yeah same. It does position better for me now though.For me the ads are wider than my screen, it probably cuts off the outer 15 percent or so on each side.
Perfect!I think I have sorted it. @Velrun does it look better on your device?
[deck]
and [cubedeck]
tags. In fact, they also worked before, but now they work and get card images from scryfall.Creatures (16) 4 Putrid Leech #LSV says don't play any, but...? 4 Sprouting Thrinax 4 Bloodbraid Elf #skill, amirite 2 Siege-Gang Commander 2 Broodmate Dragon Planeswalkers (2) 2 Garruk Wildspeaker Spells (15) 2 Bituminous Blast 4 Blightning 4 Lightning Bolt 3 Maelstrom Pulse 2 Terminate | Lands (27) 9 Forest #who needs dual lands 9 Mountain 9 Swamp | Sideboard (15) 1 Bituminous Blast 2 Deathmark 2 Duress 4 Goblin Ruinblaster 1 Great Sable Stag 1 Liliana Vess 1 Malakir Bloodwitch 1 Master of the Wild Hunt 1 Mind Rot 1 Terminate (0) |