Loyalty filter added

[Vafthrudnir]

Related Ticket(s)

• Fixes Add Loyalty filter #3084

Short roundup of the initial problem

"Loyalty" was missing from the possible options for card filter.

Screenshots

[tooomm]

• Works as a normal digit filter ✔️
• Utilizes new relative filter, too ✔️
• Placed at correct alphabetical position in filter list ✔️

But I have a problem with using <= and <... ❌
>, >=, =, == and only digits are working!

[ZeldaZach]

So loyalty is technically a string, as there are cards with "X" loyalty. If you want to convert "X" => 0, that's fine with me.

[Vafthrudnir]

Loyalty is stored as an int value in the code, and what caused this problem is that all non-planeswalker cards are indistinguishable from those with 0 or X loyalty (both are stored as 0).
As I see it there are two ways to solve this problem:

• Either apply type filtering automatically to planeswalker whenever loyalty is set,
• or set the default loyalty of all cards to -1, which is updated when a valid loyalty value is read from the cardlist xml

I'd rather choose the second solution due to the runtime we can spare by not having to compare the type of each card to the "Planeswalker" string when loyalty filter is used. What are your opinions?

[ZeldaZach]

I'd prefer the 2nd option as planeswalker is tied to magic. If the field doensn't exist it should be easy to say it's a negative.

[tooomm]

Hmm, we handle X loyalty planeswalkers wrong right now - they are tagged as 0 loyalty:

        <card>
            <name>Nissa, Steward of Elements</name>
            <set rarity="Mythic Rare" muId="426906" num="204">AKH</set>
            <color>U</color>
            <color>G</color>
            <manacost>XGU</manacost>
            <cmc>2</cmc>
            <type>Legendary Planeswalker — Nissa</type>
            <tablerow>1</tablerow>
            <text>+2: Scry 2.
0: Look at the top card of your library. If it's a land card or a creature card with converted mana cost less than or equal to the number of loyalty counters on Nissa, Steward of Elements, you may put that card onto the battlefield.
−6: Untap up to two target lands you control. They become 5/5 Elemental creatures with flying and haste until end of turn. They're still lands.</text>
            <loyalty>0</loyalty>
        </card>

The root issue is from upstream: mtgjson/mtgjson#320

I prefer to not do any shenanigans here. Can we make the loyalty filters support strings already?
That way once mtgjson fixes things, it'll work in all clients immediately.
It would return no result if somebody searches for Loyalty = X right now, that shouldn't be too bad?
There is only one single X-loyalty planeswalker as of now...

[Vafthrudnir]

If we don't do any shenanigans here, three planeswalkers will be left out when we filter to loyalty=0:

• Arlinn, Embraced by the Moon
• Garruk, the Veil-Cursed
• Nissa, Steward of Elements

I think it would be nice to remain consistent with the handling of X values: if they mean 0 in CMC, they should mean 0 here as well - or is it totally different?
Regarding the other two, they probably really shouldn't show up in this query, since their loyalty is technically not 0 but undefined.

[ZeldaZach]

I'm happy with this PR as-is. When MTGJSON v4 is released, I'll give ample notice to put a change in. Maybe we can leave a TODO alert to update this when that happens.

[tooomm]

I think it would be nice to remain consistent with the handling of X values: if they mean 0 in CMC, they should mean 0 here as well - or is it totally different?

Planeswalker with Loyalty = 0 don't/can't exist 😉
But I understand your point.

For the back-sides of the transform-planeswalkers (Arlinn, Embraced by the Moon and Garruk, the Veil-Cursed) shouldn't they have no Loyalty at all rules wise?
mtgjson has that right btw - so this should end up in a new oracle parsing issue maybe.
But maybe we just add that key to every planeswalker card? And these are planeswalkers on both sides... unlike these DFC planeswalkers from Origins.
For Kytheon, Hero of Akros we corractly have no loyalty value in cards.xml

[Vafthrudnir]

Ok, I made loyalty into a string, to handle X values whenever we'll have any.
I also checked the mtgjson database, and it indeed shows no loyalty value for the two back-sided planeswalkers, and shows null instead of 0 for Nissa - I think the parsing itself should be modified to transform this to X as a separate issue.
If you agree I'd leave this PR as is, and leave the rest to the json parsing issue.

[tooomm]

Looks good now. The <= etc. work again for me, too.

If we do nothing on our end and mtgjson changes their value from "loyalty": null to "loyalty": "X", Oracle then parses that correctly and stores the X as <loyalty>X</loyalty>.
Just simulated that. 👍

Only issue: relative filtering wouldn't work to find Nissa yet.
You have to filter loyalty for exactly x or X.

[ZeldaZach]

I'm good here