Closed tylermurry closed 5 years ago
27 boosters from M20 gives me a very strong clue as to what's going on here: I think you aren't getting enough dual lands. You can see here that we add a dual land to the pack for ~ 5 out of every 12 packs: https://github.com/jjallaire/draftpod/blob/master/src/store/modules/draft/set/set-m20.js#L23-L27.
This is random though, so your 27 packs might have had less dual lands than normal (making it tough to get enough in a pool). If you replace that block with:
return cards.concat(selectCards(filters.basicLand, 1));
My guess is that things will work.
That said, I think we might need to be a bit more forgiving here and just fill in with basic lands.
I made that change and it doesn't fix it, so perhaps we're somehow short of commons. In any case we should still be able to handle this scenario. Will investigate further.
I'm wondering if maybe the issue is that there are no basic lands in the cardpool? Most of the time you don't need basics in the cardpool but in the case of M20 you do since they are included in packs (those that don't have dual lands)
I thought the same thing and added 100 of each basic land just to be sure. No luck. I also tried adding 50 of every card instead of the current count which gave me 10k cards in the pool. No luck.
I will say that it's as if the engine didn't recognize that I added basics. It still counted them as 405 cards with 500 extra basics.
[image: image.png]
On Sun, Oct 13, 2019 at 3:01 PM J.J. Allaire notifications@github.com wrote:
I'm wondering if maybe the issue is that there are no basic lands in the cardpool? Most of the time you don't need basics in the cardpool but in the case of M20 you do since they are included in packs (those that don't have dual lands)
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jjallaire/draftpod/issues/58?email_source=notifications&email_token=AB7JF3IS4J2NLA6HRCPULGLQON5CPA5CNFSM4JAIKVWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBC6XKQ#issuecomment-541453226, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7JF3MVGEYVAR3LHO2MW4DQON5CPANCNFSM4JAIKVWA .
As I was stepping through in Chrome, I noticed that some packs only had 14 cards and not 15 so the basic land theory seems sound. Investigating on my end as well.
Tyler
On Sun, Oct 13, 2019 at 3:04 PM Tyler Murry tyman7@gmail.com wrote:
I thought the same thing and added 100 of each basic land just to be sure. No luck. I also tried adding 50 of every card instead of the current count which gave me 10k cards in the pool. No luck.
I will say that it's as if the engine didn't recognize that I added basics. It still counted them as 405 cards with 500 extra basics.
[image: image.png]
On Sun, Oct 13, 2019 at 3:01 PM J.J. Allaire notifications@github.com wrote:
I'm wondering if maybe the issue is that there are no basic lands in the cardpool? Most of the time you don't need basics in the cardpool but in the case of M20 you do since they are included in packs (those that don't have dual lands)
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jjallaire/draftpod/issues/58?email_source=notifications&email_token=AB7JF3IS4J2NLA6HRCPULGLQON5CPA5CNFSM4JAIKVWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBC6XKQ#issuecomment-541453226, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7JF3MVGEYVAR3LHO2MW4DQON5CPANCNFSM4JAIKVWA .
I think we needed 2 fixes here:
1) Be more forgiving if there aren't enough dual lands (since it's random, you might not see this error every time but it would probably come up here and there w/ a 27 pack pool). That change is here: https://github.com/jjallaire/draftpod/commit/aa5b626b7821d90e05fe3e3301b929afba62cf96
2) Add the basics to the pool. If you add this to the end of your CSV then things should work correctly (they do on my end):
4,0,Plains,Core Set 2020,261,Near Mint,English,,,,,,,,
4,0,Plains,Core Set 2020,262,Near Mint,English,,,,,,,,
4,0,Plains,Core Set 2020,263,Near Mint,English,,,,,,,,
4,0,Plains,Core Set 2020,264,Near Mint,English,,,,,,,,
4,4,Island,Core Set 2020,265,Near Mint,English,,,,,,,,
4,4,Island,Core Set 2020,266,Near Mint,English,,,,,,,,
4,4,Island,Core Set 2020,267,Near Mint,English,,,,,,,,
4,4,Island,Core Set 2020,268,Near Mint,English,,,,,,,,
4,0,Mountain,Core Set 2020,273,Near Mint,English,,,,,,,,
4,0,Mountain,Core Set 2020,274,Near Mint,English,,,,,,,,
4,0,Mountain,Core Set 2020,275,Near Mint,English,,,,,,,,
4,0,Mountain,Core Set 2020,276,Near Mint,English,,,,,,,,
4,4,Swamp,Core Set 2020,269,Near Mint,English,,,,,,,,
4,4,Swamp,Core Set 2020,270,Near Mint,English,,,,,,,,
4,4,Swamp,Core Set 2020,271,Near Mint,English,,,,,,,,
4,4,Swamp,Core Set 2020,272,Near Mint,English,,,,,,,,
4,4,Forest,Core Set 2020,277,Near Mint,English,,,,,,,,
4,4,Forest,Core Set 2020,278,Near Mint,English,,,,,,,,
4,4,Forest,Core Set 2020,279,Near Mint,English,,,,,,,,
4,4,Forest,Core Set 2020,280,Near Mint,English,,,,,,,,
I still think that this will be a common source of error/frustration, so we should probably automatically add the lands if they aren't there.
Bingo, yep I think that's the right fix too. Shouldn't have to have basics in the collection to smooth it over so I like that. What does your CI/CD process look like? Is that change something that will go in soon?
I appreciate you hoping on that so quickly! Again, if you need any assistance in the future I would be happy to help out where I can.
Thanks! Tyler
On Sun, Oct 13, 2019 at 3:14 PM J.J. Allaire notifications@github.com wrote:
I think we needed 2 fixes here:
1.
Be more forgiving if there aren't enough dual lands (since it's random, you might not see this error every time but it would probably come up here and there w/ a 27 pack pool). That change is here: aa5b626 https://github.com/jjallaire/draftpod/commit/aa5b626b7821d90e05fe3e3301b929afba62cf96 2.
Add the basics to the pool. If you add this to the end of your CSV then things should work correctly (they do on my end):
4,0,Plains,Core Set 2020,261,Near Mint,English,,,,,,,, 4,0,Plains,Core Set 2020,262,Near Mint,English,,,,,,,, 4,0,Plains,Core Set 2020,263,Near Mint,English,,,,,,,, 4,0,Plains,Core Set 2020,264,Near Mint,English,,,,,,,, 4,4,Island,Core Set 2020,265,Near Mint,English,,,,,,,, 4,4,Island,Core Set 2020,266,Near Mint,English,,,,,,,, 4,4,Island,Core Set 2020,267,Near Mint,English,,,,,,,, 4,4,Island,Core Set 2020,268,Near Mint,English,,,,,,,, 4,0,Mountain,Core Set 2020,273,Near Mint,English,,,,,,,, 4,0,Mountain,Core Set 2020,274,Near Mint,English,,,,,,,, 4,0,Mountain,Core Set 2020,275,Near Mint,English,,,,,,,, 4,0,Mountain,Core Set 2020,276,Near Mint,English,,,,,,,, 4,4,Swamp,Core Set 2020,269,Near Mint,English,,,,,,,, 4,4,Swamp,Core Set 2020,270,Near Mint,English,,,,,,,, 4,4,Swamp,Core Set 2020,271,Near Mint,English,,,,,,,, 4,4,Swamp,Core Set 2020,272,Near Mint,English,,,,,,,, 4,4,Forest,Core Set 2020,277,Near Mint,English,,,,,,,, 4,4,Forest,Core Set 2020,278,Near Mint,English,,,,,,,, 4,4,Forest,Core Set 2020,279,Near Mint,English,,,,,,,, 4,4,Forest,Core Set 2020,280,Near Mint,English,,,,,,,,
I still think that this will be a common source of error/frustration, so we should probably automatically add the lands if they aren't there.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jjallaire/draftpod/issues/58?email_source=notifications&email_token=AB7JF3I4UXJQLBSO42KJ2VLQON6TFA5CNFSM4JAIKVWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBC7CKA#issuecomment-541454632, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7JF3M7KOL2TZTMMMD6GGLQON6TFANCNFSM4JAIKVWA .
Commits to master are deployed within a few minutes to https://master.draftpod.org. The production branch deploys to https://draftpod.org (I have already merged this change to production so it should be available both places).
Dope. Nice setup!
Just submitted a feature request as well. Love the site!
Thanks, Tyler
On Sun, Oct 13, 2019 at 3:43 PM J.J. Allaire notifications@github.com wrote:
Commits to master are deployed within a few minutes to https://master.draftpod.org. The production branch deploys to https://draftpod.org (I have already merged this change to production so it should be available both places).
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jjallaire/draftpod/issues/58?email_source=notifications&email_token=AB7JF3O5RIJJNWSAURC2TXDQOOCAXA5CNFSM4JAIKVWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBC74EQ#issuecomment-541457938, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7JF3KLD4NS75N4NQRRLGDQOOCAXANCNFSM4JAIKVWA .
This commit should ensure that a cardpool like the one you originally uploaded will still work fine (we just automatically provide 10x the basics for sets that use basics in their booster packs): https://github.com/jjallaire/draftpod/commit/12ba39038ce74c72d832fc9340ed42bd5d767fbe
I have a collection of 405 cards that was formed by opening 27 boosters of M20, however I am not able to use this cardpool to start a draft and get the following error:
Here is the CSV export from deckbox.org: