loathers / autoscend

An ascension script for KoLMafia
Other
45 stars 65 forks source link

Autoscend tries to take CCSC option in Protestors in WotSF #1432

Closed VuvuzelaVirtuoso closed 2 months ago

VuvuzelaVirtuoso commented 3 months ago

Context

Expected/Desired Behavior

It would be ideal for Candy Cane Sword Cane choice adventure options to not be taken (even if they have not been used yet) if the CCSC is not able to be equipped.

Current Behavior

When owning Candy Cane Sword Cane in WotSF path and trying to do Protestors in L11, in spite of being unable to equip the CCSC autoscend will attempt to choose choice 2 in the "Bench Warrant" choice adventure (choiceAdventure857, in the "Mob of Zeppelin Protestors" zone) and abort after it cannot take it. (I do not have a Disembodied Hand to know if it would equip the CCSC to that)

Failure Information (for bugs)

https://github.com/loathers/autoscend/commit/01b057438d2712e962c4d547fe9c80a72067bd83 Looking at lines 2441-2448 here, it seems like all the other choice option evaluation statements involving CCSC have logic to check for if "available_choice_options" contain the CCSC option, but the protestor portion of L11_redZeppelin was written differently to just choose option 2 by default if you just have the CCSC at all.

Failure Logs

Please include any relevant log snippets or files here. You should still attach the full KoLMafia session log if possible. CandyCaneSworderror vuvuzela_virtuoso_20240322.txt

VuvuzelaVirtuoso commented 3 months ago

Also looking at lines 1787-1790 and 1812 in that commit https://github.com/loathers/autoscend/commit/01b057438d2712e962c4d547fe9c80a72067bd83 the logic for HiddenApartment also should have the same problems for paths that can't equip CCSC because it just references auto_haveCCSC() before determining that it should start trying to force elevator if you have Twice-Cursed instead of Thrice-Cursed.

Looks like I just didn't run into an abort with that in my run because it chose not to try to use an NC force to hit elevator and I hit a shaman before I hit the elevator.

VuvuzelaVirtuoso commented 3 months ago

Not entirely sure what the best way around this would be--for the choiceAdventure857 stuff, not really sure if the appropriate place in the code for this would just be in place of the existing if[auto_haveCCSC()) block there, but I imagine something like this probably would help, since autoscend doesn't necessarily need to know if have the CCSC option is still available before taking that option.

if(choice == 857) // Bench Warrant (Mob of Zeppelin Protestors)
{
     if(available_choice_options() contains 2) // only available with the Candy Cane Sword Cane equipped
     {
             run_choice(2); // double protestor removal once per ascension
     }
     else
     {
            run_choice(1);
     }
}

I would probably just PR the above, but I'm not sure how to accordingly adjust the logic behind the calculations autoscend makes for protestor count that currently do make an assessment solely on if you have CCSC. https://github.com/loathers/autoscend/commit/01b057438d2712e962c4d547fe9c80a72067bd83#diff-181b7911a36ad06813c83d4cc1f763b76496160996ef512beb1cecc3f5006120 ...and decent chance I'm misreading this but the calcs on 2516-2519 of level_11.ash in the CCSC commit read to me like if you have CCSC, autoscend currently makes its decision on which protestor NC option to take based on calculations that assume you're get double value out of removing Sleaze Protestors on each visit to Bench Warrant no matter what, rather than just the first visit with CCSC?

And yeah, no clue what to do for the Hidden_Apartment cursesNeeded thing, since I'm guessing the reason the logic is built differently there is because autoscend is actually making a decision on whether or not to try to force the adventure based off assuming availability of the CCSC option, rather than trying for the adventure and the CCSC option being available just being a bonus/the optimal play no matter what. I guess there's always the option to hard code in certain paths following different logic, as I've seen elsewhere in the script? Which, even if not ideal for long-term solvency might just be the more-reasonable option to implement effort-wise? /shrug

Donald-Lafleur commented 3 months ago

I am thinking that the cleanest solution long term is to just change how auto_haveCCSC() behaves by changing it to check for whether the CCSC can be equipped instead of just that it's valid. I'm pretty sure this would solve all the CCSC related issues for WotSF and probably any other path with equipment restrictions like AoB.

I'm hesitant to submit that PR though, because that's not what the function name implies it does. On the other hand, we probably only ever care whether we both have it and can equip it, since it's only use is being carried into NCs to unlock new options.

I tried to find a clear precedent for how auto_haveX functions for weapons/offhands behave. The only similar functions I found were for Extingo and CMG and they behave in opposite ways with auto_haveFireExtinguisher() not checking whether it can be equipped.

boolean auto_haveFireExtinguisher()
{
    item exting = wrap_item($item[industrial fire extinguisher]);
    return possessEquipment(exting) && auto_is_valid(exting);
}
boolean auto_haveCursedMagnifyingGlass()
{
    if (possessEquipment($item[cursed magnifying glass]) && auto_can_equip($item[cursed magnifying glass])) {
        return true;
    }
    return false;
}

Maybe editing it to use auto_can_equip and changing the name of the function to auto_canUseCCSC() to make it match the June Cleaver function would be best, since that's how every call-site uses it already. I'll ascend into WotSF today and try it out.