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.

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

Search the cards while typing in the deck editor and ignore the case #1663

Merged
merged 1 commit into from Apr 26, 2017
Merged

Search the cards while typing in the deck editor and ignore the case #1663

merged 1 commit into from Apr 26, 2017

Conversation

Zayelion
Copy link

fix for #1658

@IceYGO
Copy link
Collaborator

IceYGO commented Jan 26, 2016

I cannot very well review my own commit, so if anyone else want to take a look at it...

I am pretty sure this commit only affect the deck editor and not the in-game dialog.
It is probably just a line to change in order to make it work though.

@Zayelion
Copy link
Author

True, given the complexity and lack of a test framework someone needs to compile and test this.

@adrianojn
Copy link

👍 LGTM

@Zayelion
Copy link
Author

LGTM

@salix5
Copy link
Collaborator

salix5 commented Mar 24, 2016

My opinion:
Case sensitive is not a bug, since the card names in the official database do have uppercase and lowercase letters, and the names are case sensitive.
I believe that XYZ-Dragon Cannon is not an "Xyz" card without further note in wikia is an example.

@Zayelion
Copy link
Author

That logic is faulty. It fundamentally creates difficultly using the program if the user is not familiar with the exact capitalization of the cards. It has nothing to do with archetypes, there is a separate search filter for that.

I set up a strawpoll http://strawpoll.me/7170410/r and gave it to reddit to vote. I'll pass it off to the DevPro userbase when I get home.

@DailyShana
Copy link
Contributor

DailyShana commented Mar 24, 2016 via email

@Zayelion
Copy link
Author

We can introduce an option called 'latin=1' to enable case-free searching
and fixing the alignment for English.

and the r key changing the font so it is illegible.

@adrianojn
Copy link

@salix5 please read about Letter case

In particular:

If an alphabet has letter case, all or nearly all letters have both forms. Paired forms are considered variants of the same letter: they have the same name and pronunciation and will be treated identically when sorting in alphabetical order.

So yes, this is a bug for all western languages.

This is an example of a bug in Ygopro caused by cultural differences (only western languages have letter case). Others examples are:

  • Ygopro does not support left-to-right languages
  • Word wrapping is wrong for all languages expect CJK

@Zayelion
Copy link
Author

This issue isnt unique either, this is a generic programmer oversight because its harder to do.

Its hard to quantify why this "feels wrong to the user", but the point is, a user interface should be user focused.

@salix5
Copy link
Collaborator

salix5 commented Mar 25, 2016

About letter case:
I agree that the query text should be case-insensitive now, since I didn't notice that many databases(including the official card database) do take this approach.

About NormalizeChar:
My opinion is that we should not put any limit on the query text except for the preserved chars(@ for setname, $ for card name). There are cards has signs like #. / in their names, so I think that converting the letter case is enough.

About EGET_EDITBOX_CHANGED:
This feature may be convenient in alphabetical language like English, but it is somewhat unnecessary in languages like Chinese. For example, typing 機殼(Qli) will search for 機 first, which will find 500+ irrelevant cards since this character appears in 機械族(Machine type), and many other setnames/nouns have similar problems.

DeckBuilder may involve some language-specific details, and I don't know much about this part.
Since DeckBuilder is a rather independant module, maybe you can put some of these features in other language-specific branches?

@mercury233 mercury233 merged commit 0586ca0 into Fluorohydride:master Apr 26, 2017
@mercury233
Copy link
Collaborator

mercury233 commented Apr 26, 2017

97a3b04 added a switch for auto search in system.conf

@mercury233
Copy link
Collaborator

3d75697 I agree with salix5 on NormalizeChar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants