MenuScroller

Getting a heart attack here… what’s going on with VASSAL.tools.menu.MenuScroller?

It gets a JMenu or JPopupMenu, and attaches a listener to it.

Then it overrides Object.finalize() where it detaches the listener from the passed menu. This is very problematic already, finalize() has long been deprecated and no sane person should be using it, for various reasons.

But it doesn’t stop here.

The only reference to MenuScroller is from VASSAL.configure.BeanShellExpressionConfigurer, where it passes a menu into MenuScroller, but does not save the reference to the MenuScroller, making it eligible for GC right away.

This is saying, hey I want to attach a listener to this menu, and right after it’s attached I allow the VM to deattach it at any time, or not deattach it at all, and to make things even more interesting I get involved in the finalization process and with some luck I get the whole VM to choke and die.

This is “let’s play russian roulette, but the one cartridge may contain a regular bullet, or a blank, or be a shotgun shell, or an expanding projectile banned by the Hague Convention, or a miniature version of a tactical warhead, and none of the participants can know what it is”.

I’m afraid of asking what this thing does at all, there is some kind of timer involved too, we are not scrolling the menu for the user are we? I might be a backend dev but I know some basic rules of frontend design and one of them is, you don’t mess with the users GUI, you don’t move his cursor, you don’t move his mouse, you don’t move his windows or scroll for him.

Can this thing be removed entirely?

It’s only referenced the one time it looks like, in the BeanShell “expression box”, and now I guess I understand why that box “sometimes populates and sometimes totally doesn’t”.

From memory, this had to do with the way brain-dead way Java displayed large pop-up menus back in Java 1.5. Because the BeanShellExpressionConfigurer generates pop-up menus dynamically based on the properties and traits available in a GamePiece, the generated pop-up menu can have many entries. In 1.5, Java happily displayed these right off the bottom of the visible screen so that the bottom entries where impossible to access.

The MenuScroller controlled the maximum number of entries that should be displayed in a pop-up menu and provide top and bottom scroll icons to allow the excess items to be scrolled inside the pop-up menu.

What is the current best practice for this?

Good question. I know next to nothing about the front-end side of Java :smiley:

The functionality sounds legit, I’d just implement it differently, probably extend the JMenu, or wrap it, sth like “menu = new VassalScrollableMenu(new JPopupMenu());”, decorator pattern style. Or just attach a listener to the existing menu and let the GC take care of their lifecycles and finalization.

But first I’d need a test corset, any ideas how I can test this? Perhaps a module which has such a menu with too many entries?