Loathing-Associates-Scripting-Society / philter

Inventory cleanup script for KoLmafia
Other
7 stars 5 forks source link

Philter Manager uses wrong ruleset file name when using multiple accounts #61

Open pastelmind opened 3 years ago

pastelmind commented 3 years ago

Case 1: Original bug report

Excerpt of the original bug report from @Phillammon, over at Discord:

I opened the philter ui on \<Account A>, everything worked fine. After that, opened it on \<Account B>, it was set to use \<Account A>'s files. Then opened it on \<Account C>, it was set to use \<Account B>'s files, despite \<Account B> not having any files

Case 2: Personal anecdote

While attempting to reproduce the report above, I experienced a very similar glitch. Here is what happened:

Note: I have a "main" and "alt" account. Both accounts share a single cleanup ruleset file (OCDdata_shared.txt) across all my accounts.

  1. I started KoLmafia and logged onto my "main" account. After playing 200-300 turns, I opened Philter Manager (localhost:60080) to categorize some items. Then I ran ocd-cleanup.ash to cleanse my inventory.

    • At this point, I received the bug report. The KoLmafia session was still logged in.
  2. To investigate, I launched a second KoLmafia instance and logged into my "alt" account.

    • At this point, my ZLib settings files looked like this:

      // vars_defaults.txt
      BaleOCD_DataFile        string  alt 20210523 11:54:58 KST
      // vars_main.txt
      BaleOCD_DataFile        string  shared 20210523 11:54:58 KST
      // vars_alt.txt
      BaleOCD_DataFile        string  shared 20210523 11:54:58 KST

      I can attest this because I keep track of my settings using Git, and I commit them every day.

  3. Using the second KoLmafia session ("alt" account), I opened Philter Manager (localhost:60081) and changed the ruleset file name from shared to alt. I clicked "Save", then "Copy contents" in the popup. Then I navigated to a different tab (inside Philter Manager) and back to reload the current setting.

    • Result: Philter Manager displayed a value of main for the ruleset file name, instead of alt.
    • At this point, my ZLib settings files looked like this:

      // vars_defaults.txt
      -BaleOCD_DataFile       string  alt 20210523 11:54:58 KST
      +BaleOCD_DataFile       string  alt 20210524 19:04:09 KST
      // vars_main.txt
      BaleOCD_DataFile        string  shared 20210523 11:54:58 KST
      // vars_alt.txt
      -BaleOCD_DataFile       shared

Further investigation

This may be related to #58.

pastelmind commented 3 years ago

Running ocd-cleanup.ash can silently change the default value of BaleOCD_DataFile and BaleOCD_StockFile.

Necessary conditions:

The effect can be directly observed with a ZLib verbosity setting ≥ 4. To change it, enter zlib verbosity = 4 in the gCLI. (Default is 3)

When ocd-cleanup.ash is invoked for the first time (either directly, or when included by our relay backend), it alters the default value of ZLib variables BaleOCD_DataFile and BaleOCD_StockFile:

 > ocd-cleanup.ash
New default value for ZLib string setting: BaleOCD_DataFile => <playername>
New default value for ZLib string setting: BaleOCD_StockFile => <playername>
Updating inventory...

Wait, how does this work?

💡 First, a primer on ZLib variables (click to expand/hide) All ZLib variables have a "default value" and a "custom value". The **default value** is set when the variable is created, usually by scripts like `ocd-cleanup.ash`. They are stored in `data/vars_defaults.txt` and are shared across all accounts. Initially, the **custom value** does not exist. It is set when the user changes the variable to a value _that differs from the default value_. They are stored in `data/vars_.txt` and are per-user. When the user sets the variable to its default value, ZLib is clever enough to "delete" the custom value from `data/vars_.txt`. ZLib provides two functions for managing variables: `getvar()` and `setvar()`. `getvar()` is used to retrieve a variable. It first looks up the custom value. If the custom value does not exist, it then looks up the default value. `setvar()` is used to _set the default value_ of a variable. If the default value has been already set, `setvar()` does nothing--unless the default value has changed, in which case it changes it again.

Here's the bug🐛

ocd-cleanup.ash sets the default value of BaleOCD_DataFile to the current player ID. https://github.com/Loathing-Associates-Scripting-Society/philter/blob/5ee11322cb16f2ecc4a8d93ae92f0f3f1949b534/release/scripts/ocd-cleanup.ash#L10-L11

This is usually not a problem:

Now, what if you had two KoLmafia instances running together?

  1. Launch two instances of KoLmafia. Login as player X in the first, and player Y in the second.
  2. Run ocd-cleanup.ash as player X. This sets the default value of BaleOCD_DataFile to X.
  3. Run ocd-cleanup.ash as player Y. This sets the default value of BaleOCD_DataFile to Y.
  4. Run ocd-cleanup.ash as player X. This does not change the default value of BaleOCD_DataFile. Instead, the script will attempt to use data/OCDdata_Y.txt for player X. Which may or may not exist, but is most likely not what you wanted.

Both Philter Manager and Philter Manager classic import ocd-cleanup.ash each time they are invoked. Because of this, you can unknowingly change the default value before you directly run ocd-cleanup.ash.