Generating realistic terrain data

Question

I have some fascination with terrain generators, so I whipped up this generator in Python. The way it works is pretty simple, but I figure that I should give an explanation for those who don't want to try and figure it out.

1. The initial 2D terrain array is generated with a provided width and height, and then filled entirely with NoneTypes.
2. Then the array is iterated over a certain amount of times and "seeds" are placed in random positions. The value of these "seeds" is between a maximum height and a minimum height.
3. After this happens, the array is then iterated over another given amount of times, and on each iteration, if a tile is found that isn't a NoneType, it's top, bottom, left and right neighbor values are determined by this: tile_value + randint(min_change, max_change). This process then repeats until the iteration ends.
4. The array is iterated over one last time and replaces any remaining NoneTypes with the minimum height value.

---

"""
A basic library containing a class for
generating basic terrain data.
"""
from random import randint
class Terrain(object):
 """
 Class for generating "realistic" looking terrain.
 """
 def __init__(self, max_height, min_height, max_change, min_change, width, height, iterations, seed_iterations):
 self.max_height = max_height
 self.min_height = min_height
 self.max_change = max_change
 self.min_change = min_change
 self.width = width
 self.height = height
 self.iterations = iterations
 self.seed_iterations = seed_iterations
 self.terrain = [[None for _ in range(self.width)] for _ in range(self.height)]
 def _seed_terrain(self):
 """
 Seed a random integer value at a random location. 
 """
 for _ in range(self.seed_iterations):
 self.terrain[randint(0, self.height-1)][randint(0, self.width-1)] = randint(self.min_height+1, self.max_height-1)
 def _iterate_terrain(self):
 """
 Loop through the terrain and change each
 nearby tile to a value slightly larger or 
 smaller than the current tile value.
 """
 for _ in range(self.iterations):
 for row in self.terrain:
 for tile in row:
 current_row_index = self.terrain.index(row)
 current_tile_index = row.index(tile)
 upper_index = self.terrain.index(row)+1
 lower_index = self.terrain.index(row)-1
 right_index = row.index(tile)+1
 left_index = row.index(tile)-1
 if tile != None:
 try:
 self.terrain[upper_index][current_tile_index] = tile + randint(self.min_change, self.max_change) if tile >= self.min_height and tile <= self.max_height else tile
 self.terrain[lower_index][current_tile_index] = tile + randint(self.min_change, self.max_change) if tile >= self.min_height and tile <= self.max_height else tile
 self.terrain[current_row_index][right_index] = tile + randint(self.min_change, self.max_change) if tile >= self.min_height and tile <= self.max_height else tile
 self.terrain[current_row_index][left_index] = tile + randint(self.min_change, self.max_change) if tile >= self.min_height and tile <= self.max_height else tile
 except IndexError:
 continue
 def _final_pass(self):
 """
 Check to make sure that there are no
 NoneTypes left in the array, and if
 there are, change them to the minimum
 tile value.
 """
 for row in self.terrain:
 for tile in row:
 if tile == None:
 self.terrain[self.terrain.index(row)][row.index(tile)] = self.min_height-1
 def generate(self):
 """
 Generate the final terrain.
 """
 self._seed_terrain()
 self._iterate_terrain()
 self._final_pass()
 def render(self, spacing=" "):
 """
 Render the completed terrain.
 """
 for row in self.terrain:
 print spacing.join([str(tile) for tile in row])
 def return_terrain(self):
 return self.terrain

---

Few questions that you can answer about my code if you want.

1. Is this code Pythonic?
2. How can it be made faster? Can it be made Faster?
3. Is there a different way I should implement this?
4. How can I shorten some lines?

Answer 1

A few quick suggestions:

• Line 11: lines are supposed to be a maximum of 79 characters wide (PEP 8, Maximum Line Length). Just add a line break or two to the list of arguments.

• (削除) Line 20: rather than initialising your terrain with a pair of list comprehensions, you can do it like this:

  self.terrain = [[None] * self.width] * self.height
  

  (削除ここまで)

  This introduced a bug – see Gareth Rees’s comment for why you shouldn’t do this.

