Bug - Fixed [Patch] Undocumented Tonic Djinn Daily Variable Not Updating

Saklad5

Member
While scripting, I’ve noticed that there is a Daily Variable (_tonicDjinn) set up to track whether the player has used a Tonic Djinn that day. However, it always remains false, making it completely useless.

I searched through the source code, and I quickly found out why.

When a Tonic Djinn is used, the response text is parsed for certain strings:

  1. “put the bottle away” is checked first, and finishes the function call. This is presumably meant to represent the player backing out of the use by choosing “Let me think about it.”.
  2. If the function continues, _tonicDjinn is set to true. This is supposed to mean that either the player used the Tonic Djinn or they were unable to.
  3. Finally, if the string “already had a wish today” is present, the function finishes.

There is a major problem with this: the string “put the bottle away” is present both when cancelling and when trying to use it a second time. This means that players who have already used one outside of KoLmafia will not have _tonicDjinn set to true, as the code acts like they could use it but cancelled. Because of the order of the checks, that means _tonicDjinn will always remain false.

I solved this in the attached SVN patch by swapping the order of the checks. In the revised code, the “already had a wish today” string is checked first. If present, it sets _tonicDjinn to true then finishes. This effectively updates the variable if a Tonic Djinn was used outside of KoLmafia. Next, it checks for the string “put the bottle away”. This definitely means the player cancelled, since they otherwise wouldn’t get that far in the function. If that is true, that returns without changing the variable. Finally, _tonicDjinn is set to true and breaks. That should only run if the player successfully used a Tonic Djinn.

If this isn’t the right place to submit SVN patches, please tell me where to do so. I spent quite a while looking for the right thread.

View attachment tonicFix.patch
 

fronobulax

Developer
Staff member
r18559

I only inspected it, not but did not test. It looked good to me so....

In general there is no particular place for proposed patches. The procedure is to post it somewhere and hope a dev will take an interest, review and commit it.

In the case of this particular patch the level of detail in the commentary contributed to my willingness to review it.
 

Saklad5

Member
If there’s one thing KoLmafia needs, it’s detailed documentation. I might as well try to provide that for my own patches.

The primary source of documentation is a wiki, so I’m probably going to try adding undocumented Daily Variables like this when I get the time. I only found this by searching through the relevant file to see if it existed.
 

fronobulax

Developer
Staff member
Unfortunately, the patch appears to have been unsuccessful. I’m not sure why it still won’t update.

So you are saying the patch did not do what it was supposed to do?

Shall I revert it or give you a couple days to figure out how to correct it?
 

Saklad5

Member
I suppose you should revert it. I don’t see how the glitch can still be the case, since I was fairly sure I had figured out what the root cause was. Is it working for you?

The problem with testing this is that I can only do so once every 24 hours.
 

Darzil

Developer
Trying to commit a fix.

Issue was that using the Tonic redirects to a choice adventure, so you have to handle it in ChoiceManager.
 
Top