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

Don't send empty macros to KoL #410

Merged
merged 1 commit into from
Dec 26, 2021
Merged

Conversation

buttercookie42
Copy link
Contributor

@buttercookie42 buttercookie42 commented Dec 23, 2021

Sometimes Kolmafia might generate a combat macro which, after optimisation,
constists solely of the placeholders

#mafiaround
#mafiamp
#mafiaheader

handleMacroAction() then further optimises that kind of macro down into a bunch
of newlines, which then cause an "Invalid macro" error when sent to KoL.

With this change, we also check that the macro isn't empty after the pro-
cessing done by handleMacroAction(), and only send it on to KoL if it's truly
non-empty.

See also https://kolmafia.us/threads/consult-script-ccs-that-gets-optimised-out-to-nothing-leads-to-invalid-macro-abort.26620/

Copy link
Contributor

@gausie gausie left a comment

Choose a reason for hiding this comment

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

Do you think it would be wise to log a message here saying that we're choosing not to submit a blank macro? When does this tend to happen.

@buttercookie42
Copy link
Contributor Author

Yes, probably not a bad idea, will add a log message.

Like I explained in the forum thread, at least for me this seems to happen when I have a custom combat script consisting of a single conditional action (like olfaction or the generic "special action") plus a consult script. When the conditional action doesn't take place (e.g. in the case of olfaction because I've already olfacted something, so Kolmafia seems to directly skip generating an olfaction action in the macro, or in the case of the "special action" because there's no special action scheduled in that round) Kolmafia seems to generate this sort of

#mafiaround
#mafiamp
#mafiaheader

de facto (but not technically) empty macro, which the MACRO_COMPACT_PATTERN replace step then turns into a bunch of newlines. This still used to work last year, but it no longer does now, so probably something changed on the KoL-side, because I think I also tried a build from last year and this time it no longer worked, either.

Sometimes Kolmafia might generate a combat macro which, after optimisation,
constists solely of the placeholders

#mafiaround
#mafiamp
#mafiaheader

handleMacroAction() then further optimises that kind of macro down into a bunch
of newlines, which then cause an "Invalid macro" error when sent to KoL.

With this change, we also check that the macro isn't empty *after* the pro-
cessing done by handleMacroAction(), and only send it on to KoL if it's truly
non-empty.
@gausie
Copy link
Contributor

gausie commented Dec 24, 2021

Yes, probably not a bad idea, will add a log message.

Like I explained in the forum thread, at least for me this seems to happen when I have a custom combat script consisting of a single conditional action (like olfaction or the generic "special action") plus a consult script. When the conditional action doesn't take place (e.g. in the case of olfaction because I've already olfacted something, so Kolmafia seems to directly skip generating an olfaction action in the macro, or in the case of the "special action" because there's no special action scheduled in that round) Kolmafia seems to generate this sort of

#mafiaround #mafiamp #mafiaheader

de facto (but not technically) empty macro, which the MACRO_COMPACT_PATTERN replace step then turns into a bunch of newlines. This still used to work last year, but it no longer does now, so probably something changed on the KoL-side, because I think I also tried a build from last year and this time it no longer worked, either.

Right I see, so it sends that macro then calls your consult script? I wonder if a message will be annoying then. We shall see, and can always remove it.

Copy link
Contributor

@gausie gausie left a comment

Choose a reason for hiding this comment

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

Using the debug log is smart, thanks.

@gausie
Copy link
Contributor

gausie commented Dec 24, 2021

Would like a test for this if there's an obvious way to do so. It looks like the closest public caller of this method is FightRequest.runOnce

@buttercookie42
Copy link
Contributor Author

Makes sense, though I might need a little bit to figure out your testing framework and how a test could look like.

@gausie gausie merged commit 573bdd4 into kolmafia:main Dec 26, 2021
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