search

april / decklist

The code behind decklist.org, which generates DCI Registration Sheets.
https://decklist.org/
MIT License
44 stars 20 forks source link

remove unneeded unicode handling #35

Closed tooomm closed 6 years ago

tooomm commented 6 years ago

The Æ Ligature (Non-Functional)

Æ, pronounced "ash," is the loneliest ligature. How many of you even knew what it was called? With the introduction of Aetherborn and no desire whatsoever to ask people to type Ætherborn, that letter also has no place in Oracle anymore, and forty-four cards got an exceptionally rare name change in the Oracle card reference.

from https://magic.wizards.com/en/articles/archive/feature/kaladesh-update-bulletin-2016-09-30

Some time ago the regarding unicode characters got removed from all cards by wizards. Your card source "mtgjson", reflects that change since version 3.10.4, too. In return that character can't show up any longer.

tooomm commented 6 years ago

Can you help with tracking down the correct places where they can/should be removed @Nightfirecat?

You can directly commit changes to my branch as well! Actually, just maintainers can... sry.

Nightfirecat commented 6 years ago

Within the file, the only lines that directly work with the ae fix are L122-L128, L193-L195, L255-L257, and L326-L328. I'll try to make some time after work today to review them and get back to you on that.

Nightfirecat commented 6 years ago

Well, based on a quick test on my own, it looks like all of that code can effectively be removed. I added a commit to my own copy of your tooomm-remove_ae branch which removes all of the "ae" parsing/sorting code. Feel free to merge/rebase/re-author it as you want into your branch. :)

tooomm commented 6 years ago

@Nightfirecat Did you add several cards with capital Æ and lower case æ etc. manually to the decklist to test? Also, what happens if somebody sends such characters via direct linking to the webpage?

Nightfirecat commented 6 years ago

I did not; that should be resolved by adding code above recognized = ... which reads something like this:

// Only perform lookup using "ae", not "Æ" or "æ"
card = card.replace('\u00c6', 'Ae').replace('\u00e6', 'ae');
tooomm commented 6 years ago

@Nightfirecat I copied your changes and suggestion over. Could you test some of the mentioned cases locally, please? I don't have it running...

Nightfirecat commented 6 years ago

Manually entering (and passing via parameter) 1 Ætherling to the main deck is parsed/displayed properly.

You should be able to test locally on your machine by just opening index.html in your browser.

tooomm commented 6 years ago

Jep, that works... thanks. :)

Nightfirecat commented 6 years ago

@april This can be closed, #42 implemented these changes.