Home > aen's Blog

Build 16

smee, build 16

I made a pretty decent amount of progress on the internal overhaul today!

I have a few goals, including:

Less Restrictive Coupling

The biggest item I wanted to start addressing was the tight coupling between a large number of my classes and the static Main.main instance.

Name It What It Is

First of all, the class name Main isn't very apt. I renamed this to MapEditor. Then I renamed the instance to map_editor.

Luckily, Eclipse's refactoring features make this sort of modification nearly instantaneous. The name change actually went a long way towards easing the load on my brain when interpreting all of the code I had cranked out so far. Don't underestimate the benefits of succinct naming!

So what's this "tight coupling" and why should I want to avoid it?

The Problem

The crux of the matter is that I need to reference the various different components of the map editor all over the place. The easiest way to meet this need is to make a single public static instance of the master component, in this case my map editor class.

This approach has the benefit that you don't have to think that hard about it. However, the downside is that all of the various classes which reference this static instance are now dependent on it. They are "tightly bound" to it.

If you decide to move that instance into another class, you will have to make a lot of modifications in many places to mirror this change. This may or may not be a difficult undertaking, depending upon the quality of the code refactoring tools at your disposal.

If you decide you want to be able to edit many maps at once, automated refactoring isn't going to be of any help to you. You'll have to make a potentially large number of modifications, since this design is also tightly bound to the concept of editing one map at a time.

This is just what I want to be able to support in the future, so I decided to hunker down sooner rather than later and loosen things up a bit.

A Solution

To make the dependency less restrictive, I instead pass in an instance of MapEditor into various action classes and components, in their constructors. As before, these classes will be dependent upon a MapEditor object, but they will no longer be dependent on a specific MapEditor object.

This makes the design a little more fluid, and allows me to construct multiple MapEditor objects and pass in references for each one wherever they are needed.

Static Instance vs Infinite References

It may seem excessive to pass in an instance of MapEditor into every sub-component, just so I can query the master component's other sub-components whenever I happen to need access to them. And, well, it may be a bit! But it's a step in the right direction.

I was talking this over with a couple other people, and the Static Instance versus Infinite References did seem to be the main two options.

After thinking about it a little further, I came to the conclusion that there's probably a happy middleground somewhere, and it probably has to do with events and listeners.

Exploring Possibilities

The idiom in Java, when creating custom events, is to extend the EventObject class. You create a constructor that takes an Object as the first argument, the event's "source," and then add any further relevant data to the mix. This data is what gets packaged up and sent out to all parties who listen for these events.

For the most part, I tend to ignore the source. I have to cast it to get what I want out of it anyway, and that's mildly annoying. But it always seemed a pity to never really make use of it.

With the "static versus infinite" question buzzing around in my brain, I was boggling over how to get an instance of MapEditor into a bunch of classes for querying purposes without having to actually... you know. Pass it in. Which is all rather paradoxical, really!

The actual annoyance is less that I had to pass it in, and more that I had to maintain a reference to it at all times in my various components, thus adding an extra bit of fluff to each class that needed it.

Thinking about this further, I realized that events and listeners were the perfect way to weasel my MapEditor references into these classes without having to maintain that reference actively in each class. Since I don't tend to use the source object in each event, I can just stick a MapEditor reference in there and cast it out when I need it.

When I need access to other components, it's almost always as a result of some action being performed in the first place, which requires a response. For example, after tiles have been plotted, I need the minimap to refresh. After tiles selected, I need the Selections area to refresh, and so forth. So I just need to fire off events with a MapEditor instance as the source... Genius, man! Genius.

...Duh!

After all this amazing enlightenment, I then began to realize that if I simply use events and listeners correctly in the first place, I don't need access to components across component boundaries. Or so I suspect!

As an example, when I was first performing tile plots to the viewport, I would call repaint() on the viewport's contents (a MapPanel) directly inside the mousePressed() handler which performed each plot. This required that I have access to the MapPanel component from within the MapPanelController class, which contained the mouse handlers.

Once I moved to event-based handling of plots, the controller would simply call set_tile() on the current Map, which would in turn fire off MapRefreshEvent objects to all listeners.

The MapPanel was registered as a listener for map refresh events, and so when those events were triggered, MapPanel would call repaint on itself. This is significant because a dependency has been removed-- MapPanelController no longer depends upon knowledge of MapPanel. MapPanel now simply merrily repaints itself as it intercepts map refresh events.

It may seem fairly trivial that a controller have knowledge of the component it is directly controlling. So let's take the example one step further: the minimap.

The Minimap Should Listen Up

When I implemented the minimap, I again reverted back to the habit of coding the minimap refresh code directly into the MapPanelController, at the end of mouseReleased() handler-- this made the minimap refresh after the mouse was released following each plotting "stroke."

However, this of course then made the MapPanelController class dependenct upon the Minimap class. And I was using that single static instance to get at it:

Main.main.minimap().minimap_panel().repaint();

