RMXP ANTI-PATTERNS

Bad programming habits that you'll pick up from RMXP

  • Gibmaker
  • 06/14/2015 12:28 AM
  • 4258 views
Years ago, I wasted my university tuition on an theatre studies degree, and my life sucked. But then, thanks to doodling around with the script editor in RPG Maker XP, I gradually learned about object-oriented programming, to the point that I went back to school for computers and I'm now working as a Ruby on Rails developer. My life no longer sucks, and it's all thanks to RPG Maker. And this website. Thank you, rpgmaker.net.

That said, the more experience I get with the Ruby language, the more appalled I become as I revisit the RMXP code base. While RMXP is perfectly functional and bug-free, the code is full of bad programming practices (sometimes called "anti-patterns" or "code smells") which I unfortunately adopted myself in my early programming days, simply because I didn't know any better.

In some places, the designers seem to have used very primitive practices for the purpose of making the code easier for Ruby-newbies to understand; for example, the Ruby community considers "for" loops to be deprecated compared to the Array.each method, but the RMXP script is full of them, perhaps because they are a little easier to comprehend. Other helpful Array functions such as #map and #select are completely ignored. This is ultimately bad for newbies, because without any other examples, an inexperienced programmer will never learn how to use Ruby to its full potential.

Experienced or not, a programmer who extends the RMXP script will inevitably start copying the style and quirks of the existing script. Indeed, I've even seen code repositories that require you to make your submissions look exactly like the base code. This is probably for the best, but if you ever wish to take your new-found Ruby knowledge elsewhere, it's important to be aware of the assumptions you may have made based on your exposure to RMXP.

Obviously my experience is primarily with RPG Maker XP, but many of these design practices persist in RPG Maker VX as well.

With this in mind, here are four terrible RXMP anti-patterns:

1) Game_Temp

Global variables are variables that start with $, and can be accessed from anywhere in the program. While global accessibility is very convenient, it should be used sparingly. When you make a global variable, you are asserting that this variable is relevant everywhere in the program, and furthermore, that there should only ever be one unique instance of it.

Most of the major entities in RMXP are saved in global variables, such as $game_map and $game_player. It certainly makes it easy to manipulate these entities in Script event commands (which was perhaps part of the rationalization to make them global in the first place). Interpreter commands can also easily reach out to manipulate any part of the game. However, using global variables irresponsibly for the sake of convenience will creep up and destroy you like a bad habit.

Particular attention should be given to the monstrosity known as Game_Temp. This class basically provides system-wide flags that can be accessed from literally anywhere. The existence of such a class suggests that the developers didn't know how to get information from point A to point B, so they created a general-purpose dumping ground, with the only rule being that $game_temp "is not included with save data".

Except for small groups of attributes, nothing thrown together in Game_Temp is actually related to each other. I'm confident that everything in this class could have been instead worked into other classes that actually use the information. The scene-changing flags could have been worked into the Scene classes; $scene itself is globally accessible anyway. The player-transferring attributes could be extracted into a separate class responsible for teleporting the player. The message- and choice-related attributes could be part of Window_Message or Interpreter, since those are the only classes where they are relevant.

Proper scoping of classes is a critical part of Ruby programming. If you find yourself stuck because some class relies on another and you have no clear way to pass the appropriate information, it means you have to rethink your program and restructure the classes.

In one of my projects that eventually collapsed under its own weight because of bad programming, you'd better believe that I put anything and everything into Game_Temp. The simple existence of this class tricked me into thinking that a globally-accessible junk drawer was an acceptable design decision. Please don't ever do this.

2) Comments everywhere and I mean everywhere

Ruby has the capacity to be a "self-documenting" language. What that means is that by using clean style and suitable choices of method/variable names, the purpose of a block of code is evident without having to write comments at all.

In RPG Maker XP's code, almost every single line seems to need a comment:
# Interpreter 1 - line 110
# Initialize loop count
@loop_count = 0
# Loop
loop do
  # Add 1 to loop count
  @loop_count += 1
  # If 100 event commands ran
  if @loop_count > 100
    # Call Graphics.update for freeze prevention
    Graphics.update
    @loop_count = 0
  end
  # If map is different than event startup time
  if $game_map.map_id != @map_id
    # Change event ID to 0
    @event_id = 0
  end
  # If a child interpreter exists
  if @child_interpreter != nil
    # Update child interpreter
    @child_interpreter.update

I'll temper my criticism once again by saying that maybe the developers anticipated that newbie programmers would be inspecting this code, and so they overloaded their code with explanation; but really, what extra clarification does the comment # Update child interpreter give us when the actual code is @child_interpreter.update? Why in God's name did we need the comment # Loop to figure out that the next line is the start of a loop?

