Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added check for duplicate cards #300

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

added check for duplicate cards #300

wants to merge 1 commit into from

Conversation

rglerum
Copy link

@rglerum rglerum commented Aug 6, 2023

Fixes #287

Tested on the new spoilers; master has multiple copies of the new cards, this resolves that. Also tested on KHM w/ double-sided cards.

Master:

    <card>
      <name>Alrund, God of the Cosmos // Hakka, Whispering Raven</name>
      <text/>
      <prop>
        <type>Legendary Creature — God // Legendary Creature — Bird</type>
        <maintype>Creature</maintype>
        <cmc>5</cmc>
      </prop>
      <set rarity="Mythic Rare" picURL="https://cards.scryfall.io/normal/front/b/7/b751cf69-0a02-4cd2-abd4-cdb65ca620a8.jpg?1631054227">KHM</set>
      <tablerow>2</tablerow>
    </card>
    <card>
      <name>Alrund, God of the Cosmos // Hakka, Whispering Raven</name>
      <text/>
      <prop>
        <type>Legendary Creature — God // Legendary Creature — Bird</type>
        <maintype>Creature</maintype>
        <cmc>5</cmc>
      </prop>
      <set rarity="Mythic Rare" picURL="https://cards.scryfall.io/normal/front/5/d/5d131784-c1a3-463e-a37b-b720af67ab62.jpg?1665157358">KHM</set>
      <tablerow>2</tablerow>
    </card>

Branch:

    <card>
      <name>Alrund, God of the Cosmos // Hakka, Whispering Raven</name>
      <text/>
      <prop>
        <type>Legendary Creature — God // Legendary Creature — Bird</type>
        <maintype>Creature</maintype>
        <cmc>5</cmc>
      </prop>
      <set rarity="Mythic Rare" picURL="https://cards.scryfall.io/normal/front/b/7/b751cf69-0a02-4cd2-abd4-cdb65ca620a8.jpg?1631054227">KHM</set>
      <tablerow>2</tablerow>
    </card>

ebbit1q
ebbit1q requested changes Aug 7, 2023
View reviewed changes
Copy link
Member

@ebbit1q ebbit1q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a nice idea, however cockatrice when reading the xml will merge duplicated entries, this pr will instead ignore them.

please merge duplicated entries in your pr, almost all tags will be identical in duplicates except for the <set> entries, we'd like to retain all set tags with unique contents inside the tags, having unique arguments to the tag does not matter as those will get ignored (wouldn't be harmful though), what matters is what's in between the >tags<, they will only be different if a card is in multiple prerelease sets at the same time.

@tooomm
Copy link
Member

tooomm commented Feb 14, 2024

Hi @rglerum, did you have a chance to look into the review and comment from ebbit?

Do you plan to adjust your code for that? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebbit1q ebbit1q ebbit1q requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cards are duplicated in spoiler files when there are several pictures available
3 participants
@rglerum @tooomm @ebbit1q