AFR (and most of AFC) Tokens #108

mainman879 · 2021-06-28T14:11:49Z

Tokens added for (so far): Prosperous Innkeeper, Lolth, Spider Queen, Drizzt Do'Urden, and Ellywick Tumblestrum.

Could not find the pictures for these tokens on Scryfall but will add picurls to them when they are added. I have not added any dungeon cards or their relations as I do not know whether they should use the token relation system or not.

Tokens added for (so far): Prosperous Innkeeper, Lolth, Spider Queen, Drizzt Do'Urden, and Ellywick Tumblestrum. Could not find the pictures for these tokens on Scryfall but will add picurls to them when they are added

tooomm · 2021-07-03T17:21:01Z

In order to correctly flag this as WIP and to prevent an accidental merge before it's ready, I converted this into a Draft PR.
Just hit the Ready for review button when you are done! @mainman879 👍

apply some macroing: jV/token k:norm =^df>$p :'<,'>sort :'<,'>norm ^2f<d$^P also add missing set tags make related before reverse-related the template from now on

mainman879 · 2021-07-12T04:04:50Z

@tooomm what do you think about merging this before the final few images get leaked for upcoming tokens (which could take weeks). There's a ton of functionality in these changes that people want now (dungeons mainly). Adding the last few token images can be done later very easily (I will be opening a new PR for the AFC set after this one is merged and can include those last images in it too.)

tooomm · 2021-07-12T09:54:17Z

There are another two weeks before the set officially releases. I think they probably add the other pics by then.

However, with things being available on MTGA and people wanting to play the set already...
It makes sense to have something useful in between I guess? Some people might complain about missing stuff though.

As this is likely to be the new normal for upcoming releases we can give it a try maybe? @Psithief

mainman879 · 2021-07-13T11:25:25Z

The rest of the tokens for AFR are now on scryfall. I'll be adding them later today and setting this as ready for review.

tooomm

(Conflicts with the master branch are resolved.)

Gave this a brief look, I did not double check if there is a token missing or if all card relations are correct.

I assume the empty set tags like <set>AFC</set> are placeholders which do miss card art links for now?

Please update the date at the beginning of the file (and in the dedicated version file) as well!
(Forgot this myself as well when doing the quick beast fix the other day... 🙈 )

tooomm · 2021-07-13T15:18:50Z

tokens.xml

-            <reverse-related exclude="exclude" count="x">Maja, Bretagard Protector</reverse-related>
-            <reverse-related count="x=1">Usher of the Fallen</reverse-related>
+            <reverse-related count="x" exclude="exclude">Maja, Bretagard Protector</reverse-related>
+            <reverse-related>Usher of the Fallen</reverse-related>


@Psithief added the count="x=1" for "Usher of the Fallen" in f0b7fee#diff-ef2c5b2e0216513ba7314de2c6637a3b48b09a5ea3ee44d13c951195fb293697R3716

Maybe the idea was to account for something like double Boast triggers?

But looking at other Boast cards that create tokens, they also have no count value linked in the token file. 👍
https://scryfall.com/search?q=o%3Aboast+o%3Atoken

I think the count for this relation should be removed as Boast is once per turn, and none of the other token relations assume a token doubler or similar.

I think the count for this relation should be removed as Boast is once per turn, and none of the other token relations assume a token doubler or similar.

Agreed.

@mainman879 @tooomm Don't make unrelated changes in a pull request. A separate pull request for clean-up is best.

tooomm · 2021-07-13T15:39:32Z

tokens.xml

+                <pt>4/4</pt>
+            </prop>
+            <set picURL="https://c1.scryfall.com/file/scryfall-cards/large/front/6/5/65f8e40f-fb5e-4ab8-add3-a8b87e7bcdd9.jpg?1626139916">AFR</set>
+            <!-- this token is created by another token only -->


Why not reverse-link to the Tomb of Annihilation dungeon token here?

Instead you linked it the other way around :) https://github.com/Cockatrice/Magic-Token/pull/108/files#diff-ef2c5b2e0216513ba7314de2c6637a3b48b09a5ea3ee44d13c951195fb293697R8783

I did it as related because there is precedence for it in other tokens such as the Ajani emblem that is related to cat tokens. There are a few others as well. It doesn't make a difference either way I suppose but we should keep it consistent one way or the other.

