Results 1 to 9 of 9

Thread: [Patch] Undocumented Tonic Djinn Daily Variable Not Updating

  1. #1
    Senior Member
    Join Date
    Apr 2018
    Posts
    133

    Post [Patch] Undocumented Tonic Djinn Daily Variable Not Updating

    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.

    tonicFix.patch

  2. #2
    Developer fronobulax's Avatar
    Join Date
    Feb 2009
    Location
    Central Virginia, USA
    Posts
    4,159

    Default

    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.

  3. #3
    Senior Member
    Join Date
    Apr 2018
    Posts
    133

    Default

    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.

  4. #4
    Senior Member
    Join Date
    Apr 2018
    Posts
    133

    Default

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

  5. #5
    Developer fronobulax's Avatar
    Join Date
    Feb 2009
    Location
    Central Virginia, USA
    Posts
    4,159

    Default

    Unfortunately, the patch appears to have been unsuccessful. I’m not sure why it still won’t update.
    Originally Posted by Saklad5 View Post
    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?
    You just vehemently agreed with me
    Originally Posted by Veracity View Post
    I agree with frono.
    Originally Posted by Veracity View Post

  6. #6
    Senior Member
    Join Date
    Apr 2018
    Posts
    133

    Default

    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.

  7. #7
    Developer
    Join Date
    Apr 2010
    Posts
    4,827

    Default

    Trying to commit a fix.

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

  8. #8
    Developer
    Join Date
    Apr 2010
    Posts
    4,827

    Default

    r18590

  9. #9
    Developer fronobulax's Avatar
    Join Date
    Feb 2009
    Location
    Central Virginia, USA
    Posts
    4,159

    Default

    r18590
    Originally Posted by Darzil View Post
    Thank you. I got distracted...
    You just vehemently agreed with me
    Originally Posted by Veracity View Post
    I agree with frono.
    Originally Posted by Veracity View Post

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •