Another issue I see.. In FreeColMenuBar some of the menu items are defined as seperate classes and included in the ActionManager. Others are defined as anonymous inner classes.
I think all of them should be included in the ActionManager, and defined as seperate classes, if only for consistency. That would help us with keeping them all valid as well. Presently, the biggest problem with the menus is that the mnemonics (and accelerators?) are overriding each other so they don't do what you would expect.
And last, since I'm new on this project, how much liberty should I take writing the code? I would rather not waste time writing something someone else is doing, or something that won't get accepted as a contribution, etc. Are there any guidelines, pitfalls, etc., I should watch out for?
I was stumbling through the code, trying to fix the menu options so they work as one would expect. OptionGroup is a class used like a Factory for menu options. It stores an instance of each Action in a List. When another actor needs an Action, it iterates through the List to find the ID they requested.
I know its a small thing, but wouldn't this be better implemented as a HashMap rather than an ArrayList as it is? We could store each action by its ID in the HashMap, then it would take 1 step to find it versus at most N as it is now.
I can make the change easy enough. But what do you think?