Bug - Fixed getPermedSkills() not correctly showing all skills in avatar path

[SketchySolid]

I have a character in AoSP who has several perms, but `js getPermedSkills()` returns only [ Cannelloni Cocoon: false ]. See attached "view source" html from my profile page, which shows I also have Torso Awareness, Advanced Cocktailcrafting, and Super Human Cocktailcrafting permed.

I've experienced similar issues with getPermedSkills() not returning most of my skills with another character while in Grey You. Once the prism is broken and I'm back in aftercore, it works correctly again.

 

[SketchySolid]

Are any other details needed for this issue?

 

[Veracity]

I converted your file into HTML such as we would actually get from KoL, rather than ... a picture of an HTML file?
(In the future, please get a DEBUG log.)

I made a test method in CharSheetRequest - List<ParsedSkillInfo> parseSkills(string responseText) - and added "test charsheet-skills" to call that method with your responseText and display the resulting list.

With your responseText, it reproduces your bug: It shows all your active skills, but not the "permed skills you cannot currently use" at the end.

Somebody rewrote charsheet skill parsing to use xpath, as opposed to regexp matching or whatever we had, last time I looked at it, years ago.

That's rather out of my wheelhouse, but I'll start a PR to work on it. Maybe somebody will help.

 

[Veracity]

I finally looked at this again.

For whatever reason, parsing skills on the char sheet intentionally excludes that are permed but unavailable in current path.

At the time you got this charsheet, you were on day 96 of the run.
Looks like you used your Big Book of Every Skill and have learned a bunch of skills, one of which was previously permed.
All of those skills would normally be unavailable of AoSP, but your book gave them to you.

The other three skills you permed are unavailable in AoSP and you have not yet re-learned them.

    // Skills that you cannot use right now (e.g. due to path restrictions) are wrapped in a <span> tag:
    //
    //    <span id="permskills" ...>...</span>

The xpath expression:

                      "//a[contains(@onclick,'skill') and not(ancestor::*[@id='permskills'])]",

Since we parse the char sheet to find out your currently available skills, it intentionally omits permed-but-not-available skills.

I am honestly not sure what the contract of permed_skills() should be.

1) Include only skills that are currently available to you, accounting for path (or Standard) restrictions
2) Include all skills you have ever permed, even they are not currently available to you.

Since we intentionally do not parse permed-but-not-available skills, you get option 1.
You expected it to be option 2.

What is your use case for option #2?

I suppose we could parse the DOM a second time and select ONLY the permed-but-not-available skills and save those ... somewhere else.

 

[Veracity]

I am honestly not sure what the contract of permed_skills() should be.

1) Include only skills that are currently available to you, accounting for path (or Standard) restrictions
2) Include all skills you have ever permed, even they are not currently available to you.

Since we intentionally do not parse permed-but-not-available skills, you get option 1.
You expected it to be option 2.

What is your use case for option #2?

I realize this PR has been open for almost eight months - and I opened a (draft) PR to investigate it two weeks later - but, here we still are.

Four months ago, I arrived at a point where I had a decision I didn't feel I could answer on my own, since I had no idea what the intended use of the get_permed_skills() function is. I can't think of a script that _I_ would want to write or use that needs it, but not everybody is like me.

When/why did we implement it?

Perhaps a script wants to look at your currently available skills and see which ones are not currently permed. Sort of like what KoLmafia shows you when it decorates the Gash for you - which is in aftercore, by definition. OK. get_permed_skills(), as it currently exists, lets you do that.

But what, exactly, is the use case for a script to know which skills are NOT available to you that you already have permed?
Which is to say, what is the use case for calling permed_skills in-run?

I asked that question four months ago - and have received no response. I guess I should have marked this "Bug - Waiting for Info".

I am tired of seeing a seven-month-old Draft PR. I am going to mark this "Waiting for Info" and either close the PR in a week (if no info is forthcoming) or finish it up - assuming I see a use case for get_permed_skills() called in-run (as opposed to aftercore) including skills you can't currently use.

Unusable permed skills by Veracity0 · Pull Request #1254 · kolmafia/kolmafia

From this bug report: https://kolmafia.us/threads/getpermedskills-not-correctly-showing-all-skills-in-avatar-path.28216/ I munged his supplied charsheet to be usable I created CharSheetRequest List...

github.com

 

[phreddrickk]

I initially created get_permed_skills with the intention of using it to help write a script that automatically perms skills for me when I ascend. While you can always use an xpath on the permery to determine what skills are available to be permed, it's helpful to have access to this information before hopping the gash; I could add a warning if, say, you have an unpermed skill that came from a non-reusable skillbook but insufficient karma to perm it. Another potential use case I considered (but haven't personally pursued) is letting ascension scripts validate in advance whether or not you fulfill their requirements before hopping the gash.

Even for players who choose to remain in aftercore as an avatar class (breaking ronin but not prism), they always have to break prism just before ascending anyways. That being said, if the purpose of get_permed_skills in the above two situations is to let players validate their decisions before doing a potentially-destructive behavior (like going to valhalla), breaking prism could be thought of as potentially-destructive behavior in that situation.

I think in general it's better to present the user with as much information as possible, so I would err on the side of "show all the unuseable permed skills".

 

[SketchySolid]

Sorry for not responding earlier. The use case was for my script planning which class to switch to when breaking the prism after a grey you run, in order to be able to perm a high-value skill that you didn't know yet. You have to make the choice as you break the prism, which is before entering aftercore, which means you don't having access to your perm list before you need that information.

 

[Veracity]

OK. I finished it up and that PR is now ready for review.

 

[Veracity]

Revision r27422.