Bug - Fixed Selecting adventure does not always make it visible

[Veracity]

When KoLmafia reads lastAdventure and attempts to set the AdventureSelectPanel to point to it, the adventure is not always visible.

AdventureSelectPanel.updateSelectedAdventure does this:

			( (AutoFilterTextField) this.zoneSelect ).setText( location.getZone() );

This sets the text in the filter box to the "zone" - "The Sea" or "Dreadsylvania", for example, which will filter the displayed adventures in the list of locations to show only those in the selected zone. It then does this:

			this.locationSelect.setSelectedValue( location, true );
			this.locationSelect.ensureIndexIsVisible( this.locationSelect.getSelectedIndex() );

which selects the specific location. That works, as you can see if you last adventured in Dreadsylvania: Dreadsylvanian Castle, for example; there are only three locations in Dreadsylvana, all of which fit in the 4-location window, and the correct one is selected.

However, if your location was, say, The Sea: The Coral Corral, although that location IS selected, you have to scroll to find it; that location is not made visible, contrary to what you might expect by the call to ensureIndexIsVisible.

Java documentation says this about JList.ensureIndexIsVisible:

Scrolls the list within an enclosing viewport to make the specified cell completely visible. This calls scrollRectToVisible with the bounds of the specified cell. For this method to work, the JList must be within a JViewport.

The JList is inside a GenericScrollPane, which is a JScrollPane, which manages a viewport.

So, what's the deal?

 

[Veracity]

AdventureSelectPanel.updateSelectedAdventure is called with a KoLAdventure.
It sets the "zone" selector to the zone in which that location lives.
It then sets the selected item in the whole list to the selected adventure.

If the zone selector is an AutoFilterTextField, first it sets the text to the zone - which supposedly refilters to show everything in that zone - and then when it sets the selected item, that item will be visible and it will scroll to it.

That doesn't work, since the AutoFilterTextField pauses before it does its update and the location is not in the visible section when I set it selected. The second time through, though, it will highlight it, since the zone's locations are all present.

Now, if I force the AutoFilterTextField to update immediately, it works - except when I get an ABE exception in the Swing thread, occasionally.

 

[Veracity]

Yeah. Well, all that code in AutoFilterTextField to do pausing and defer updates was, presumably, because updating a filter on the underlying list model can be slow. Unfortunately, it just doesn't work correctly.

The first rule of optimization: "You can make any code as fast as you want if you don't care if the results are correct."

I do care. And that "optimization" is gone, and this bug is fixed. I have an idea on how to make filtering in a LockableListModel much faster: when calculating which elements become visible (or cease to be visible), track ranges of indices and fire addInterval or removeInterval events on ranges, rather than firing one event per changed element. But that will go into another revision.

This is fixed in revision 13131

 

[fronobulax]

The first rule of optimization: "You can make any code as fast as you want if you don't care if the results are correct."

Jon Bentley used to tell us that it was easier to make a working program fast than it was to make a fast program work.

Of course, these days I seem to be the poster child for properly deciding whether something "works".