Bug - Fixed Maximizer suggests equipping a watch in the wrong slot

R

Ryo_Sangnoir

Developer
Staff member
1736611138516.png
My current acc1 here is the Cincho de Mayo, which doesn't offer any +adv or +fites. The Counterclockwise Watch is +3 compared to the baywatch, but it suggests replacing acc1 instead of acc3...

I have reproduced this with a test, which is a rarity for maximizer bugs.

Code: 
    @Test
    void suggestsReplacingExistingWatch() {
      var cleanups =
          new Cleanups(
              withEquippableItem("Counterclockwise Watch"), // 10, watch
              withEquipped(Slot.ACCESSORY1, "Cincho de Mayo"), // 0, not a watch
              withEquipped(Slot.ACCESSORY2, "numberwang"), // 5, not a watch
              withEquipped(Slot.ACCESSORY3, "baywatch") // 7, a watch
              );

      try (cleanups) {
        assertTrue(maximize("adv,fites,-tie"));
        assertEquals(15, modFor(DoubleModifier.ADVENTURES), 0.01);
        assertThat(getBoosts(), not(hasItem(recommendsSlot(Slot.ACCESSORY1))));
        assertThat(getBoosts(), not(hasItem(recommendsSlot(Slot.ACCESSORY2))));
        assertThat(getBoosts(), hasItem(recommendsSlot(Slot.ACCESSORY3, "Counterclockwise Watch")));
      }
    }
 
Last edited:
Ryo_Sangnoir said:
I have reproduced this with a test, which is a rarity for maximizer bugs.
Click to expand...

I added it to https://github.com/kolmafia/kolmafia/pull/2471/files# in hopes that my non-failing maximizer tests might get some more eyeballs.
:-)

The line

assertThat(getBoosts(), not(hasItem(recommendsSlot(Slot.ACCESSORY1))));

generated an error

Code: 
Expected: not a collection containing slot ACCESSORY1
     but: was <[equip acc1 Counterclockwise Watch (+3), <html><font color=gray>keep acc2: numberwang</font></html>, <html><font color=gray>keep acc3: baywatch</font></html>]>

I've seen that in some of my (disabled) tests and never figured it out. Anything I know about hamcrest, I learned by rote. I don't understand why the message is "not a collection containing slot ACCESSORY1" since the returned text is equipping something in acc1. Can you explain it to me, please?
 
We expect getBoosts() to return a collection, and for that collection to not have an item that recommends slot ACCESSORY1.

However, the collection was
Code: 
[<[equip acc1 ...]>]
which does equip something in acc1, so the assertion fails.
 
Ryo_Sangnoir said:
We expect getBoosts() to return a collection, and for that collection to not have an item that recommends slot ACCESSORY1.

However, the collection was
Code: 
[<[equip acc1 ...]>]
which does equip something in acc1, so the assertion fails.
Click to expand...
Thank you.

What I am missing then is how something(?) decides that "equip acc1" is not the same as as recommending something for ACCESSORY1. Is there a (KoLmafia) semantic that I am misunderstanding that says emitting a command to equip something does not imply that the Maximizer recommended it?
 
The line

assertThat(getBoosts(), not(hasItem(recommendsSlot(Slot.ACCESSORY1))));

is a test. When the test fails, it prints what you see: we expected (something), but actually (something else).

In this case, we expect that running the maximizer does not change accessory 1, but in fact maximizer suggests to change accessory 1 to the Counterclockwise Watch.

The test is expected to fail. The reason the test fails is the bug I am reporting in this thread.
 
Ryo_Sangnoir said:
The line

assertThat(getBoosts(), not(hasItem(recommendsSlot(Slot.ACCESSORY1))));

is a test. When the test fails, it prints what you see: we expected (something), but actually (something else).

In this case, we expect that running the maximizer does not change accessory 1, but in fact maximizer suggests to change accessory 1 to the Counterclockwise Watch.

The test is expected to fail. The reason the test fails is the bug I am reporting in this thread.
Click to expand...

OK. I think the thing that was different from what I expected was that (in some sense) the test was "change, or not, acc1". I have seen cases (not necessarily in tests) where the maximizer does change the slot of an accessory even though the score is not effected, so I would tend to write a test that passed if the desired accessory was in any slot. If "-tie" can be understood to mean "don't change a slot unless the score changes" then the test is correct but there may be an opportunity to improve the maximizer documentation to state that more obviously.

I will observe that any maximizer suggestion that removes the baywatch and adds the CCW watch is in some sense correct regardless of slots. So different keywords may require a different test.

Thank you. Straightened me out, given me a few rabbit holes to explore and maybe even the key to why my attempts to write a failing test ended up writing passing tests instead.
 
fronobulax said:
If "-tie" can be understood to mean "don't change a slot unless the score changes"
Click to expand...
This is not a correct interpretation of either the current or the desired behavior.

The maximizer may want to change a slot even if it specifically does not change the score when equipping an outfit which overall will provide a bonus (attributed to some other slot), or to satisfy some condition such as +equip; -tie has no bearing on either of these.

I have a dumb fix that seems to work: just add a MutexI entry for the various watches. (#2664)
 
You must log in or register to reply here.
Back
Top