Question(s) about LockableListModel and the source code in general

ajoshi

Member
My first question is... is this the right forum for questions about the KoLmafia source code? I couldn't find any other place, so I'm posting here, but I honestly have no clue if this is wrong.

Second, I see LockableListModel being used a lot in the source code. This seems to be some sort of UI component, but it's being used to store data (I think) even when the GUI is turned off. What really does LockableListModel do? Some sort of synchronized UI list? But comments say that Lockable aspects have been removed, so it's purely a UI element, correct?

I ask because I am attempting to link the KoLmafia jar in a Swing-less Java environment (I use the CLI), and LockableListModel keeps causing NoClassDefFoundErrors. I was initially commenting out instances of LockableListModel, but I'm not sure if this would break the CLI as well, or if LockableListModel is only used in the GUI, and I could remove any occurrences of it without affecting the CLI.

I'd appreciate any help!
 

lostcalpolydude

Developer
Staff member
My first question is... is this the right forum for questions about the KoLmafia source code? I couldn't find any other place, so I'm posting here, but I honestly have no clue if this is wrong.
This is as good a place as any for that.

Second, I see LockableListModel being used a lot in the source code. This seems to be some sort of UI component, but it's being used to store data (I think) even when the GUI is turned off. What really does LockableListModel do? Some sort of synchronized UI list? But comments say that Lockable aspects have been removed, so it's purely a UI element, correct?

I ask because I am attempting to link the KoLmafia jar in a Swing-less Java environment (I use the CLI), and LockableListModel keeps causing NoClassDefFoundErrors. I was initially commenting out instances of LockableListModel, but I'm not sure if this would break the CLI as well, or if LockableListModel is only used in the GUI, and I could remove any occurrences of it without affecting the CLI.

I'd appreciate any help!

I've never added a LockableListModel object to the project except when copying a very similar setup, but it's roughly used for List-type objects. Those objects are used for plenty of non-GUI internal purposes. You could probably refactor those into other List types with the GUI removed (a large project, I expect), but it would likely be more straightforward to fix the NoClassDefFoundErrors that are showing up.
 

Veracity

Developer
Staff member
Code:
public class LockableListModel<E>
	extends AbstractListModel
	implements Cloneable, List<E>, ListModel, ComboBoxModel, MutableComboBoxModel
If you take out the GUI-specific things - in particular, the "Model" interfaces - it's just a list. Internally, it uses ArrayLists.

If you really don't need it to be a "Model" for use in Swing, you could probably just turn it into another name for an ArrayList and leave all the code that uses it intact.
 

ajoshi

Member
Awesome, thanks, guys! I'll try to refactor it and see if I can use ArrayLists instead. I can't really resolve the NoClassDefFoundErrors since I'm trying to get mafia running on Android (using some funky runtime jar loading... uh oh), and Swing components can't be resolved at all. I could fork off the KoLmafia project, but I'd much rather try to use the same codebase that computer users are using.

Additionally, would it be reasonable for me to try and separate the UI Lists from the data Lists and then see if those changes can be integrated into trunk, or would that be more of a "If it ain't broke, don't fix it" sort of scenario?
 

Veracity

Developer
Staff member
There is no difference between the "UI lists" and the "Data lists"; KoLConstants.inventory is your inventory, and is used as a list all over the source - but if we want to show it in a UI element, we just use that list itself as the "model" and hand it to the Swing component. That works because LockableListModel (and SortedListModel) are both "lists" and "listmodels".

We really do not want to have to update a "data list" AND a "UI list" in parallel.

If you want to minimize changes, you are better off using a version of LockableListModel which is, essentially, just an ArrayList, and just not loading any of the UI code that uses those lists.

In theory, the "swingui" directory and subdirectories is all the UI code, but over the years, other parts of the source tree now reference methods from that section of the code. Ideally, we'd have the non-UI stuff in /session, or what have you, but we've gotten a bit sloppy.

