This is on one of several bug reports that exist because, in my opinion, we are trying to make "lastLocation" server two different roles. This note discusses the proposed refactoring.In my opinion the graphical widget should reflect the state of some internal variable, rather than the other way around. Sounds like some refactoring is in order.
Seems to me, we have two locations of interest:
lastAdventure - the location where you last spent a turn
nextAdventure - the location where you are about to spend a turn.
You can see lastAdventure in the charpane and in api.php?what=status:
"lastadv":{"id":"365","name":"The Spirit World","link":"adventure.php?snarfblat=365","container":"place.php?whichplace=forestvillage"}
"lastadv":{"id":"186","name":"The Briny Deeps","link":"adventure.php?snarfblat=186","container":"place.php?whichplace=thesea"}
"lastadv":{"id":"190","name":"An Octopus's Garden","link":"adventure.php?snarfblat=190","container":"seafloor.php"},
"lastadv":{"id":"195","name":"The Marinara Trench","link":"adventure.php?snarfblat=195","container":"seafloor.php"}
"lastadv":{"id":"207","name":"Mer-kin Elementary School","link":"adventure.php?snarfblat=207","container":"sea_merkin.php"}
"lastadv":{"id":"339","name":"Dreadsylvanian Village","link":"adventure.php?snarfblat=339","container":"clan_dreadsylvania.php"}
"lastadv":{"id":"316","name":"The Gourd!","link":"adventure.php?snarfblat=316","container":"place.php?whichplace=junggate_2"}
(Note that what KoL calls "zones" and "containers", KoLmafia calls "locations" and "zones". KoL "containers" do not map one-to-one with KoLmafia "zones"; we have a zone called "The Sea", whereas we can see at least three KoL "containers", all of whom contains KoL "zones" which are in "The Sea", as far as item enchantments are concerned.)
Used to be, we had our own location names, and we didn't bother looking at the two places that KoL displayed this, since our name and KoL's name didn't necessarily match. We've gone to a lot of trouble to make them match, and are probably 99% there, but we still don't actually save what we find there. We do look for new locations there and temporarily add them. Instead, we have a setting - lastAdventure - tells us what the last location was that we watched you adventure in.
That setting determines the "zone" and "location" that are applied to modifiers and is used to set the AdventureFrame in the GUI to track where you are currently adventuring.
How do we set lastAdventure?
- KoLAdventure.setNextLocation
-- from GenericRequest.invokeCounterScript
-- from DreadsylvaniaRequest when going straight to a noncombat
-- from KoLAdventure.run()
- ASH set_location
- GenericRequest.checkItemRedirection when using an item which will redirect to a fight
- UseItemRequest when using an item which we know uses adventures (is that redundant?)
- RelayRequest.sendWarnings when we detect you are about to "adventure"
- UseItemRequest when doing the "Welcome Back" adventure
- VioletFogManager when you are diverted by an astral mushroom
Remember that I said that we conceptually have two locations of interest? Unfortunately, we are trying to make do with one. We are missing nextAdventure. I think adding that will rationalize a lot of stuff, but it is an incompatible change: both beforeBattleScripts and counterScripts are invoked when we are about to adventure somewhere and in both cases, we set "lastAdventure" to where we are about to go.
Let's consider how I'd like it to work.
There will be two settings:
lastAdventure - the location where you last spent a turn
- When you log in, we look at api.php and set lastAdventure to what we see there - including registering an unknown adventure location. We could look at the "container" value and decide what the KoLmafia "zone" is.
- When we submit a URL to adventure somewhere - either via automation, via visit_url, or in the Relay Browser - we figure out which "adventure" this is and set lastAdventure to remember that.
- When we detect that we have been diverted - astral mushroom, Teleportitis (can't do this yet, unfortunately) - set lastAdventure to the real place we visited.
*** when we set lastAdventure, we update the Adventure Location in the GUI so you can see where you just went.
nextAdventure - the location where you are about to spend a turn.
- when we set lastAdventure, as a above, set nextAdventure to match.
- when you change the selected location in the GUI, set nextAdventure to new selection
- when we are about to submit a URL to adventure - via automation, visit_url, or the Relay Browser - set nextAdventure to where we are about to go
- When we are about to get into a fight via an item, set nextAdventure to None
- When we complete a fight, if nextAdventure is None, set it to lastAdventure
*** when we set nextAdventure, update modifiers.
*** The loc() and zone() functions in modifiers will now look at nextAdventure, not lastAdventure
*** beforeBattleScripts and counterScripts need to look at nextAdventure, not lastAdventure
Changes to observed behavior as a result of the above:
- when you use set_location() are select something in the GUI, modifiers update immediately.
- when you use an item which redirects to a fight, lastAdventure does not change
- when you complete an item-redirected fight, modifiers get set back to where they were before.
- even if a counterScript redirects you, which will change lastAdventure, when we submit the URL after invoking the counterScript, it will be changed back.
It will be tricky to figure out exactly the places where these setting are changed; I may not have them precisely correct. But I think this will be much more rational and will solve a number of problems.
Unfortunately, it is not backwards compatible.
What do you think?