Bug - Fixed Parse charsheet.php for skill IDs, not names

[philmasterplus]

As of writing, when KoLmafia parses charsheet.php, it extracts each skill by name. This causes problems when two different skills have the same name. For example, skill ID 11002 ("Ferocity" in Avatar of Boris) and 24017 ("Ferocity" in Dark Gyffte). If a character is in AoB, KoLmafia thinks that the character has the DG Ferocity instead of AoB Ferocity. We could add special checks for these skills, but as skills are continuously added to the game, such approach is simply not scalable.

This patch eliminates the problem by parsing skill IDs instead of names. There is also a fallback for name checks in case KoLmafia doesn't recognize a skill ID.

I tested this on r20539 against 3 different characters, two of which were in Avatar of Boris. Both characters had regular permed skills that they couldn't use due to being AoB; the patched code correctly ignored these skills.

Incidentally, I tried to use JDOM (org.jdom2.*) for XPath since KoLmafia already includes JDOM. However, JDOM relies on Jaxen for XPath. Since KoLmafia doesn't bundle Jaxen, I had to resort to Java's built-in XPath packages (javax.xml.xpath.*) instead. If we can add Jaxen, please let me know, because Java's built-in DOM packages (org.w3c.dom.*) are cumbersome to use. Besides, we'd finally get a chance to use proper XPath in ASH.

 

[fronobulax]

I understand you to say that mafia's XPath support comes from javax.xml.xpath and you are asking that the existing support be deprecated or removed and replaced with Jaxen.

If I have this right how about breaking out your feature request as a FR in a separate post?

Note that replying here to agree with me or correct me, kind of misses the point

I'll look at the ID patch but it might not get done today so of someone else gets to it, fine.

 

[philmasterplus]

I understand you to say that mafia's XPath support comes from javax.xml.xpath and you are asking that the existing support be deprecated or removed and replaced with Jaxen.

If I have this right how about breaking out your feature request as a FR in a separate post?

Note that replying here to agree with me or correct me, kind of misses the point

I'll look at the ID patch but it might not get done today so of someone else gets to it, fine.

Actually, KoLmafia uses HtmlCleaner's half-baked implementation of XPath for the ASH xpath() function. KoLmafia bundles JDOM (lib/jar/jdom-2.0.6.jar) only because HtmlCleaner depends on JDOM, and we need HtmlCleaner to sanitize bad HTML.

It seems that JDOM can offer proper XPath capabilities, but only if Jaxen is also bundled. Therefore I am suggesting that we bundle Jaxen as well.

If we bundle JDOM + Jaxen, I see two benefits:

1. We can use proper XPath inside KoLmafia itself without the hassle of using org.w3c.dom. This is the big reason.
2. We can "fix" the ASH xpath() function. This is a secondary reason.

 

[fronobulax]

Actually, KoLmafia uses HtmlCleaner's half-baked implementation of XPath for the ASH xpath() function. KoLmafia bundles JDOM (lib/jar/jdom-2.0.6.jar) only because HtmlCleaner depends on JDOM, and we need HtmlCleaner to sanitize bad HTML.

It seems that JDOM can offer proper XPath capabilities, but only if Jaxen is also bundled. Therefore I am suggesting that we bundle Jaxen as well.

If we bundle JDOM + Jaxen, I see two benefits:

1. We can use proper XPath inside KoLmafia itself without the hassle of using org.w3c.dom. This is the big reason.
2. We can "fix" the ASH xpath() function. This is a secondary reason.

Note that replying here to agree with me or correct me, kind of misses the point

 

[gausie]

I've always found attempting to read KoL's horrendous HTML with xpath to be frustratingly unreliable. Would Jaxen be capable?

In any case, your patch looks and runs great as far as I can see, and trusting your extended testing it is merged in r20541

 

[philmasterplus]

I assume so, but I have never used Jaxen myself. It would be nice if someone with more expertise (and authority to add JARs) is willing to test it, but I'll take a look.

 

[MCroft]

I bumped last month's thread for a related xpath issue. To Frono's point, we should discuss there.