Refactoring Part 2


Now I’m finally going to talk about specifics.

In the last post I mentioned attempting to refactor the build panel into both an in-game construction device and a level editor device.  It turns out one of the issues that had been bothering me for a while was in the bridge display.  Every time you place a bridge a new random one takes it’s place.  I put this in the build panel, because it was the obvious place to do so, but it bugged me that there was so much code in the build panel for just displaying icons and now it also had to be responsible for managing bridge replacement when all the other icons for other units and structures just stayed there (for the most part).

The level editor didn’t need this whole bridge mechanism, it just needed a panel that would show icons and allow you to drag them onto the field.  I finally had a reason to split out the bridge code.  It was beautiful.  At least mostly, I still haven’t entirely pulled out old assumptions.  I tore all the code that handled bridges into a separate file.  Because it was well separated I could remove a number of if checks and actually shrink the code somewhat because now I could assume that I was only dealing with bridges in that code.

I’ve been talking about assumptions a lot.  Back in school, it would have been called “invariants”.  Invariants are great, it means you *know* a certain thing is going to be true (or false) at a given time and so there’s no need to check.  Good coding practice will tell you to check anyway, and that’s true, but now you replace an “if this do this” to “assume this or throw an exception” and if your code never throws that exception during testing you did well.

So assumptions aren’t bad, they’ll make the code more efficient in the long run, but may also make it less general.  There’s a choice to make.  I could have written the level editor without reusing the build panel, but I was predicting when I made the choice that there would be a lot of repeated code and not much efficiency to be had.  As I said, it really was the same thing.  But in general, if there’s that choice to be made, I’ll usually pick the one with the smaller code, because smaller code looks like it was easier to do.

It turns out there are another couple bonuses which might be had from refactoring, but which I haven’t yet attained.  I’ll get to the bonuses in a few paragraphs (see that foreshadowing). The stumbling block I’m sitting on is that the bridge piece when placed notifies the build panel that it has been placed so that the build panel knows to randomly select a new piece of bridge to put in there.  Once I split out that bridge control code, the bridge piece was notifying the wrong system.

My assumption, for efficiency was to have the bridge piece just grab the build panel, because every draggable grabs the build panel.  In fact, it’s in the base class.  The build panel is supposed to know enough about all construction to darken the icon while any of its objects is being placed and not allow you to click a new one until you either place or cancel the last one.  So each object which is in the build panel needs to tell it when it’s placed so that the build panel knows it can relinquish the lock on the display.

I could use a publish subscribe model as I mentioned previously for the pathfinder, notifying any subscribers when the bridge is placed.  This would allow both the build panel and the bridge controller to get notifications and each do the appropriate thing with a good split. But this is a larger chunk of code that I’d have to write and in reality, it’s only this one case.  So having a full publish/subscribe is a bit over the top and a little to much architecting.  At least it feels that way…

I mentioned bonuses that I haven’t yet attained.  I haven’t attained them because currently the build panel still assumes there are icons for bridges at the top and still in the build panel.   The build panel then has to call into functions in the bridge controller to let it know when it gets notification from a bridge piece.  There’s a level of pointless indirection in that call.  The first bonus then would be that the level editor uses that space to place icons like “save/load” and “quit” as well as other editor specific icons like the player’s priest (his avatar in the game) which sets his starting location.  But because the build panel currently assumes bridges in those locations, the save/load button becomes “bridge slot 1″ (by a function call, SetBridgeIcon (1, “Save Icon”) normally used by the bridge controller when a new bridge is ready to replace the icon) and the priest icon is “bridge slot 6″.  Just arbitrary bridge locations instead of arbitrary icon locations.  And it looks silly.

The other bonus I would receive by fixing this is that there is a bug in Unity with the gui elements.  Because I’m changing the icons, on these buttons it redraws them.  For some reason, Unity’s gui doesn’t remove old draw elements, so as you place bridges, the draw calls goes steadily upwards.  Likely it’s drawing nothing, but a draw call is a call out to the graphics card and bridging is a very common thing so much that thousands of bridges will be placed during a game.  1000′s of draw calls will cripple the graphics performance.

By just establishing a graphical placeholder in the build panel and letting either the editor or the bridge controller actually draw in the icons they  need I can remove the sillyness in the editor of setting “Bridge Icons” and in the bridge controller I can optimize out the GUI bug.

So I still haven’t arrived at a solution.  I may just go ahead and write a publish/subscribe mechanism with as many assumptions as I can put in to keep the code as small as possible.  And maybe I’ll figure out a way to refactor it into something smaller later.

  1. No comments yet.
(will not be published)
*

Switch to our mobile site