Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track Powerful Glove's charge from description #1020

Merged
merged 6 commits into from Aug 25, 2022

Conversation

libraryaddict
Copy link
Contributor

@libraryaddict libraryaddict commented Aug 24, 2022

Noticed the charge wasn't being tracked when an issue popped up with "Use the Force" being used in a macro which changed the final output of the resulting page to being a choice, meaning mafia never saw the combat page, meaning mafia never realized I used a powerful glove charge in fight to switch the monster.

Or basically.
If a charge is used without mafia knowing, mafia will never adjust for it. Feels strange this wasn't tracked previously.

Tracking when it is fully charged feels meaningless as it should only be inaccurate if someone was messing with the preference, but included for the full coverage.

@libraryaddict libraryaddict requested a review from a team as a code owner August 24, 2022 18:51
@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #1020 (d3a0139) into main (24b1222) will increase coverage by 0.01%.
The diff coverage is 85.71%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1020      +/-   ##
============================================
+ Coverage     28.50%   28.51%   +0.01%     
- Complexity    13580    13588       +8     
============================================
  Files          1031     1031              
  Lines        160579   160598      +19     
  Branches      35281    35285       +4     
============================================
+ Hits          45774    45800      +26     
+ Misses       106369   106360       -9     
- Partials       8436     8438       +2     
Impacted Files Coverage Δ
...net/sourceforge/kolmafia/request/FightRequest.java 30.35% <66.66%> (+0.01%) ⬆️
...urceforge/kolmafia/session/ConsequenceManager.java 64.16% <81.81%> (+5.51%) ⬆️
...sourceforge/kolmafia/persistence/ItemDatabase.java 43.61% <100.00%> (+0.27%) ⬆️
...urceforge/kolmafia/session/ResponseTextParser.java 22.32% <100.00%> (+0.29%) ⬆️
...sourceforge/kolmafia/textui/langserver/Script.java 81.91% <0.00%> (-3.20%) ⬇️
...forge/kolmafia/persistence/ConcoctionDatabase.java 46.85% <0.00%> (-0.06%) ⬇️
src/net/sourceforge/kolmafia/Modifiers.java 63.11% <0.00%> (+0.05%) ⬆️
...sourceforge/kolmafia/session/EquipmentManager.java 34.64% <0.00%> (+0.41%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24b1222...d3a0139. Read the comment docs.

@Veracity0
Copy link
Contributor

You could make this even better.

Consider this:

When you enter a fight, unless your macro is executed as an autoattack, you will be shown an initial fight page, complete with the "skills" dropdown.

adventure.php?snarfblat=149
302 Field: location = [fight.php?ireallymeanit=1661372296]
fight.php?ireallymeanit=1661372296

<option value="4012" picurl="saucegeyser" >Saucegeyser (24 Mojo Points)</option>
<option value="7327" picurl="shrinkenemy" >CHEAT CODE: Shrink Enemy (5 of today's remaining 100%)</option>
<option value="7326" picurl="replaceenemy" >CHEAT CODE: Replace Enemy (10 of today's remaining 100%)</option>

FightRequest.parseAvailableCombatSkills(String responseText):

    Matcher m =
        FightRequest.AVAILABLE_COMBATSKILL_PATTERN.matcher(
            responseText.substring(startIndex, endIndex));
    while (m.find()) {
      int skillId = StringUtilities.parseInt(m.group(1));
      String skillName = SkillDatabase.getSkillName(skillId);
      if (skillName == null) {
        skillName = m.group(2);
        SkillDatabase.registerSkill(skillId, skillName);
      }

The pattern:

  private static final Pattern AVAILABLE_COMBATSKILL_PATTERN =
      Pattern.compile("<option[^>]*?value=\"(\\d+)[^>]*?>(.*?) \\((\\d+)[^<]*</option>");

notice that:

group(1) -> the skillId
group(2) -> the skillName
group(3) -> the number after the "(" - which is usually the MP cost.
That last group is never used.

I think you could capture the entire phrase inside the () and determine right from there the charge on your powerful glove.

Here are all the ones on MY skill dropdown that are not simply (XX Mojo Points):

<option value="188" picurl="light" >Implode Universe (13 times today, 50 MP)</option>
<option value="19" picurl="snout" >Transcendent Olfaction (3 uses left today)</option>
<option value="7327" picurl="shrinkenemy" >CHEAT CODE: Shrink Enemy (5 of today's remaining 100%)</option>
<option value="7326" picurl="replaceenemy" >CHEAT CODE: Replace Enemy (10 of today's remaining 100%)</option>
<option value="7310" picurl="bat" >Become a Bat (10 time(s) remaining today))</option>
<option value="7309" picurl="puff" >Become a Cloud of Mist (10 time(s) remaining today))</option>
<option value="7308" picurl="wolfmask" >Become a Wolf (10 time(s) remaining today))</option>

@libraryaddict
Copy link
Contributor Author

Don't really plan to add more skill parsing to this PR, added cosplay only cos it occured to me. Probably should just share the same html file tbh

@libraryaddict
Copy link
Contributor Author

Although it occurs to me that if we were going to track all this stuff, could add it into Consequences as something like COMBAT_SKILL given how many different formats there are for counters in skill names.

@Veracity0
Copy link
Contributor

Interesting. Notice FightRequest.parseAvailableCombatSkills:

    int startIndex = responseText.indexOf("<select name=whichskill>");
    if (startIndex == -1) {
      return;
    }
    int endIndex = responseText.indexOf("</select>", startIndex);
    if (endIndex == -1) {
      return;
    }

    Matcher m =
        FightRequest.AVAILABLE_COMBATSKILL_PATTERN.matcher(
            responseText.substring(startIndex, endIndex));

Notice that responseText.substring(startIndex, endIndex)) is just the Dropdown with the skills.

If we have ConsequenceManager do COMBAT_SKILLS, it is exactly this spot in FightRequest that would call it, using exactly that substring of the responseText.

Copy link
Contributor

@Veracity0 Veracity0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!

@Veracity0
Copy link
Contributor

Veracity0 commented Aug 25, 2022

Noticed the charge wasn't being tracked when an issue popped up with "Use the Force" being used in a macro which changed the final output of the resulting page to being a choice, meaning mafia never saw the combat page, meaning mafia never realized I used a powerful glove charge in fight to switch the monster.

I'm curious about this. Did KoL actually show the output of the previous rounds, but because Use The Force redirected to a choice, we didn't process that output?

What did the session log say?

I can try this out myself with debug log on and look at the requests submitted and the responses - and redirects - received. That might very well be a bug we need to deal with in another way.

But fixing that does not invalidate anything about this PR.

Should we open a new Bug to discuss it?
And how about receiving a redirect after ascending? I have thoughts about that, too. Another Bug?

Having conversations in the comments of approved/merged PRs doesn't seem the right way proceed. 😀

@libraryaddict
Copy link
Contributor Author

Noticed the charge wasn't being tracked when an issue popped up with "Use the Force" being used in a macro which changed the final output of the resulting page to being a choice, meaning mafia never saw the combat page, meaning mafia never realized I used a powerful glove charge in fight to switch the monster.

I'm curious about this. Did KoL actually show the output of the previous rounds, but because Use The Force redirected to a choice, we didn't process that output?

What did the session log say?

I can try this out myself with debug log on and look at the requests submitted and the responses - and redirects - received. That might very well be a bug we need to deal with in another way.

But fixing that does not invalidate anything about this PR.

Should we open a new Bug to discuss it? And how about receiving a redirect after ascending? I have thoughts about that, too. Another Bug?

Having conversations in the comments of approved/merged PRs doesn't seem the right way proceed. 😀

The TL;DR

I executed a macro with replace then use the force. I had debug logs turned on for that test, no combat logs were returned. Just a redirect to saber. I'm not sure there's much you can do.

@Veracity0 Veracity0 merged commit 8af45d8 into kolmafia:main Aug 25, 2022
@libraryaddict libraryaddict deleted the track_glove branch August 29, 2022 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants