MagicTheGathering / mtg-sdk-dotnet

Magic: The Gathering SDK - C#.NET
MIT License
63 stars 22 forks source link

Filter Cards via query parameters: compile error #55

Closed ghost closed 3 years ago

ghost commented 3 years ago

Hi

Following the readme, I try to make a Task to Filter Cards via query parameters. Using the exact readme code, I get this compile error. What is the correct code line please?

async Task GetCardsByQuery() { //Filter Cards via query parameters ICardService service = serviceProvider.GetCardService(); var result = await service.Where(x => x.Set, "ktk").Where(x => x.SubTypes, "warrior,human").AllAsync(); }

Error CS0411 The type arguments for method 'IMtgQueryable<ICardService, CardQueryParameter>.Where(Expression<Func<CardQueryParameter, U>>, U)' cannot be inferred from the usage. Try specifying the type arguments explicitly.

bmeijwaard commented 3 years ago

The issue here is the the .Where function takes a generic type as an argument and is then locked in on that type on both the property as well as the value to lookup. This causes an issue with array's. Most of the time this is fine for types with a single value such as an int or string.

Can you try this:

var result = await service
                    .Where(x => x.Set, "ktk")
                    .Where(x => x.SubTypes, new[] { "warrior", "human" }) // notice that the lookup value here is a string array to match .SubTypes type.
                    .AllAsync();
ghost commented 3 years ago

This works! Can this error be fixed in the readme, because it's wrong there?

ghost commented 3 years ago

Is it possible to make a search that gives all cards that CONTAIN a substring in the card name?

For example: find all cards having "bolt" in their card name (not case sensitive) as substring This will give these cards: https://gatherer.wizards.com/Pages/Search/Default.aspx?name=+[bolt] Notice it will find cards like Beacon Bolt but also Boltwing Marauder.

Is this possible in the Web API and this library and if yes, what's the correct code line?

bmeijwaard commented 3 years ago

This works! Can this error be fixed in the readme, because it's wrong there?

To be continued.

Is it possible to make a search that gives all cards that CONTAIN a substring in the card name?

For example: find all cards having "bolt" in their card name (not case sensitive) as substring This will give these cards: https://gatherer.wizards.com/Pages/Search/Default.aspx?name=+[bolt] Notice it will find cards like Beacon Bolt but also Boltwing Marauder.

Is this possible in the Web API and this library and if yes, what's the correct code line?

The API should already be searching for any card given the string as a substring. For example armogoy will return any card containing the substring, regardless of its position in the string. If you search for bolt, then it would returns every card that has bolt in it somewhere. It is also case insensitive.

So in the example below this should find all Tarmogoyf cards

await service.Where(x => x.Name, "ARMOGOY").AllAsync();

Does that answer your question?

bmeijwaard commented 3 years ago

Created pull request (https://github.com/MagicTheGathering/mtg-sdk-dotnet/pull/56#issue-530510969) This should make properties like .SubTypes accept a single string, similar as with .Name or .Set as specified in the documentation.

ghost commented 3 years ago

Great, that's what I wanted!

Considering the PR, you changed the array, so is it now again this?

Where(x => x.SubTypes, "warrior,human")

bmeijwaard commented 3 years ago

Great, that's what I wanted!

Considering the PR, you changed the array, so is it now again this?

Where(x => x.SubTypes, "warrior,human")

Yes. you can do .Where(x => x.SubTypes, "warrior,human") or do .Where(x => x.SubTypes, "warrior|human") Queries can have | for an OR operator or have , for AND operator.

ghost commented 3 years ago

But only the card name has possibility to search substrings, all other strings are matched EXACTLY the same, correct?

For example you can't do this to find any subtypes with "hum" inside: .Where(x => x.SubTypes, "hum")

bmeijwaard commented 3 years ago

But only the card name has possibility to search substrings, all other strings are matched EXACTLY the same, correct?

For example you can't do this to find any subtypes with "hum" inside: .Where(x => x.SubTypes, "hum")

That is correct. For subtypes it would appear a full text match is required. So hum will find 0 results, human will find x > 0. This is part of the API and cannot be changed in this SDK.

ghost commented 3 years ago

OK, I understand, thanks for the help!

Now we are talking about queries, I found another strange thing.

A ICard has the property IsMultiColor, which is a boolean telling if the card has more then 1 color. So you expect you can do a query on it, right?

But doing this query is not possible, I get the following error. Is it possible IsMultiColor is missing there?

var cardRequest = cardservice.Where(x => x.IsMultiColor, true);

So I can't currently do this query and I needed it :-)

Error CS1061 'CardQueryParameter' does not contain a definition for 'IsMultiColor' and no accessible extension method 'IsMultiColor' accepting a first argument of type 'CardQueryParameter' could be found (are you missing a using directive or an assembly reference?)

ghost commented 3 years ago

An additional question about doing a query for the card color.

  • Suppose I want to get all Black cards, I now do this query on the Colors property. Colors[0] should contain the color of a card, in case the card is not colorless (and not multi-color).

var cardRequest = cardservice.Where(x => x.Colors[0], "Black");

If I run this query, I get a System.NullReferenceException.

Is this because some cards are colorless and Colors[0] does not exist then?

OR

is this a bug in the library that throws this exception (so the library should handle the exception)?

I'm not sure. So I'm stuck here: how to make a query on a card's color like above?

  • Maybe this should be the correct query I think? But currently I can't query on the IsMultiColor boolean.

var cardRequest = cardservice.Where(x => x.IsMultiColor, false).Where(x => x.Colors[0], "Black");

But then this query will still try to read Colors[0] for cards that have NO color. In this case Colors[0] doesn't exist.

  • Maybe the ICard interface needs a new property (boolean) which tells if a card is colorless or not: IsColorLess And then the correct query to get all Black cards would be:

var cardRequest = cardservice.Where(x => x.IsColorLess, false).Where(x => x.IsMultiColor, false).Where(x => x.Colors[0], "Black");

Please tell me your thoughts about this! I can't make it work with the current state of the library. Maybe I'm doing something wrong?

Thanks in advance!

ghost commented 3 years ago

There are more properties missing like ManaCost in a query (maybe even more missing things). If possible, I would like to have those too so I can use them in my program.

bmeijwaard commented 3 years ago

The API has options for cmc, colors and coloridentity. Looking at a card result set like ie Emrakul, the Aeons Torn. The colors and colorIdentity returns an empty array. Meaning that colorless is probably not searchable thru the API.

IsMultiColor is not an existing parameter in the API.

ManaCost is not an existing parameter. The closest is using the cmc in combination with colors or colorIdentity.

For more information on what the API offers check out the docs here: https://docs.magicthegathering.io/

.Where(x => x.Colors[0], "Black"); will always fails, since x.Colors is referencing a property that is purely for referencing purpose and will never be filled. .Where(x => x.Colors, new [] { "Black" }); should work, until the pull request has been merged and published on NuGet. In that case it will become .Where(x => x.Colors, "Black");

ghost commented 3 years ago

I used (don't know what library version) Colors[0] and it contained the color of the card. For multicolor cards Colors was filled too and it contains each color of the mana cost.

So I don't know what you mean exactly

ghost commented 3 years ago

I used your advice, but unfortunately it doesn't work

This query should give all "ktk" edition Black cards but it returns cards with casting cost using all other colors too. Could you please try it to verify? I can't get it to work...

var cardRequest = cardservice.Where(x => x.Set, "ktk").Where(x => x.Colors, new[] { "Black" });

bmeijwaard commented 3 years ago

I used your advice, but unfortunately it doesn't work

This query should give all "ktk" edition Black cards but it returns cards with casting cost using all other colors too. Could you please try it to verify? I can't get it to work...

var cardRequest = cardservice.Where(x => x.Set, "ktk").Where(x => x.Colors, new[] { "Black" });

The given code snippet should result in performing this request https://api.magicthegathering.io/v1/cards?colors=black&set=ktk. The result set of this will give you all the cards of Khans of Tarkir that contain the color black, but dont have to be only black, it also returns card that share other colors. Like Abzan for example.

I used (don't know what library version) Colors[0] and it contained the color of the card. For multicolor cards Colors was filled too and it contains each color of the mana cost.

So I don't know what you mean exactly

In the result set, yes it would. Since Colors in a collection of ICard the property is filled. Therefor Colors[0] (accessing array index position 0) will work, but not in this specific case where the Colors property is only used to build the query parameters.

ghost commented 3 years ago

Ok, but this still leaves me to the question: how can I get all red/blue/green/white/black ONLY cards and what query to use?

thx

bmeijwaard commented 3 years ago

The API does not provide that advanced filtering. This library also does not encapsulate additional filtering functionality. That would be redundant since we can use ie Linq on the result set to achieve the same.

The SDK output generates a List which can be used for further filtering on your side of the program. An example (not the only way):

// First retrieve the initial chunk of desired cards.
var result = await service
   .Where(x => x.Set, "ktk")
   .Where(x => x.Colors, new[] { "black" })
   .AllAsync();

// Second, filter down the result set with specific / additional filtering.
var onlyBlackCards = result.Value
   .Where(c => c.Colors.All(color => color.Equals("black", StringComparison.OrdinalIgnoreCase)))       // 1
   .Where(c => c.ColorIdentity.All(color => color.Equals("b", StringComparison.OrdinalIgnoreCase)));   // 2
  1. Will exclude multi color cards, such as 'Siege Rhino'.
  2. Will exclude cards with multiple colors in their identity, such as 'Siege Rhino' and 'Kheru Dreadmaw'. The 'Kheru Dreadmaw' won't be removed by the 1. Linq query.
ghost commented 3 years ago

Thx a lot, I will try this!!!

PS: is "Black" or "black" inside the Colors?

bmeijwaard commented 3 years ago

The result set contains ie "colors":["Red"],"colorIdentity":["B","R"], but for the request it is case insensitive. But if you peek in the result set you can see it.

This issue can probably be closed.

ghost commented 3 years ago

Yes, thx!