ClassCastException when constructing Gdl programmatically

[gsj601]

I was trying to programmatically construct variations of Nim (specifically, nim1 on games.ggp.org/base). To be sure of what part broke, if any, my process was to incrementally build a List (for later use in constructing a Propnet) out of a String per rule. For each String, I was calling Game.preprocessRulesheet(), then calling SymbolFactory.create() to get a Symbol, then calling GdlFactory.create() to create a Gdl object of just that one rule. Apparently, preprocessing a rulesheet assumes that there will be many symbols, and so wraps the single rule in parentheses. (The concrete syntax of GDL doesn't normally have parentheses around the whole rule set, so, I don't know why a Symbol would...) This is a problem for the GdlFactory, though, because in createGdl(), it checks if it has a SymbolList, and if it does, it casts to a SymbolList, and then casts the first Symbol inside that list to a SymbolAtom... and throws a ClassCastException. With some debugging println()'s thrown in, this code will reliably throw an Exception:

System.out.println("(role player1)");
String processed = Game.preprocessRulesheet(rule);
System.out.println(processed);
Symbol s = SymbolFactory.create(processed);
System.out.println(s);
return GdlFactory.create(s);

If I remove preprocessRulesheet() from the process, there's no exception, all my GDL strings get turned into Gdl objects, and a Propnet can be successfully built.

So that's one issue... but I don't know how to fix it, or know if it should be fixed. For preprocessing whole rulesheets, it doesn't throw the ClassCastException, so I don't think everyone's working code needs to change to handle the one case where maybe you want to double-check that your code is preprocessed correctly, but it's not an entire rulesheet. But! It did point out what is pretty clearly a bug in GdlFactory's create(Symbol) method:

public static Gdl create(Symbol symbol) throws GdlFormatException
    {
        try
        {
            return createGdl(symbol);
        }
        catch (Exception e)
        {
            createGdl(symbol);
            throw new GdlFormatException(symbol);
        }
    }

The try throws any exception, the same method is tried again, the same exception will always be thrown, and the more meaningful GdlFormatException will never be reached. The fix is to get rid of the call to createGdl inside of the catch. I have a fork with the fix already, and will be making a pull request as soon as I figure out how. (My first one ever!)

[gsj601]

Pull request is here: https://github.com/gsj601/ggp-base-castFix/pull/1

I'm pretty sure that's not how they're supposed to work. But because I already had a github-fork of ggp-base, I can't make another one, and you can only make a github-pull-request between an upstream repo and a repo that github thinks is actually a fork. (Seems like a pretty big flaw in the system to me? My other fork has a lot of other stuff going on in it. I guess, with a properly constructed pull request, only the one difference would get pulled. But stil.)

[AlexLandau]

Game#preprocessRulesheet is for entire rulesheets; you don't need it for individual sentences. Aside from the parentheses, the only other thing it does is to remove comments, which I imagine aren't an issue in your case. (Putting parentheses around them makes it match the format expected by the GDL communication protocol when sending the game as part of a START request.)

If you want to do a "proper" pull request, you can create a new branch based off upstream-master in gsj601/ggp-hardware and make the change there. (You may need to update upstream-master, as I recently made a wide-reaching whitespace change.) Having multiple branches fulfills most of the use cases of having multiple forked repositories.

If you don't want to try that, I can just reproduce this on my end instead.

[gsj601]

I want to give a proper pull request a try. After splitting ggp-base up into jars, my work isn't really a fork of ggp-base anymore anyways, so, that repo stands to have some changes done to it eventually anyways.

[gsj601]

Is there supposed to be an official way to attach a Pull Request to an Issue, other than a link? It's officially a pull request on ggp-base, though. #95

[AlexLandau]

Yep, you just add a link. Thanks for the fix!