mainman879 · 2021-07-13T16:50:07Z

The empty AFC set tags are for AFC specific tokens that haven't been revealed yet. These can be added later, and mostly serve as a reminder for myself to fill them in when possible.

…nto patch-1

tokens.xml

Psithief · 2021-07-17T05:39:56Z

tokens.xml

            <reverse-related>Prying Blade</reverse-related>
-            <reverse-related count="2">Rapacious Dragon</reverse-related>
            <reverse-related>Ragavan, Nimble Pilferer</reverse-related>
+            <reverse-related count="2">Rapacious Dragon</reverse-related>


Sorting any unrelated entries should not have been done in this pull request.

tokens.xml

Psithief · 2021-07-17T05:47:50Z

tokens.xml

-            <reverse-related exclude="exclude" count="x">Maja, Bretagard Protector</reverse-related>
-            <reverse-related count="x=1">Usher of the Fallen</reverse-related>
+            <reverse-related count="x" exclude="exclude">Maja, Bretagard Protector</reverse-related>
+            <reverse-related>Usher of the Fallen</reverse-related>


@mainman879 @tooomm Don't make unrelated changes in a pull request. A separate pull request for clean-up is best.

tokens.xml

Psithief · 2021-07-17T06:29:43Z

tokens.xml

            <set picURL="https://img.scryfall.com/cards/large/front/1/c/1c97e5b2-a024-4aa7-a9b8-45f441aad138.jpg">M19</set>
-            <reverse-related>Ajani, Adversary of Tyrants</reverse-related>
            <related count="3">Cat   </related>
+            <reverse-related>Ajani, Adversary of Tyrants</reverse-related>


Reordering elements should be a separate pull request.

tokens.xml

Psithief · 2021-07-17T06:45:44Z

Add this URL for Dragon Spirit : https://media.wizards.com/2021/afr/en_7iaWmuoF0c.png

See https://magic.wizards.com/en/articles/archive/card-preview/tokens-adventures-forgotten-realms-2021-07-12 for the rest.

mainman879 · 2021-07-17T13:13:40Z

I do not want to delete the sorting and putting things into a consistent order just to immediately reupload it under 2 other pull requests. If someone wants to do this and edit this PR and reupload it themselves I welcome them to do so. I'll keep those conversations as unresolved.

mainman879 · 2021-07-17T13:58:24Z

Thanks for fixing the tab issues @tooomm . I usually check for this issue manually but must've forgot in my morning grogginess. I'll have to get an actual code editor instead of notepad++ because it seems to love to use tab indents instead of spaces.

Psithief · 2021-07-23T08:31:11Z

I'll mebe reviewing and/or merging this in a few hours time.

Psithief · 2021-07-23T12:17:09Z

I extracted and committed 1b781c2 in your name.

Psithief · 2021-07-23T12:25:05Z

@mainman879 I propose to squash this pull request as per the commits shown here: https://github.com/Cockatrice/Magic-Token/commits/pullrequest108

Psithief

I found two issues. I was sure I mentioned the Death-Priest but apparently not.

tokens.xml

Psithief · 2021-07-23T12:40:29Z

It makes sense to have something useful in between I guess? Some people might complain about missing stuff though.

As this is likely to be the new normal for upcoming releases we can give it a try maybe? @Psithief

What about a spoiler branch that's not quite held to the same standard as the master branch? @tooomm

mainman879 · 2021-07-23T13:35:20Z

@mainman879 I propose to squash this pull request as per the commits shown here: https://github.com/Cockatrice/Magic-Token/commits/pullrequest108

I don't know how to squash or what squashing is on github but whatever you think is good go for it.

AFR (and most of AFC) Tokens

ebbit1q · 2021-07-24T14:37:22Z

I thought you intended to squash them? now we have these commits applied to master directly

Psithief · 2021-07-25T18:16:39Z

@ebbit1q The commits with the same author got squashed in sequence. I would have liked to have squashed it further but would have lost authorship information. If it there was just one author I'd have squashed it to one commit.

ebbit1q · 2021-07-25T22:04:52Z

whatever makes you happy, I don't care much about the small fixes I do

mainman879 added 10 commits June 28, 2021 10:07

AFR Tokens (In-Progress)

9b3b9ac