It is tempting to do that: for example, the Familiar Trainer Frame has methods in it to train familiars, equip them, what have you, which don't have anything to do with the UI. Rather than put those things into session/FamiliarManager, say, and calling them from the UI component and also from elsewhere in KoLmafia that may need them, non-UI parts of KoLmafia call the methods from the frame.

I'm all for cleaning that up - and it would be necessary for your project - and that is completely appropriate for integration into the trunk. But "separating the UI lists from the data lists" would be a huge can of brokenness, and I think that having "UI LockableListModel" and "non-UI LockableListModel" modules, one of which you load, depending on whether you want Swing or not, would be the simplest solution.
 

Veracity

Developer
Staff member
I did a grep to see which modules not in the swingui directory import modules that are in that directory.
Some of these - KoLmafiaGUI and KoLDesktop, for example - really are, essentially, UI classes themselves.
Some of the included modules - the MRU lists, for example - are in swingui for no obvious reason; why not in utilities?
Many other imports could be done differently. For example, KoLCharacter maintains a familiar list and then calls GearChangeFrame whenever it changes. Seems like the latter could just register a listener on (familiars) and whenever KoLCharacter changes the familiar list, it fires an event on (familiars).

This is all standard MVC, but KoLmafia does not cleanly use that scheme - although it could.

This would be a big project, but would make the codebase a lot cleaner.

Code:
./BuffBotHome.java:60:import net.sourceforge.kolmafia.swingui.BuffBotFrame;
./CakeArenaManager.java:43:import net.sourceforge.kolmafia.swingui.FamiliarTrainingFrame;
./chat/ChatManager.java:75:import net.sourceforge.kolmafia.swingui.ChatFrame;
./chat/ChatManager.java:76:import net.sourceforge.kolmafia.swingui.ContactListFrame;
./chat/ChatManager.java:77:import net.sourceforge.kolmafia.swingui.GenericFrame;
./chat/ChatManager.java:78:import net.sourceforge.kolmafia.swingui.TabbedChatFrame;
./chat/ChatSender.java:54:import net.sourceforge.kolmafia.swingui.CommandDisplayFrame;
./chat/ChatSender.java:56:import net.sourceforge.kolmafia.swingui.widget.ShowDescriptionList;
./CreateFrameRunnable.java:48:import net.sourceforge.kolmafia.swingui.ChatFrame;
./CreateFrameRunnable.java:49:import net.sourceforge.kolmafia.swingui.ContactListFrame;
./CreateFrameRunnable.java:50:import net.sourceforge.kolmafia.swingui.GenericFrame;
./CreateFrameRunnable.java:51:import net.sourceforge.kolmafia.swingui.LoginFrame;
./CreateFrameRunnable.java:52:import net.sourceforge.kolmafia.swingui.SendMessageFrame;
./CreateFrameRunnable.java:53:import net.sourceforge.kolmafia.swingui.SkillBuffFrame;
./CreateFrameRunnable.java:54:import net.sourceforge.kolmafia.swingui.TabbedChatFrame;
./CreateFrameRunnable.java:56:import net.sourceforge.kolmafia.swingui.menu.GlobalMenuBar;
./KoLAdventure.java:80:import net.sourceforge.kolmafia.swingui.AdventureFrame;
./KoLAdventure.java:81:import net.sourceforge.kolmafia.swingui.CouncilFrame;
./KoLAdventure.java:82:import net.sourceforge.kolmafia.swingui.GenericFrame;
./KoLCharacter.java:117:import net.sourceforge.kolmafia.swingui.AdventureFrame;
./KoLCharacter.java:118:import net.sourceforge.kolmafia.swingui.GearChangeFrame;
./KoLCharacter.java:119:import net.sourceforge.kolmafia.swingui.MallSearchFrame;
./KoLCharacter.java:120:import net.sourceforge.kolmafia.swingui.SkillBuffFrame;
./KoLConstants.java:62:import net.sourceforge.kolmafia.swingui.menu.PartialMRUList;
./KoLConstants.java:63:import net.sourceforge.kolmafia.swingui.menu.ScriptMRUList;
./KoLDesktop.java:60:import net.sourceforge.kolmafia.swingui.AdventureFrame;
./KoLDesktop.java:61:import net.sourceforge.kolmafia.swingui.ChatFrame;
./KoLDesktop.java:62:import net.sourceforge.kolmafia.swingui.GenericFrame;
./KoLDesktop.java:63:import net.sourceforge.kolmafia.swingui.SendMessageFrame;
./KoLDesktop.java:65:import net.sourceforge.kolmafia.swingui.button.DisplayFrameButton;
./KoLDesktop.java:66:import net.sourceforge.kolmafia.swingui.button.InvocationButton;
./KoLDesktop.java:67:import net.sourceforge.kolmafia.swingui.button.RelayBrowserButton;
./KoLDesktop.java:69:import net.sourceforge.kolmafia.swingui.listener.TabFocusingListener;
./KoLmafia.java:125:import net.sourceforge.kolmafia.swingui.AdventureFrame;
./KoLmafia.java:126:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./KoLmafia.java:127:import net.sourceforge.kolmafia.swingui.DescriptionFrame;
./KoLmafia.java:128:import net.sourceforge.kolmafia.swingui.GearChangeFrame;
./KoLmafia.java:129:import net.sourceforge.kolmafia.swingui.GenericFrame;
./KoLmafia.java:130:import net.sourceforge.kolmafia.swingui.SystemTrayFrame;
./KoLmafia.java:132:import net.sourceforge.kolmafia.swingui.listener.LicenseDisplayListener;
./KoLmafia.java:134:import net.sourceforge.kolmafia.swingui.panel.GenericPanel;
./KoLmafiaGUI.java:66:import net.sourceforge.kolmafia.swingui.BuffBotFrame;
./KoLmafiaGUI.java:67:import net.sourceforge.kolmafia.swingui.BuffRequestFrame;
./KoLmafiaGUI.java:68:import net.sourceforge.kolmafia.swingui.CakeArenaFrame;
./KoLmafiaGUI.java:69:import net.sourceforge.kolmafia.swingui.CalendarFrame;
./KoLmafiaGUI.java:70:import net.sourceforge.kolmafia.swingui.ClanManageFrame;
./KoLmafiaGUI.java:71:import net.sourceforge.kolmafia.swingui.ContactListFrame;
./KoLmafiaGUI.java:72:import net.sourceforge.kolmafia.swingui.FamiliarTrainingFrame;
./KoLmafiaGUI.java:73:import net.sourceforge.kolmafia.swingui.ItemManageFrame;
./KoLmafiaGUI.java:74:import net.sourceforge.kolmafia.swingui.LoginFrame;
./KoLmafiaGUI.java:75:import net.sourceforge.kolmafia.swingui.MuseumFrame;
./KoLmafiaGUI.java:76:import net.sourceforge.kolmafia.swingui.MushroomFrame;
./KoLmafiaGUI.java:77:import net.sourceforge.kolmafia.swingui.OptionsFrame;
./KoLmafiaGUI.java:78:import net.sourceforge.kolmafia.swingui.SendMessageFrame;
./KoLmafiaGUI.java:79:import net.sourceforge.kolmafia.swingui.SkillBuffFrame;
./KoLmafiaGUI.java:80:import net.sourceforge.kolmafia.swingui.StoreManageFrame;
./KoLmafiaGUI.java:81:import net.sourceforge.kolmafia.swingui.SystemTrayFrame;
./KoLmafiaGUI.java:233:			Class frameClass = Class.forName( "net.sourceforge.kolmafia.swingui." + frameName );
./maximizer/Maximizer.java:76:import net.sourceforge.kolmafia.swingui.MaximizerFrame;
./persistence/ConcoctionDatabase.java:93:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./persistence/ConcoctionDatabase.java:94:import net.sourceforge.kolmafia.swingui.ItemManageFrame;
./persistence/ItemDatabase.java:87:import net.sourceforge.kolmafia.swingui.DatabaseFrame;
./preferences/Preferences.java:70:import net.sourceforge.kolmafia.swingui.AdventureFrame;
./request/AdventureRequest.java:74:import net.sourceforge.kolmafia.swingui.RequestSynchFrame;
./request/BountyHunterHunterRequest.java:59:import net.sourceforge.kolmafia.swingui.AdventureFrame;
./request/CharPaneRequest.java:70:import net.sourceforge.kolmafia.swingui.MallSearchFrame;
./request/CharPaneRequest.java:71:import net.sourceforge.kolmafia.swingui.RequestFrame;
./request/CoinMasterRequest.java:60:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./request/Crimbo11Request.java:52:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./request/DrinkItemRequest.java:59:import net.sourceforge.kolmafia.swingui.GenericFrame;
./request/EatItemRequest.java:67:import net.sourceforge.kolmafia.swingui.GenericFrame;
./request/FreeSnackRequest.java:50:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./request/GameShoppeRequest.java:54:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./request/GenericRequest.java:106:import net.sourceforge.kolmafia.swingui.RequestSynchFrame;
./request/LoginRequest.java:56:import net.sourceforge.kolmafia.swingui.AnnouncementFrame;
./request/PeeVPeeRequest.java:55:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./request/RelayRequest.java:107:import net.sourceforge.kolmafia.swingui.AdventureFrame;
./request/RelayRequest.java:108:import net.sourceforge.kolmafia.swingui.CommandDisplayFrame;
./request/SpleenItemRequest.java:53:import net.sourceforge.kolmafia.swingui.GenericFrame;
./request/StorageRequest.java:63:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./request/UntinkerRequest.java:63:import net.sourceforge.kolmafia.swingui.GenericFrame;
./request/UseItemRequest.java:91:import net.sourceforge.kolmafia.swingui.GenericFrame;
./RequestEditorKit.java:108:import net.sourceforge.kolmafia.swingui.RequestFrame;
./RequestEditorKit.java:110:import net.sourceforge.kolmafia.swingui.widget.RequestPane;
./RequestThread.java:53:import net.sourceforge.kolmafia.swingui.SystemTrayFrame;
./session/ContactManager.java:50:import net.sourceforge.kolmafia.swingui.ContactListFrame;
./session/EquipmentManager.java:74:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./session/EquipmentManager.java:75:import net.sourceforge.kolmafia.swingui.GearChangeFrame;
./session/EventManager.java:54:import net.sourceforge.kolmafia.swingui.SystemTrayFrame;
./session/HaciendaManager.java:62:import net.sourceforge.kolmafia.swingui.CouncilFrame;
./session/InventoryManager.java:93:import net.sourceforge.kolmafia.swingui.GenericFrame;
./session/IslandManager.java:65:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./session/LeafletManager.java:48:import net.sourceforge.kolmafia.swingui.CouncilFrame;
./session/LoginManager.java:69:import net.sourceforge.kolmafia.swingui.GenericFrame;
./session/LogoutManager.java:58:import net.sourceforge.kolmafia.swingui.LoginFrame;
./session/MushroomManager.java:60:import net.sourceforge.kolmafia.swingui.MushroomFrame;
./session/ResultProcessor.java:91:import net.sourceforge.kolmafia.swingui.CoinmastersFrame;
./session/SorceressLairManager.java:69:import net.sourceforge.kolmafia.swingui.CouncilFrame;
./session/StoreManager.java:70:import net.sourceforge.kolmafia.swingui.StoreManageFrame;
./session/TavernManager.java:55:import net.sourceforge.kolmafia.swingui.CouncilFrame;
./StaticEntity.java:56:import net.sourceforge.kolmafia.swingui.DescriptionFrame;
./StaticEntity.java:57:import net.sourceforge.kolmafia.swingui.panel.GenericPanel;
./textui/command/CallScriptCommand.java:53:import net.sourceforge.kolmafia.swingui.GenericFrame;
./textui/command/CouncilCommand.java:39:import net.sourceforge.kolmafia.swingui.CouncilFrame;
./textui/command/FaxbotCommand.java:47:import net.sourceforge.kolmafia.swingui.FaxRequestFrame;
./textui/command/ShowDataCommand.java:67:import net.sourceforge.kolmafia.swingui.CalendarFrame;
./textui/command/TestCommand.java:100:import net.sourceforge.kolmafia.swingui.SkillBuffFrame;
./textui/command/TrainFamiliarCommand.java:39:import net.sourceforge.kolmafia.swingui.FamiliarTrainingFrame;
./textui/RuntimeLibrary.java:190:import net.sourceforge.kolmafia.swingui.FaxRequestFrame;
./textui/RuntimeLibrary.java:191:import net.sourceforge.kolmafia.swingui.widget.InterruptableDialog;
./utilities/InputFieldUtilities.java:62:import net.sourceforge.kolmafia.swingui.GenericFrame;
./utilities/InputFieldUtilities.java:64:import net.sourceforge.kolmafia.swingui.widget.AutoFilterTextField;
./utilities/InputFieldUtilities.java:65:import net.sourceforge.kolmafia.swingui.widget.GenericScrollPane;
 