With my new design, I would end up with a reference to MapEditor hanging out in MapPanelController, changing it to this:

map_editor.minimap().minimap_panel().repaint();

But the fact of the matter is, registering the minimap to listen for the appropriate refresh events negates the need for MapPanelController to have knowledge of MapEditor.

The controller should be concerned with performing operations on a model. For example, plotting tiles on a map. The map is the model. The controller should not necessarily be concerned with how all other components respond to that task, such as by refreshing or making new calculations, etc.

The controller shouldn't need to worry about all that. Don't stress it out, man! Let the controller chill. It'll do its own thing. Everybody else can watch (listen) and react accordingly.

Lesson Learned

So, as you can see, shifting from tightly bound actions and reactions to a more event-based design removes a lot of complexity. Suddenly all of the classes which issue events don't need to know about every other class that needs to act on those events.

It seems as though this obviates the entire issues involving the need for references to MapEditor everywhere. I'll let you know how everything continues to develop!

Yet More Events & Listeners

One of the new events I created for the minimap was the PlottingStrokeFinishedEvent. It's a mouthful, I know. But I think it fairly well describes its operation.

One of the issues with shifting the minimap refreshes over to an event-based occurrence was that I couldn't just listen for map refresh events. The reason is that repainting the minimap is relatively costly, since I am basically using the same renderer as the main viewport, but at a much smaller scale. This means every single tile on every single layer in the map is iterated over during each refresh of the minimap.

For larger maps, refreshing the minimap after each tile plot in the viewport would result in slow and patchy plotting. To lessen the impact, I decided to simply update the minimap only after each stroke of tiles was completed. That is, after the mouse was released.

There was no existing event to denote the end of a stroke, and obviously I didn't want to put the minimap refresh directly in the mouse handler for all of the previously discussed reasons, so PlottingStrokefinishedEvent was born!

In the mouseReleased() handler inside MapPanelController, I now call firePlottingStrokeFinished();. This method notifies all PlottingStrokeFinishedEvent listeners that a stroke was completed. I register the mnimap as such a listener, and simply call repaint() inside its plottingStrokeFinished() handler!

Other Refresh Scenarios

After the minimap was refreshing correctly after tile plots, a few other situations were left to handle. I needed to refresh it after undo and redo operations, as well as after a layer was removed or if any layers were reordered. Refreshing after a layer was added was not necessary, since new layers are always empty.

Events already existed for layer removal and reordering, ala MapLayerEvent and its listener, issued by the Map object. I made Minimap listen for these.

However, there was not an event to indicate an undo or redo operation had just completed. For these, I created UndoPerformedEvent and RedoPerformedEvent. These are two fairly non-specific event types which may only ever service the minimap refresh. But that's fine! You gotta start somewhere.

These two final event types in place, I set them to be fired off from within the undo() and redo() methods of MapEditor. Then I registered Minimap yet again as a listener for these new events.

With all of this in place, tile plots, undone and redone plots, and layer reordering were all refreshing the minimap! Gravy.

The Nuances of Mouse Plottery

One wrinkle in the code for plotting tiles was the way the undo history and mouse clicking were handled. If you held down the left mouse button to plott tiles, then accidentally tapped the right mouse button at some point, all of the tiles plotted up to that point would vanish into the ether, never to be undone.

This was because my code would create a new CompositeEdit every time the mousePressed() handler was called. If you'll remember, the CompositeEdit class accumulates Edits, which are currently tile plot edits only. As you drag your mouse when plotting, individual tile plot edits for each grid cell dragged over are combined into the composite, and then "committed" to the undo history when you release the mouse button.

Of course, this completely disregards the fact that you can have more than one button pressed at a time. Instead of creating a new composite edit at every mouse press and comitting it after every mouse release, I needed to create a new composite only after the first mouse button was pressed and commit it only after the last mouse button was released.

UUDD LRLR BA START

Additionally, I wanted the plotting behavior to allow you to hold down the left mouse button, then purposely tap or sustain the middle or right mouse buttons for plotting other tiles, then release them and continue plotting with the initial tile. This would all be one discrete undoable operation. It sounds fairly simple, but it actually requires a little bit of elbow grease!

Long story short, I had to keep track of each type of tile associated with each button, and keep track of which one was pressed most recently when deciding which tile to plot with when multiple buttons were pressed. The most complex scenario would, for example, be pressing the left button, then the middle, then the right, then releasing the right-- you would expect the tile associated with the middle button to be the actively plotting tile at that point. And it takes a little work!

I used timestamps and an array to make it as easy as possible. The array let me loop over everything to compare timestamps without having to do a bunch of crazy conditional statements that juggle three separate variables.

That Is All

It may have been a silly amount of trouble to go to for a scenario that may hardly ever happen, but all this fancydancin' makes tile plotting that one extra bit shiny. And with enough shininess you may outshine all other shiners!

Failing that, if nobody appreciates all your hard work you can always pop'em in the gorram mouth!