Tokens added for (so far): Prosperous Innkeeper, Lolth, Spider Queen, Drizzt Do'Urden, and Ellywick Tumblestrum. Could not find the pictures for these tokens on Scryfall but will add picurls to them when they are added

Update tokens.xml

cbe209c

Update tokens.xml

686f7cd

Update tokens.xml

6b5f790

Update tokens.xml

e34816b

Updated to June 30th reveals on scryfall

a6cabbb

Update tokens.xml

d1ba3d0

Note to Self: Add all the dungeons tomorrow

2b3399a

Added Dungeons

e6ebc5d

Removing Accidental duplicate

a6c28f8

mainman879 mentioned this pull request Jul 3, 2021

The "All Tokens" button exists even if all tokens are set to exclude. Cockatrice/Cockatrice#4395

Open

tooomm marked this pull request as draft July 3, 2021 17:18

mainman879 and others added 5 commits July 3, 2021 19:33

Update tokens.xml

52c6d10

Replaces problematic tab indents with spaces

fb31047

Update tokens.xml

7fc681f

formatting

67a2d6c

apply some macroing: jV/token k:norm =^df>$p :'<,'>sort :'<,'>norm ^2f<d$^P also add missing set tags make related before reverse-related the template from now on

"count" before "exclude" formatting on every card for consistency

543b96c

Update tokens.xml

9517a77

Adding all the token images and a couple missed tokens

cdeca34

mainman879 marked this pull request as ready for review July 13, 2021 12:50

Merge branch 'master' into patch-1

f4a0e2d

tooomm reviewed Jul 13, 2021

View reviewed changes

Psithief self-requested a review July 13, 2021 16:37

tooomm mentioned this pull request Jul 14, 2021

Improve documentation #111

Open

Changes version.txt and sourceversion to date of last token addition

e19d815

Merge branch 'patch-1' of https://github.com/mainman879/Magic-Token i…

6a065e4

…nto patch-1

mainman879 changed the title ~~AFR Tokens (In-Progress)~~ AFR (and most of AFC) Tokens Jul 16, 2021

Psithief requested changes Jul 17, 2021

View reviewed changes

Adds most of the changes PsiThief requested

8ae4693

tooomm mentioned this pull request Jul 17, 2021

CI: Check xml file formatting and validate against xsd #65

Open

spaces > tabs

24d29c9

mainman879 requested a review from Psithief July 21, 2021 19:18

Psithief requested changes Jul 23, 2021

View reviewed changes

tokens.xml Outdated Show resolved Hide resolved

tokens.xml Show resolved Hide resolved

Update tokens.xml

4832136

Psithief approved these changes Jul 24, 2021

View reviewed changes

Psithief added a commit that referenced this pull request Jul 24, 2021

Merge pull request #108 from mainman879/patch-1

21ecbdd

AFR (and most of AFC) Tokens

Merge branch 'master' into patch-1

396a32a

Psithief merged commit 3c3b541 into Cockatrice:master Jul 24, 2021

mainman879 deleted the patch-1 branch July 24, 2021 09:28

AFR (and most of AFC) Tokens #108

AFR (and most of AFC) Tokens #108

Uh oh!

Conversation

mainman879 commented Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tooomm commented Jul 3, 2021

Uh oh!

mainman879 commented Jul 12, 2021

Uh oh!

tooomm commented Jul 12, 2021

Uh oh!

mainman879 commented Jul 13, 2021

Uh oh!

tooomm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tooomm Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mainman879 commented Jul 13, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Psithief commented Jul 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mainman879 commented Jul 17, 2021

Uh oh!

mainman879 commented Jul 17, 2021

Uh oh!

Psithief commented Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Psithief commented Jul 23, 2021

Uh oh!

Psithief commented Jul 23, 2021

Uh oh!

Psithief left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Psithief commented Jul 23, 2021

Uh oh!

mainman879 commented Jul 23, 2021

Uh oh!

ebbit1q commented Jul 24, 2021

Uh oh!

Psithief commented Jul 25, 2021

Uh oh!

ebbit1q commented Jul 25, 2021

Uh oh!

Reviewers

Assignees

Labels

mainman879 commented Jun 28, 2021 •

edited

Loading

tooomm left a comment •

edited

Loading

tooomm Jul 13, 2021 •

edited

Loading

Psithief commented Jul 17, 2021 •

edited

Loading

Psithief commented Jul 23, 2021 •

edited

Loading