Feature - Implemented Increasing turns of hobo songs in the relay browser doesn't consider records

[lostcalpolydude]

The maximizer uses those commands to figure out how to get effects. There's stuff to see a "use" command and check if you have the item, and other stuff to check if you have a skill for "cast" commands. There's nothing for "ashq", and it doesn't seem trivial to add that filtering.

 

[Veracity]

Yup. I don't like this one bit.

 

[Catch-22]

Okay, so it's still broken with the new patch, just not as broken as it was before, right?

I'll look into it, everybody remain calm.

 

[fronobulax]

Okay, so it's still broken with the new patch, just not as broken as it was before, right?

I'll trust that assessment enough to not revert it immediately.

 

[Veracity]

It was not broken before. It just didn't have the "feature" you were asking for. On the other hand, with this patch, the modifier maximizer is now broken in a way it was not before.

I consider that to be a step backwards.

 

[Catch-22]

Okay, so I looked into it. There was definitely a typo for "Antsy in your Pantsy" (I did warn it was untested). I've attached the fix for that.

Am I missing something regarding the modifier maximizer issue? We have gone from something that didn't work at all before (just showed a note/comment telling you what you should do) to something that now works, semi-intuitively (the use() function will still try acquire the item if it can, and KoLmafia checks if you have a skill before casting it with use_skill() too).

Unless I am missing something, the only downsides I can see to using ashq are it doesn't look as nice and if you don't have the skill it won't tell you to "learn to cast it, or get it from a buffbot". Even if that is an issue, it seems like a fairly small one, and one that only affects 6 effects that show up in the modifier maximizer at that.

lostcalpolydude, is it possible you happened to try this with Antsy in Your Pantsy, which is currently bugged?

If the only issue is with the "look and feel" of the handful of effects that do show up this way in the modifier maximizer, I'd be happy to take a closer look at smoothing things over in the MaximizerFrame, but I don't think it's worth holding back the added functionality that this patch brings to the table in the mean time.

Edit:

It was not broken before. It just didn't have the "feature" you were asking for. On the other hand, with this patch, the modifier maximizer is now broken in a way it was not before.

Veracity, I got sidetracked in the middle of writing this post and hadn't noticed your reply. This isn't a feature I was asking for, I don't even use recordings at all. Are you able to tell me which part is specifically broken so I can look into fixing it? Thanks.

 

[lostcalpolydude]

lostcalpolydude, is it possible you happened to try this with Antsy in Your Pantsy, which is currently bugged?

All of the "use either" commands work fine. I see that Antsy had a typo before, and that the fix makes it functional. It doesn't remove it from the maximizer listing when you don't have access to those items.

 

[Catch-22]

All of the "use either" commands work fine. I see that Antsy had a typo before, and that the fix makes it functional. It doesn't remove it from the maximizer listing when you don't have access to those items.

Okay, I'll work on a fix for that. If this were done though, things could be made to look easier on the eyes in the maximizer frame and writing code for if ( cmd.startsWith( "either" ) ) would also be a simple task.

Could I get your thoughts on that? I specifically wrote that FReq after doing the ashq stuff in the effects database because I knew native CLI functionality would be better suited for the task.

 

[Theraze]

Two problems with the current "ashq" hack. One, it displays those items/effects as available even during a hardcore run, when neither of the effects is actually available. Two, unlike the standard implemented entries, there's no meat cost given, which tells the user that they already have this available in their inventory and all they have to do is use it... which probably isn't true. Just noticed both of these when I was trying to maximize moxie to simplify the current run a bit.

 

[lostcalpolydude]

One option is to just remove any ashq entries from the maximizer. I have a feeling people would be upset about that approach though.

I'm not planning to work on an "either" command.

 

[roippi]

My preference would be to drop the ashq stuff and to work on a proper CLI command. Whatever we go with, it eventually needs to end up in the same format as other equivalencies:

use 1 Angry Farmer candy (+20)
... or use 1 Tasty Fun Good rice candy (+20)

(and, of course, only be displayed when appropriate)

 

[Theraze]

Personally, I'd rather not have it displaying the ashq entries as possible when they aren't. They currently constitute a bug. If a new solution that solves this isn't immediately forthcoming, the easiest way around that is to roll back the change that makes it try to automatically switch.

New features good. New bugs bad. New features that add bugs that aggravate Veracity, very bad.

 

[Catch-22]

I'm not planning to work on an "either" command.