I think this over-abundance of comments is partly a side-effect of a bizarre style choice in the RMXP script, which is to have absolutely no blank lines. Just like with the text in this article, blank lines are used to separate code into paragraphs, which improves readability. For some reason, the RMXP coders decided that blank lines weren't allowed in their script, so in many places I suspect that these needless comments were a way of spacing out their code. Here's how that block above would look with all the pointless comments removed:
@loop_count = 0
loop do
  @loop_count += 1
  if @loop_count > 100
    # Call Graphics.update for freeze prevention
    Graphics.update
    @loop_count = 0
  end
  if $game_map.map_id != @map_id
    @event_id = 0
  end
  if @child_interpreter != nil
    @child_interpreter.update

It looks rather crowded. But with some judicious use of whitespace ...
@loop_count = 0
loop do
  @loop_count += 1

  if @loop_count > 100
    # Call Graphics.update for freeze prevention
    Graphics.update
    @loop_count = 0
  end

  if $game_map.map_id != @map_id
    @event_id = 0
  end

  if @child_interpreter != nil
    @child_interpreter.update

... it becomes much more readable. Each paragraph encapsulates a single intension, and by looking at the code itself you can figure out what each paragraph is meant to accomplish. The one comment that I left in provides context not immediately apparent in the code itself; that if you don't call Graphics.update frequently, the game will think your code is hanging and crash.

I'm willing to bet that most custom code for RMXP has conformed to the same over-use of comments and aversion to blank lines. I've done it myself and I continue to do it when writing for XP, simply because of the natural desire to make my code fit in with what is all ready there. Just don't let this style become a permanent habit that you take to other projects outside of RMXP, because it's lousy style!

Of course, the worst possible use of comments is to compensate for unclear methods and variables. Which brings us to ...

3) Designating non-numeric entities with numbers

If you've ever peeked at the Scene_Battle implementation, you might have seen this:
# Scene_Battle 1 - line 309
# Branch according to phase
case @phase
when 1  # pre-battle phase
  update_phase1
when 2  # party command phase
  update_phase2
when 3  # actor command phase
  update_phase3
when 4  # main phase
  update_phase4
when 5  # after battle phase
  update_phase5
end

This smacks of high-school programming class. As you can see, a number has been assigned to each phase of the battle cycle; @phase holds an integer which corresponds to a certain phase based on this list.

Here's the problem: What if you discover that you need to create a new phase between phases 2 and 3?

The easiest solution would be to assign the new phase a value of 6, and establish a flow of ... -> 2 -> 6 -> 3 -> ... but that breaks the implicit assumption that the phases should advance in order of number. Too many changes like this and you'll have a messy and unpredictable battle cycle.

Another solution is to painstakingly change all references to "phase 3" into "phase 4", so that the new phase can be phase 3 -- and then subsequently change phase 4 into phase 5, and phase 5 into phase 6. But, miss one reference and you'll have introduced a drastic bug into the system.

The issue at the heart of this problem is: why are we using numbers to identify these phases in the first place?

The practice of using numbers to reference a collection of non-numeric entities has been debunked for decades. Even C++, which is as old as I am, has the Enumerable data type which remedies this problem. There is nothing inherently "4-ish" about the main phase, so it shouldn't be so tightly coupled with the number 4.

Instead of an integer, @phase should hold a descriptive symbol, like :pre_battle, :party_command, etc, and the battle scene can be dispatched just as easily using these symbols. I find it baffling that even though RGSS includes a perfectly functional Symbol class, symbols are never used for this purpose anywhere in the system.

Even more baffling is the choice of method names for processing each phase. Why they chose update_phase4 and relied on a comment to clarify its function, as opposed to simply naming the method update_main_phase, is a mystery for the ages. Once again they're suggesting that the value of 4 is somehow integral to the function of the main phase.

Here is a better implementation:
case @phase
when :pre_battle
  update_pre_battle
when :party_command
  update_party_command
when :actor_command
  update_actor_command
when :main_phase
  update_main_phase
when :after_battle
  update_after_battle
end

... or for bonus points:
send :"update_#{@phase}"


4) Fat classes

The Interpreter class is 2378 lines long, which is too long. Even accounting for the fact that half of those lines are pointless comments as described in section 2, it's still too much.

Almost all of Interpreter's definition is taken up with a series of methods with names like #command_x. Giving a group of methods a common prefix, such as the command_x series or the move_type_x series in the Game_Character class, is a clue that those methods should be extracted into a separate helper class.