Veracity

Developer
Staff member
OK, as a proof of concept, revision 15347 refactors the CoinmastersFrame to use listeners. All the various places which wanted to poke it now simply fire the "(coinmaster)" listener and, if you have instantiated the frame, it updates, since it has registered to get events on that listener.
 

ajoshi

Member
Thanks a lot! I'll take a look this weekend (I'm pretty lazy). And thanks for the list as well- I didn't even think of grepping for that, so I'll definitely be working on those first. And as you said, cleaning this up would be necessary for my project.

I am confused by
If you want to minimize changes, you are better off using a version of LockableListModel which is, essentially, just an ArrayList, and just not loading any of the UI code that uses those lists.
since I don't see how I can load a version of LockableListModel without loading UI code (since it seems to implement Swing or Swing-like interfaces, forcing a dependency on Swing UI), but I might end up figuring out what you mean as I clean up that list of non-MVC classes.

Once again, thanks! And I'll be sure to bug you guys with my many questions!
 

Darzil

Developer
I am confused by
since I don't see how I can load a version of LockableListModel without loading UI code (since it seems to implement Swing or Swing-like interfaces, forcing a dependency on Swing UI), but I might end up figuring out what you mean as I clean up that list of non-MVC classes.
This isn't an area I really understand (I've been changing code rather than creating new) about Java, but my interpretation would be you'd create a LockableListModel that inherits ArrayList functionality, so that it is then used instead of the Swing one.
 

