Feature - Implemented Hidden City, ` key, and "Adventure Again" link causes wasted turns in run

[Turias]

Mafia currently adds two links to the bottom of the Hidden City adventuring page:

Adventure Again in These Ruins
Explore Some Unexplored Ruins

They are placed in this order, causing the ` key to "Adventure Again in These Ruins" instead of "Explore Some Unexplored Ruins". This is almost never what you want to do, especially when in-run. I personally have wasted a few adventures over my past few ascensions hitting the ` key while doing the Hidden City.

Here are two potential patches:

1. Swap the order of these two links so that "Explore Some Unexplored Ruins" is the one that is automatically selected by the ` key.

https://gist.github.com/5434f770803c5da5a4d6/f4d823f957c3b374e5432ee6b305c19092402b59

2. If there are still unexplored squares, only show the "Explore Some Unexplored Ruins" link. Otherwise, only show the "Adventure Again in These Ruins" link.

https://gist.github.com/5434f770803c5da5a4d6/aef9dd69d180ad961d2ddcaf928a5d13a46eb4d2

(2) is less error-prone for ascensions. (1) gives the player a little more control. Personally, I think (2) makes more sense overall, but I would be happy with either.

 

[Veracity]

Mafia currently adds two links to the bottom of the Hidden City adventuring page:

Adventure Again in These Ruins
Explore Some Unexplored Ruins

They are placed in this order, causing the ` key to "Adventure Again in These Ruins" instead of "Explore Some Unexplored Ruins".

Interesting. When _I_ go to the Hidden City, the links are in the opposite order:

Explore Some Unexplored Ruins
Adventure Again in These Ruins

This allows me to just hit the "again" button and go through the unexplored squares in order.

I do this every ascension.

This is almost never what you want to do, especially when in-run. I personally have wasted a few adventures over my past few ascensions hitting the ` key while doing the Hidden City.

How odd. I wonder why it does it the opposite for you?