• Line 26: to break this line up, I’d create some temporary variables before you put it all together. For example:

  for _ in range(self.seed_iterations):
   height = randint(0, self.height-1)
   width = randint(0, self.width-1)
   value = randint(self.min_height+1, self.max_height-1)
   self.terrain[height][width] = value
  

• Lines 41–44: put spaces around binary operators. This is suggested in the Python coding standard, PEP 8.

• Lines 46, 64, 80: when comparing to None, it’s generally preferable to use x is None or x is not None instead of the standard boolean ==/!= operators.

• Lines 48–51: rather than constructing long ternary-operator-like statements, run the if statement once, and then execute the statements in succession. This will be cleaner and easier to read.

  Note that you can tidy up the condition slightly;

  if self.min_height <= tile <= self.max_height:
  

  To wrap it sensibly, either the nested for loops need to be tidied up, or you need to prune your variable names. I went with the latter; here was a first attempt:

  curr_row_idx = self.terrain.index(row)
  curr_tile_idx = row.index(tile)
  upper_idx = self.terrain.index(row) + 1
  lower_idx = self.terrain.index(row) - 1
  right_idx = row.index(tile) + 1
  left_idx = row.index(tile) - 1
  def randchange():
   return randint(self.min_change, self.max_change)
  if tile is not None:
   try:
   if self.min_height <= tile <= self.max_height:
   upper_curr = tile + randchange()
   lower_curr = tile + randchange()
   curr_right = tile + randchange()
   curr_left = tile + randchange() 
   else:
   upper_curr = tile
   lower_curr = tile
   curr_right = tile
   curr_left = tile
   self.terrain[upper_idx][curr_tile_idx] = upper_curr
   self.terrain[lower_idx][curr_tile_idx] = lower_curr
   self.terrain[curr_row_idx][right_idx] = curr_right
   self.terrain[curr_row_idx][left_idx] = curr_left 
   except IndexError:
   continue
  

  Note also that if I get an IndexError while setting the first terrain value, none of the others will be set (even if they are valid terrain points). You should probably fix that.

• You might want to add some checks to make sure that the user’s inputs make sense. Because I just punched in some numbers at random, this is what I tried first:

  x = Terrain(5, 10, 1, 2, 10, 10, 5, 50)
  x.generate()
  x.render()
  

  My max height (5) is less than the min height (10), which is clearly ridiculous, but your code accepts it anyway. Until it spat out an error message:

  Traceback (most recent call last):
   File "terrain.py", line 103, in <module>
   x.generate()
   File "terrain.py", line 88, in generate
   self._seed_terrain()
   File "terrain.py", line 28, in _seed_terrain
   self.terrain[randint(0, self.height - 1)][randint(0, self.width - 1)] = randint(self.min_height + 1, self.max_height - 1)
   File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/random.py", line 240, in randint
   return self.randrange(a, b+1)
   File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/random.py", line 216, in randrange
   raise ValueError, "empty range for randrange() (%d,%d, %d)" % (istart, istop, width)
  ValueError: empty range for randrange() (11,5, -6)
  

  The bug occurs on line 27, in randint(self.min_height + 1, self.max_height - 1), where silly values get passed to randint().

  (削除) Note that even when I supply what seem to be valid arguments:

  x = Terrain(20, 5, 10, 1, 10, 10, 20, 500)
  x.generate()
  x.render()
  

  I can’t seem to get anything except a flat landscape at (max_height + min_height).

  I suspect this is just me misunderstanding your program, but if not, a few comments on the Terrain class to explain how it should be used might be helpful. (削除ここまで) This is a bug I introduced myself. Oops.

• Gareth Rees: It's not a good idea to use [[None] * self.width] * self.height — this results in each row being the same list. See this question on Stack Overflow.
• alexwlchan: @GarethRees Ah, that would explain my bug. Oops. Thanks for pointing out.