Veracity

Developer
Staff member
I don't see how I can load a version of LockableListModel without loading UI code (since it seems to implement Swing or Swing-like interfaces, forcing a dependency on Swing UI), but I might end up figuring out what you mean as I clean up that list of non-MVC classes.
The current LockableListModel implements various UI interfaces. I suggested you make a version of that file which is still NAMED LockableListModel, but which does not declare that it implements those interfaces- or provide the methods they provide - but simply implements List<E> and Cloneable, and wraps an ArrayList.

Yes, if that's all it is, the rest of KoLmafia could simply use an ArrayList wherever it currently uses a LockableListModel, but I was trying to suggest a way to not have to modify the rest of the code.

Edit: which is basically what Darzil just said. ;)
 

xKiv

Active member
Yet another possible step is to use a factory class that knows whether LockableListModel is available and gives you what you actually have.

This would look something like
Code:
class LockableListFactory {
  
	static Class<? extends List<?>> clazz;
	
	static {
		try {
			clazz = (Class<? extends List<?>>) Class.forName("net.java.dev.spellcast.utilities.LockableListModel");
		} catch (ClassNotFoundException e) {
			clazz = (Class<? extends List<?>>) ArrayList.class;
		}
	}
	
	public static <E> List<E> instance() {
		try {
			return (List<E>) clazz.newInstance();
		} catch (InstantiationException | IllegalAccessException e) {
			throw new RuntimeException(e);
		}
	}
	
	public static <E> List<E> instance(Collection<E> c) {
		try {
			Constructor<? extends List<?>> constructor = clazz.getConstructor(Collection.class);
			List<E> ret = (List<E>) constructor.newInstance(c);
			return ret;
		} catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException
				| InvocationTargetException e) {
			throw new RuntimeException(e);
		}
	}
	
	// all the other constructors here

};