Instead of:
# Interpreter 4 - line 206
#--------------------------------------------------------------------------
# * Change Gold
#--------------------------------------------------------------------------
def command_125
  # Get value to operate
  value = operate_value(@parameters[0], @parameters[1], @parameters[2])
  # Increase / decrease amount of gold
  $game_party.gain_gold(value)
  # Continue
  return true
end

... we would have, on separate pages:
class Interpreter
  class Command
    def initialize(parameters)
      @parameters = parameters
    end

    def operate_value
      # Do something with @parameters
    end

    def execute
      # Extend with command logic
      true
    end
  end
end

class Interpreter
  class GetGold < Command
    def execute
      $game_party.gain_gold(operate_value)
      true
    end
  end
end


Instead of calling the abstrucely-named command_125 (there's that number-based naming convention again), an Interpreter would create an instance of the appropriate subclass of Command (GetGold in this case) and call execute on it.

The default RMXP script doesn't make nearly enough use of helper classes, and thus does not encourage newbie programmers to adopt the same practice. Instead, we have classes with dozens of methods, split up into several pages because they're too fat to fit in just one.

The problem with this anti-pattern is that it's partly enforced by a limitation of the script editor: no directories.

In RPG Maker, all your code is accessed through the built-in editor and saved to the Scripts.rxdata file. In a "real" Ruby project, your code would be saved in plain text files, directly on your OS's file system. The RMXP script demonstrates good practice by placing each class definition on its own "page", which is comparable to having each in a separate file, but it's impossible to create directories and sub-directories like you could on a real file system. Directories form a tree structure with branches that can be expanded or hidden as desired, making it much easier to organize a large number of files. The RGSS editor has no such feature.

Effectively, all the different classes in RMXP have been piled together into a single directory, and scripters must then rely on those class name prefixes like Game_, Window_ and Sprite_ to group similar classes together. (Remember, using prefixes to form de-facto groups is a clue that you should be making "real" groups with namespaces.) This discourages deep nesting, simply because there is no easy way to organize a large number of deeply-nested classes.

If it were possible to create directories in the script editor, the best setup for RMXP would be something like this:


interpreter.rb would contain only behaviour integral to the Interpreter class itself, which actually takes up only a few hundred of the 2378 lines. The file name naturally associates it with the similarly-named directory. Inside the directory, which we can show or hide as needed, are all of Interpreter's helper classes, in this case command definitions:


Without directories, if there were actually a separate class for each of the 95 Interpreter commands, that would be 95 more classes piled into the general directory with everything else!

(Of course, you can always define more than one class in a single view in the editor, but this is a work-around, and another bad practice.)

In short, many of RMXP's classes such as Interpreter and Game_Character are responsible for far too much behaviour, which should have been offloaded into helper classes; however, limitations in the editor would make it difficult to manage a large number of classes, so users are implicitly encouraged to use a small number of fat classes, as opposed to a larger number of lean, specialized classes, which would be preferable.

But it's not all bad.

Of course, RMXP code demonstrates some excellent design principles as well. Appreciating good things is not as fun as complaining about bad things, but here are some in brief:

  • The SpriteSet classes excellently demonstrate the principle of separating the view of data from the data itself. It would have been very easy to try and make Game_Map generate its own sprites, but this would require somehow turning them all on and off depending on whether the map is visible. Extracting all the sprite generation into a separate class that observes Game_Map makes it easy for the map data to persist even when the sprites don't.


  • Every type of window is specified in a separate class, even the most rudimentary ones such as Window_SkillStatus. As tempting as it might be to create an ad-hoc window within the Scene_Skill class, you would be effectively defining the window's behaviour within another class that isn't a window. It's ultimately better to give every window its own definition so it can govern itself.


So RMXP is by no means badly-made; it's perfectly functional and it doesn't crash like VX. (Don't make a Battle Call the last command in a VX event.)

You might say that XP does its job just well enough; it's refined enough so that it works, and then makes use of shortcuts and conveniences to be accessible to newbie Rubyists. Just be aware of these shortcomings if you ever need to use your RMXP experience to claw your way out of a dead end coffee-serving job.

Posts

Pages: 1
I have never looked at the RMXP codebase (or VX or Ace) because I am a lazy fuck, but I admit I got a chuckle from the Loop Do Loop comment.
Gibmaker
I hate RPG Maker because of what it has done to me
9274
Well, looking at the RMXP codebase is a black hole that consumes your life, and you have a wife and kids to think about.
Pages: 1