Bug - Fixed broken image for 100% familiar run warning w/ no familiar

IronTetsubo · Jun 12, 2012

Noticed this a while ago, finally got around to patching - the "no familiar" icon is "debug.gif", which (as far as I can tell) is a kolmafia-only image. As such, when you're doing a 100% familiar run and then try to use a different familiar, the warning page that pops up in the relay browser has a broken image because it references the image on kol's servers instead of localhost/kolmafia's images.

Super minor, but hey, a broken image is a broken image. Attached is a patch to address. I debated trying to find a better "no familiar" image to use instead, but couldn't find anything. If that's to be preferred (i.e., no longer using kolmafia's debug.gif), let me know what image you'd prefer and I will fix my patch. thanks!

Veracity · Jun 12, 2012

What is the image KoL uses in your Ascension History for Axecore runs?

Bale · Jun 12, 2012

Veracity said:
What is the image KoL uses in your Ascension History for Axecore runs?

None: <td></td>

HTML:

<td class=small valign=center>195   </td><td height=30 class=small valign=center>04/22/12  </td><td class=small valign=center><span title="Level at Ascension: 13">13</span>   </td><td class=small valign=center><img src="http://images.kingdomofloathing.com/itemimages/trusty.gif" width=30 height=30 alt="Avatar of Boris" title="Avatar of Boris"></td><td class=small valign=center>Mongoose  </td><td class=small valign=center><span title='Total Turns: 887'>883</span></td><td class=small valign=center><span title='Total Days: 4'>4</span></td><td></td><td class=small valign=center><img src="http://images.kingdomofloathing.com/itemimages/hardcorex.gif" width=30 height=30 alt="Hardcore" title="Hardcore"><img src="http://images.kingdomofloathing.com/itemimages/trusty.gif" width=30 height=30 alt="Avatar of Boris" title="Avatar of Boris"></td></tr>

That leads to a bug when KoLmafia looks at ascensionhistory.php for Axecore.

Veracity · Jun 12, 2012

How odd. In Safari, it displays as a bunch of little question marks, sort of like Lost's Avatar. I wonder what trickery makes the browser display that? I copied it and opened it in a new window and the image address appears to be

images/itemimages/confused.gif

Bale · Jun 12, 2012

That's a Mafia bug. Open KoL without mafia and you'll see something else.

IronTetsubo · Jun 12, 2012

shrug. should I change it to the confused.gif, above? Or I can poke around for other effects, see if I can find anything roughly symbolizing nothing or none.

Bale · Jun 12, 2012

Code:

string [string] images;
void addimage(string name, string img) {
	if(images[img] == "")
		images[img] = name;
	else images[img] = images[img] + ", "+name;
}

foreach i in $items[]
	addimage(i.to_string(), i.image);

foreach e in $effects[]
	addimage(e.to_string(), e.image.substring(47));

if(images contains "") {
	writeln("<p>Missing image information for: "+images[""]+"</p>");
	remove images[""];
}

write("<p>");
foreach i,t in images {
	write('<div title = "'+ t +'" style="float:left">');
	write('<img src="http://images.kingdomofloathing.com/itemimages/' + i + '" height=30 width=30></div>');
}
writeln("</p>");

Veracity · Jun 12, 2012

Bale said:
That's a Mafia bug. Open KoL without mafia and you'll see something else.

Well, I'll be. RequestEditorKit.java says:

Code:

		else if ( location.startsWith( "ascensionhistory.php" ) )
		{
...
				StringUtilities.globalStringReplace(
					buffer,
					"<td></td>",
					"<td><img src=\"http://images.kingdomofloathing.com/itemimages/confused.gif\" title=\"No Data\" alt=\"No Data\" height=30 width=30></td>" );
...
		}

You are correct that that KoLmafia is putting that image in.

I'm not sure what problem that is supposed to solve, although I'm sure a little digging in the SVN repository will show me which revision made it do that.

Interesting that you characterize something that KoLmafia is, apparently, specifically coded to do as a "bug". Care to elaborate?

lostcalpolydude · Jun 12, 2012

It was added in 2260.

Winterbay · Jun 12, 2012

I'm guessing that it's supposed to fix the fact that older ascencions doesn't have any familiar info and as such if Mafia's log-thingie looks for one it would be problematic and thus an image was added. Perhaps.

holatuwol · Jun 12, 2012

It's there so that people see some type of image for their older ascensions. We can remove that code if it's viewed as a bug.

IronTetsubo · Jun 12, 2012

Bale said:
Well, some fun choices for no familiar would be

http://images.kingdomofloathing.com/itemimages/driedface.gif
http://images.kingdomofloathing.com/itemimages/trivia5.gif
http://images.kingdomofloathing.com/itemimages/qpopcorn.gif
http://images.kingdomofloathing.com/itemimages/baiotalz.gif
http://images.kingdomofloathing.com/itemimages/fudgeplus.gif
http://images.kingdomofloathing.com/itemimages/missingpants.gif
http://images.kingdomofloathing.com/itemimages/triviacard.gif
http://images.kingdomofloathing.com/itemimages/tickegg.gif
http://images.kingdomofloathing.com/itemimages/cueball.gif
http://images.kingdomofloathing.com/itemimages/light.gif

... Okay, the last two would probably be the best and most boring choices. Or maybe just http://images.kingdomofloathing.com/itemimages/blank.gif

I use the following relay script for this:

Code:

string [string] images; void addimage(string name, string img) { if(images[img] == "") images[img] = name; else images[img] = images[img] + ", "+name; } foreach i in $items[] addimage(i.to_string(), i.image); foreach e in $effects[] addimage(e.to_string(), e.image.substring(47)); if(images contains "") { writeln("<p>Missing image information for: "+images[""]+"</p>"); remove images[""]; } write("<p>"); foreach i,t in images { write('<div title = "'+ t +'" style="float:left">'); write('<img src="http://images.kingdomofloathing.com/itemimages/' + i + '" height=30 width=30></div>'); } writeln("</p>");

Well, my take would be having *something* is important (i.e., not blank), otherwise there's not much for someone to click on. Of the various options, the light one - http://images.kingdomofloathing.com/itemimages/light.gif - seemed the most "you don't have a familiar" to me. Now, to make this consistent, this would also change the icon in the sidebar of the main kolmafia interface for your familiar when you don't have a familiar. I think (but have to double-check later) that would also remove the usage of debug.gif. So - devs, what's your take? Should I...

- Drop my changes entirely and change "no familiar" to use light.gif (and also remove debug.gif from source)

or something else entirely?

Bale · Jun 12, 2012

Veracity said:
Interesting that you characterize something that KoLmafia is, apparently, specifically coded to do as a "bug". Care to elaborate?

It is a bug because it is produces confusing information that does not exist in standard KoL. I do not believe it is improving the display in any way by adding something there. Furthermore, I would hesitate to think that another (including yourself) believes the added information is an improvement in that context. As Hola points out, this serves a purpose in addition to causing a problem. It was overlooked that this would insert the image even when no familiar was used. Overlooking a potential consequence is a bug.

Was that elaborate enough for you?

roippi · Jun 12, 2012

fixed after hola's 11129/30?

Veracity · Jun 13, 2012

Bale said:
IWas that elaborate enough for you?

I disagree with pretty much everything you said there, but yes, you did elaborate on your point of view.
Hola's commit makes MY point of view moot, so, whatever.

holatuwol · Jun 13, 2012

IronTetsubo said:
Drop my changes entirely and change "no familiar" to use light.gif (and also remove debug.gif from source)

I kinda like the debug bugbear, and I pushed in something equivalent to your changes in a recent commit. So consider your changes accepted. If someone really likes light.gif instead of the debug bugbear (or thinks it conveys the 'no familiar' idea better), we can re-evaluate.

Bale said:
It is a bug because it is produces confusing information that does not exist in standard KoL.

Well, granted it will break Greasemonkey / relay scripts that rely on it being blank when there's no familiar for that run, but I didn't think confused.gif was confusing or unattractive (I actually put it in because it conveyed the "nobody has any idea what familiar you used" idea rather well). But bleh, on reflection, it's not worthwhile to keep it if it prevents people from creating scripts that might rely on the empty table cell to convey the "no familiar used" information and thus treat confused.gif as a KoLmafia quirk/bug rather than as a nifty enhancement, so I removed it.

Bale · Jun 13, 2012

holatuwol said:
I didn't think confused.gif was confusing or unattractive (I actually put it in because it conveyed the "nobody has any idea what familiar you used" idea rather well).

I agree with you. It was quite effective in the original context of providing elaboration upon missing familiar information for earlier runs. The only problem is that for more recent runs, when there was actually no familiar, it adds the implication that there was a familiar whose identity is unknown.

If you could provide confused.gif for missing familiar information on runs begun before November 7, 2006 it would be helpful and no longer misleading. Obviously that is a bit more complicated.

IronTetsubo · Jun 13, 2012

holatuwol said:
I kinda like the debug bugbear, and I pushed in something equivalent to your changes in a recent commit. So consider your changes accepted. If someone really likes light.gif instead of the debug bugbear (or thinks it conveys the 'no familiar' idea better), we can re-evaluate.

Yup, totally cool with that - my original patch used the debugbear. That being said, if you're going to keep using the debugbear, why not move it to src/data/images/itemimages, and then drop the whole "if" block you added? you'd also have to change the couple of other references in persistance/*.java, but there you also get to clean up some code. As before, happy to whip up a patch if that makes sense, although I suspect it'd be that much easier to make the changes yourself.

IronTetsubo · Jun 16, 2012

Bump - you're welcome to close this bug out, since my original goal of "no more broken image!" has been solved. I'm still happy to whip up a patch to move debugbear to itemimages and remove the conditionals, if it's useful.

Bug - Fixed broken image for 100% familiar run warning w/ no familiar

IronTetsubo

Member

Attachments

Veracity

Developer

Bale

Minion

Veracity

Developer

Bale

Minion

IronTetsubo

Member

Bale

Minion

Veracity

Developer

lostcalpolydude

Developer

Winterbay

Active member

holatuwol

Developer

IronTetsubo

Member

Bale

Minion

roippi

Developer

Veracity

Developer

holatuwol

Developer

Bale

Minion

IronTetsubo

Member

IronTetsubo

Member