I'm just asking for feedback. I'd be happy to take the task on to work on "either", then use "either" for the CLI action. The only reason I haven't fixed what Theraze reported already is because I don't want to do the work twice.

So first of all, if "either" were a command, do you think that would be the appropriate command to use in these cases? If so, can you please put some feedback on the "either" thread saying that you would find the command useful, then I can work on implementing an "either" command and use that to patch this. Otherwise say "no, the either command is silly we can just use ashq commands", and I will patch the bugs Theraze reported as they are.

The thread for "either" is here. If you have an opinion on the matter, please post it so I have something to go by.

 

[roippi]

Going to make a command decision and revert the ashq lines from statuseffects.txt.

It appears that MaximizerFrame DOES know how to properly interpret the pipes - when I do "maximize familiar weight" it properly tells me that I should use a recording of chorale (since I don't have it permed). So the pipes are not just placeholders, but proper syntax that the maximizer frame understands.

Digging a little bit, it appears that EffectDatabase also knows how to handle pipes:

    public static final String getDefaultAction( final String effectName )
    {
        String rv = StringUtilities.getDisplayName( EffectDatabase.defaultActions.get( StringUtilities.getCanonicalName( effectName ) ) );
        if ( rv == null )
        {
            return null;
        }
        if ( rv.startsWith( "#" ) )
        {    // Callers of this API expect an actual command, not a note.
            return null;
        }
        return rv.split( "\\|" )[0];
    }

.. and there's another function, getAllActions, that returns an iterator which deals with pipes and the "either" keyword.

So instead of doing the ashq hack, we need to try all of the actions instead of just the default action when extending effects.

 

[Winterbay]

Another problem (almost completely different) is the maximizer suggesting casting the song even when I'm lower level than 15 or not an AT.

 

[Catch-22]

Well I'm glad you at least only reverted the ashq commands and not everything.

I knew the pipes were causing the commands to be separated by the maximizer but the CLI doesn't know how to interpret them. ashq is a valid command for the CLI but the maximizer doesn't have special handling for an ashq command like it does the others, patching an ashq command interpreter would be possible, but I suppose separating it at the extend effect level would also be possible.

I can look into that.

 

[Theraze]

Yeah... the big thing about trying to add ashq handling is that it NEEDS to know if it will be successful or not BEFORE it begins... it can't be an

ashq if (!cast) use($item[recording])

because we can't know if the item we've told it to use is actually available to us. Also, it needs to have the associated meat cost if we need to acquire the item first...

 

[Veracity]

I do not like "ashq" in statuseffects. As Theraze says, I want to know, without executing some random ash statement, whether it is possible - and if so, how - to boost the effect. I have always considered "ashq" in statuseffects.txt to be the wrong solution. I have not hidden or suppressed my opinion on that.

Please do not spend time trying to make the maximizer "support" effects whose command is "ashq". Come up with another solution.

Thanks.

 

[Catch-22]

Come up with another solution.

Here's the new solution.

This will play nice with the maximizer frame (ie. current behaviour is maintained). Given that there's currently no "either" command and nobody has expressed any interest in there being one and after extensive messing around with CLI logic flow stuff, I came to the conclusion that ASH* is really the only way to execute actions in an "if this action fails, do the next one" fashion.

This patch really does two things:

1. It updates the "up" CLI command, which did not fully support effects that can be acquired through one of multiple actions (ie. actions that are piped in the effects database).

2. It changes the extend effect command in the relay browser to use "up" instead of using the default lose_effect action in the effects database (which won't work for piped actions).

*The changes to "up" still use an ASH hack, but it works in a slightly different way. For each piped action in the default lose_effect action field it will do a cli_execute() of that action. So for an effect like "Antsy in your Pantsy" where the default lose_effect action is "use 1 sugar-coated pine cone|eat 1 maple syrup". The up command will read this as ashq (cli_execute("use 1 sugar-coated pine cone")||cli_execute("eat 1 maple syrup")).

Yes, I'm aware this is a hack but it's a hack that works and doesn't break anything else in the process. Note that the ASH hack only kicks in when there's a | in the lose_effect action. Currently this applies to the hobo songs, Gothy and Antsy in your Pantsy.

 

[slyz]

This does feel like a hack, but CLI simply isn't suited to do what we want here. ASH on the other hand has everything we need: functions with return values.

The type of "ASH hack" we should use here is probably worth talking about, but I think one is needed anyway.