then everywhere in mafia that uses ArrayListModel and isn't inside swing-specific classes would change from ArrayListModel<E> to List<E>, and swing-specific code would have to cast. The casting is not ideal, but the alternatives are either using more relfexion, or wrapper classes.
 

ajoshi

Member
Actually, I was looking at Factory-like behavior, and once I'm done with the cleanup, I'll be trying wrapper classes and factories to see which way works out best. Thanks a lot, guys- you were all insanely helpful! Not that I expected you guys to be mean, but I am overwhelmed with the help you guys have provided! :)
 

ajoshi

Member
So lazy bum me finally got around to the Listeners stuff. I added a listener to AdventureFrame so it can be updated by KoLAdventure. I also moved CouncilRequest COUNCIL_VISIT from CouncilFrame to CouncilCommand. Does that seem reasonable?

The last time I used SVN was about 8 years ago, so I'm not sure how to commit. It seems unauthenticated commits aren't allowed (lol) so how does one go about getting code into the repo? Also, is there some way for someone to review my code? I don't want to break anything! Do I make patches and send them over?

Finally, in NamedListenerRegistry, how do I know that the listener I am registering isn't overwriting a preexisting listener? I fired mine from KoLAdventure.java, so I called it "koladventure". Is that ok?
 

lostcalpolydude

Developer
Staff member
Posting a patch here is a good approach, probably as an attachment. Chances are there will be at least minor changes made before it gets added, if it's mostly good. That also puts it up for feedback if needed.
 

ajoshi

Member
This patch attempts to clean up KoLAdventure.java.

AdventureFrame.java: added listener for "koladventure"
KoLAdventure.java: Fires "koladventure", add check for GUI before checking for frame existence, moved COUNCIL_VISIT
CouncilFrame.java, CouncilCommand.java: Moved COUNCIL_VISIT from CouncilFrame to CouncilCommand
The remaining ones are just CouncilFrame.COUNCIL to CouncilCommand.COUNCIL changes due to the above change.

As you can see, I couldn't think of a way to pull out the GenericFrame.instanceExists() check out of KoLAdventure.

Also, in KoLAdventure.setNextAdventure, I call KoLCharacter.updateSelectedLocation before updateSelectedAdventure. I hope that's ok.

Thanks!
 

Attachments

  • kolmafiaPatch2.patch
    10.4 KB · Views: 19
Last edited:

lostcalpolydude

Developer
Staff member
I would suggest making your attachment not inline, it tends to be simpler. Right now I can't access whatever you attached.
 

ajoshi

Member
Hey guys, I don't mean to pressure anyone, but what's the general timeline for getting a patch looked at? I absolutely understand if people are too busy to take a look, but I'd really appreciate an update anyway!
 

Attachments

  • kolmafiaPatch2.patch
    10.4 KB · Views: 17

fronobulax

Developer
Staff member
Hey guys, I don't mean to pressure anyone, but what's the general timeline for getting a patch looked at? I absolutely understand if people are too busy to take a look, but I'd really appreciate an update anyway!

There is no timeline. What happens is that if a dev is interested in a patch they will look at it. I didn't look at it before since I was not sure whether it was of wide interest or not and the description set off my "First, Do No Harm" alarm bells. When I am not sure what I am doing I often introduce unintended side effects, and this is not an area of the code I've spent much time in.

All that said, I have the patch. I will apply it, self-test and otherwise come to a conclusion after which I will report back or commit it. Since I don't see much of this happening until the weekend, if anyone else wants to look at it or address "First, Do No Harm" that's fine with me.
 

ajoshi

Member
All that said, I have the patch. I will apply it, self-test and otherwise come to a conclusion after which I will report back or commit it. Since I don't see much of this happening until the weekend, if anyone else wants to look at it or address "First, Do No Harm" that's fine with me.

Thanks! And I do hope it does no harm! I ran my local jar and it was working fine, and in theory it should work fine as well, but I really do appreciate you taking some time to verify it!
 
Top