2
\$\begingroup\$

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?
\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – Gareth Rees Apr 14 '15 at 10:06
  • \$\begingroup\$ @GarethRees Ah, that would explain my bug. Oops. Thanks for pointing out. \$\endgroup\$ – alexwlchan Apr 14 '15 at 11:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.