nicho92 / MtgDesktopCompanion

Cards manager for magic the gathering
Apache License 2.0
155 stars 34 forks source link

PluginsAliasesParser cannot work with sets that have a colon in the key #206

Closed bewarellamas closed 2 years ago

bewarellamas commented 2 years ago

Testing some conversions with the PluginsAliasesParser class and when one of the plugin aliases has a colon in the key it will not read it which is resulting in some NullPointerExceptions when I Am trying to get the new set name.

I made a local reader for the pluginsAliases so I could do my own testing with some Card Kingdom exports and I added the following to the pluginsAliases.json file:

"Card Kingdom CSV": { "nameSet": { "Ravnica: City Of Guilds": "Ravnica", "Zendikar Rising Commander": "Zendikar Rising Commander Decks"
}

The Zendikar Rising Commander will work properly, but Ravnica: City of Guilds will not. There are a few other sets that could cause issues once this is expanded. Not sure if there is a fix for this or not.

bewarellamas commented 2 years ago

Tested with the URLTools function to be sure. Still didn't work. I have a test branch with the pluginsAliases.json file I am using.

https://raw.githubusercontent.com/bewarellamas/MtgDesktopCompanion/cardkingdomcsv-test/src/main/resources/data/pluginsAliases.json

nicho92 commented 2 years ago

the right name is "Ravnica: City of Guilds" , not "Ravnica: City Of Guilds" .;) . when i tried :

var ed = MTG.getEnabledPlugin(MTGCardsProvider.class).getSetById("RAV");
System.out.println(ed.getSet());
System.out.println(o.get("Card Kingdom CSV").getAsJsonObject().get("nameSet").getAsJsonObject().get(ed.getSet()).getAsString());
bewarellamas commented 2 years ago

I made that change and I still got the same result in my CSV as before. The code I am using to get the set name is:

String set = PluginsAliasesProvider.inst().getSetNameFor(this , mc.getProduct().getCurrentSet());

The error I get is 2022-01-14 17:47:32 ERROR org.magic.services.providers.PluginsAliasesProvider 121 - java.lang.NullPointerException: Cannot invoke "com.google.gson.JsonElement.getAsString()" because the return value of "com.google.gson.JsonObject.get(String)" is null

nicho92 commented 2 years ago

can you share me your CardKingdom CSV plugin ? it's strange, because PluginsAliasesProvider 121 is the error line for getSetIdFor() function

bewarellamas commented 2 years ago

I included links to the methods there. Sorry I realized I was using a version of PluginsAliasesProvider that had some code I added for error catching and such.

https://github.com/bewarellamas/MtgDesktopCompanion/blob/cardkingdomcsv-test/src/main/java/org/magic/api/exports/impl/CardKingdomCSVExport.java

https://github.com/bewarellamas/MtgDesktopCompanion/blob/cardkingdomcsv-test/src/main/java/org/magic/services/providers/PluginsAliasesProvider.java

nicho92 commented 2 years ago

Do you still have error ? a make a try .... i can't reproduce error.

I made a test on my CardKingDomPricer :

    public void test() throws IOException
    {
        MTGControler.getInstance();
        MTG.getEnabledPlugin(MTGCardsProvider.class).init();
        var ed = MTG.getEnabledPlugin(MTGCardsProvider.class).getSetById("RAV");
        System.out.println(PluginsAliasesProvider.inst().getSetNameFor(this, ed));

    }

And results is correct.

2022-01-31 11:13:45 DEBUG org.magic.services.network.MTGHttpClient 86  - execute GET https://raw.githubusercontent.com/nicho92/MtgDesktopCompanion/master/src/main/resources/data/pluginsAliases.json HTTP/1.1
Ravnica
nicho92 commented 2 years ago

i added your plugin to mtgcompanion repository

bewarellamas commented 2 years ago

Last I tried the issue was still there. I will try it from the mtgcomanion repo and see what happened. I could have messed up some code somewhere else.

bewarellamas commented 2 years ago

It is working after I pull down the current code. Not sure what I did, but glad you got it (and cleaned it up).