1. Swap the order of these two links so that "Explore Some Unexplored Ruins" is the one that is automatically selected by the ` key.

This patch will break the existing code and make it behave the way you don't like. Perhaps you misunderstand the code:

- We calculate an "index" in which to insert new links.
- If we have a "current" square, we insert the "Adventure Again in These Ruins" at "index". That places that link ahead of the "Return to Hidden City Map" link. We do NOT change "index".
- If you have unexplored ruins, we insert the "Explore some Unexplored Ruins" at "index" - which is to say, in FRONT of the link we inserted.

2. If there are still unexplored squares, only show the "Explore Some Unexplored Ruins" link. Otherwise, only show the "Adventure Again in These Ruins" link.

I take it that you have never found all the spheres, all the altars, and the temple before exploring the city. If I want to backfarm for a blowgun, say, and I am otherwise done with the city, I see no reason to go exploring - and, perhaps, get the dead archaeologist.

Personally, I think (2) makes more sense overall, but I would be happy with either.

I would be very unhappy with #1, which would break the order of the links.

 

[Turias]

Interesting. When _I_ go to the Hidden City, the links are in the opposite order

Huh. Very strange. I see what you mean about the buffer insertions... I am not sure why this is happening, then. Maybe there is a third link above the other two inserted by Vanilla KoL that mimics the "Adventure Again in These Ruins" functionality? I do not currently have any unexplored squares so this is going to be very hard for me to test until next ascension. However, I am positive that for the past 3 ascensions ` has done the wrong thing. I have mafia and ascension logs that display this (2-3 adventures in a row at the same square where I am sure that I hit `).

 

[Theraze]

Any chance you're using Mr Script or some other non-mafia addition to cause those links?

 

[Turias]

I don't think so, but I will do some in-depth debugging next ascension. I do not use any Greasemonkey scripts and I have very few mafia relay scripts installed (if those can even do something like this).

 

[Bale]

I have very few mafia relay scripts installed (if those can even do something like this).

They can. Relay scripts can modify a page in virtually every way imaginable. Take a look at Chit for an example of just how radical the changes can be.

 

[lostcalpolydude]

I have the links in the "right" order (unexplored followed by adventure again). The "again" button provided by mafia links to the next square. The ` key still adventures in the square I just finished. Maybe it has nothing to do with mafia and might instead be a feature request to change KoL behavior. That would require checking what happens when not logged in with mafia though...

 

[Turias]

I can confirm that:

1. vanilla KoL does *nothing* when hitting ` after spending a turn in the hidden city.
2. vanilla mafia (r10889) goes back to the square you were just in when hitting ` in the hidden city.

I am trying to figure out why this is, but I am heading down the rabbit hole.

 

[Veracity]

I assume we do something to the ` button. We DO modify the CAB - to add a script button, for example. But, I have never used the CAB and am not especially eager to educate myself about it, right at the moment. :-/

 

[Turias]

Alright, I have figured most of this out. Sorry for the initial confusion about the order of the links.

Hitting ` appears to run code in /images/scripts/actionbar.20110713.js to actually revisit your current location. The interesting code is:

		if (params["type"] == "action" && params["id"] == "repeat")
		{
			if (typeof timersfunc != 'undefined') {
				if (!timersfunc()) return;
			}
			var linx = document.getElementsByTagName("A");
			for (var i = 0; i < linx.length; i++)
			{
				var link = linx[i];
				if (link.innerHTML.match(/Adventure Again/))
					location.href = link.href;
			}
		}

As you can see, vanilla KoL literally looks for an anchor tag with the text "Adventure Again" contained somewhere inside of it. It then hits the URL of that particular tag.

So, if we look at the URLs that RequestEditorKit.fixHiddenCity are adding:

		int current = HiddenCityRequest.lastHiddenCitySquare();
		if ( current > 0 )
		{
			link.setLength( 0 );
			link.append( "<p><a href=\"hiddencity.php?which=" );
			link.append( String.valueOf( current - 1 ) );
			link.append( "\">Adventure Again in These Ruins</a>" );
			buffer.insert( index, link.toString() );
		}

		int unexplored = HiddenCityRequest.firstUnexploredRuins();
		if ( unexplored > 0 )
		{
			link.setLength( 0 );
			link.append( "<p><a href=\"hiddencity.php?which=" );
			link.append( String.valueOf( unexplored - 1 ) );
			link.append( "\">Explore Some Unexplored Ruins</a>" );
			buffer.insert( index, link.toString() );
		}

The text "Adventure Again" appears in the "Adventure Again in These Ruins" link and *NOT* the "Explore Some Unexplored Ruins" link.

Thus, to fix this issue we can simply change the text of these two links to:

Adventure Again in some Unexplored Ruins

and

Adventure Once More in These Ruins

Thoughts?

 

[Veracity]

I think both of those proposed labels sound awkward. I'd be more in favor of just changing "Adventure Again in these Ruins" to not have "Adventure Agin" in it, somehow, which would leave the ` button doing nothing - just as in vanilla KoL.

 

[Turias]

Hmmm, even though I think that I want "unexplored" 99.9% of the time, that .1% might win out as we never ever want to waste someone's adventures. So yeah, I can live with that suggestion.

However, this does make me think that StationaryButtonDecorator.getAdventureAgainLocation is very wrong, and will lead to different results depending on if the ` key is hit and the similar Combat Action Bar "again" button is hit. Shouldn't getAdventureAgainLocation do the same thing as that javascript code and look for a link with the text "Adventure Again" in it? That way things would be consistent.

 

[Turias]

Sorry for the bump, but to me, this is a fairly serious bug. Can we just change the test to "Adventure Yet Again in These Ruins" and be done with it? I am fine with alternate wording, but this isn't really a "Feature" request as someone has updated this bug to suggest. Wasting turns in-run by doing something unexpected is pretty serious.

 

[Theraze]

I'd be fine with Veracity's suggestion to just remove the "Adventure again" text to keep it reading naturally and make ` do nothing. If Turias makes a relay override that adds the "Adventure again" text on their own side to the proper adventure choice, would that grab the ` or is that a mafia behind-the-scenes thing that still wouldn't trigger?

 

[Turias]

Mafia isn't doing anything with the ` key. So yes, anyone with a relay override that changes the text of the adventure again links can alter the behavior of `.

However, I am also fine with ` doing nothing, as stated by my suggested fix in my previous post.

 

[Veracity]

Can we just change the test to "Adventure Yet Again in These Ruins" and be done with it?

Why not.

I am fine with alternate wording, but this isn't really a "Feature" request as someone has updated this bug to suggest.

Considering that it is not a bug in KoLmafia, I'm not sure how else to categorize it.

Wasting turns in-run by doing something unexpected is pretty serious.

It is unfortunate that KoL has chosen to implement the ` button in the CAB that way.

I continue to be glad that I do not use the CAB.

 

[Winterbay]

It's interesting that they've chosen the ` key as that has very different places on different keyboard layouts. I've never been able to press a button to adventure again since that button on my keyboard layout is "shift + ´" which won't work.
Apart form that I love the CAB

 

[Veracity]

Revision 10907. Untested.

 

[Turias]

Thanks!