added support for removing tiles that shouldn't exist with --check-tiles

--check-tiles is now activated by shrinking maps
better detection for --forcerender situtations
Also cleaned up some docs and comments
Moved tiledir creation to TileSet constructor

docs/running.rst

@@ -116,34 +116,57 @@ only have to use once-in-a-while.
 .. cmdoption:: --forcerender
 
     Forces The Overviewer to re-render every tile regardless of whether it
-    thinks it needs updating or not. This is similar to deleting your output
+    thinks it needs updating or not. It does no tile mtime checks, nor does it
-    directory and rendering from scratch.
+    check any last modification time of the world. It unconditionally renders
+    every tile that exists.
 
-    This is the default mode for first-time renders. This option overrides
+    The caveat with this option is that it does *no* checks, period. Meaning it
+    will not detect tiles that do exist, but shouldn't (this can happen if your
+    world shrinks for some reason. For that specific case,
+    :option:`--check-tiles` is actually more appropriate).
+
+    This is the default mode for first-time renders. This option conflicts with
     :option:`--check-tiles` and :option:`--no-tile-checks`
 
 .. cmdoption:: --check-tiles
 
-    Forces The Overviewer to check each tile on disk and compare its
+    Forces The Overviewer to check each tile on disk and check to make sure it
-    modification time to the modification time of the part of the world that
+    is up to date. This also checks for tiles that shouldn't exist and deletes
-    tile renders. This is slightly slower than the default, but can be useful if
+    them.
-    there are some tiles that somehow got skipped.
+    
+    This is slightly slower than :option:`--no-tile-checks` due to the disk-io
+    involved in reading tile mtimes and reading the world's region file headers
+    for chunk mtimes, but can be useful if there are some tiles that somehow got
+    skipped.
+
+    The caveat with this option is that it compares the tile mtimes on disk with
+    the chunk mtimes reported by Minecraft. Thus, if the Minecraft world was not
+    updated since the last render, tiles will not be detected as needing
+    updates. This means you cannot use this option in response to a changed
+    configuration setting.
 
     This option is the default when The Overviewer detects the last render was
-    interrupted midway through. This option overrides :option:`--forcerender`
+    interrupted midway through. This option conflicts with
-    and :option:`--no-tile-checks`
+    :option:`--forcerender` and :option:`--no-tile-checks`
 
 .. cmdoption:: --no-tile-checks
 
-    With this option, The Overviewer will not do any checking of tiles on disk
+    With this option, The Overviewer will determine which tiles to render by
-    to determine what tiles need updating. Instead, it will look at the time
+    looking at the saved last-render timestamp and comparing it to the
-    that the last render was performed, and render parts of the map that were
+    last-modified time of the chunks of the world. It builds a tree of tiles
-    changed since then. This is the fastest option, but could cause problems if
+    that need updating and renders only those tiles.
-    the clocks of the Minecraft server and the machine running The Overviewer
-    are not in sync.
 
-    This option is the default unless the condition for :option:`--forcerender`
+    This option does not do *any* checking of tile mtimes on disk, and thus is
-    or :option:`--check-tiles` is in effect.  This option overrides
+    the cheapest option: only rendering what needs updating while minimising
+    disk IO.
+
+    The caveat is that the *only* thing to trigger a tile update is if Minecraft
+    updates a chunk. Any other reason for needing to re-render a tile requires a
+    different mode to detect the needed update. It could also cause problems if
+    the system clock of the machine running Minecraft is not stable.
+    
+    **This option is the default** unless :option:`--forcerender` or
+    :option:`--check-tiles` is in effect.  This option conflicts with
     :option:`--forcerender` and :option:`--check-tiles`.
 
 .. cmdoption:: -p <procs>, --processes <procs>
@@ -154,6 +177,23 @@ only have to use once-in-a-while.
 
     This option can also be specified in the config file as :ref:`processes <processes>`
 
+.. cmdoption:: -v, --verbose
+
+    Activate a more verbose logging format and turn on debugging output. This
+    can be quite noisy but also gives a lot more info on what The Overviewer is
+    doing.
+
+.. cmdoption:: -q, --quiet
+
+    Turns off one level of logging for quieter output. You can specify this more
+    than once. One ``-q`` will suppress all INFO lines. Two will suppress all
+    INFO and WARNING lines. And so on for ERROR and CRITICAL log messages.
+
+    If :option:`--verbose<-v>` is given, then the first ``-q`` will counteract
+    the DEBUG lines, but not the more verbose logging format. Thus, you can
+    specify ``-v -q`` to get only INFO logs and higher (no DEBUG) but with the
+    more verbose logging format.
+
 .. _installing-textures:
 
 Installing the Textures

overviewer.py

@@ -402,8 +402,6 @@ dir but you forgot to put quotes around the directory, since it contains spaces.
 
         # create our TileSet from this RegionSet
         tileset_dir = os.path.abspath(os.path.join(destdir, render_name))
-        if not os.path.exists(tileset_dir):
-            os.mkdir(tileset_dir)
 
         # only pass to the TileSet the options it really cares about
         render['name'] = render_name # perhaps a hack. This is stored here for the asset manager

overviewer_core/tileset.py

@@ -178,10 +178,7 @@ class TileSet(object):
         This class does nothing with it except pass it through.
 
         outputdir is the absolute path to the tile output directory where the
-        tiles are saved. It is assumed to exist already.
+        tiles are saved. It is created if it doesn't exist
-        TODO: This should probably be relative to the asset manager's output
-        directory to avoid redundancy.
-
         
         Current valid options for the options dictionary are shown below. All
         the options must be specified unless they are not relevant. If the
@@ -211,12 +208,15 @@ class TileSet(object):
                 partial interrupted render left off.
 
             1
-                For render-tiles, render all whose chunks have an mtime greater
+                "check-tiles" mode. For render-tiles, render all whose chunks
-                than the mtime of the tile on disk, and their upper-tile
+                have an mtime greater than the mtime of the tile on disk, and
-                ancestors.
+                their upper-tile ancestors.
                 
                 Also check all other upper-tiles and render any that have
                 children with more rencent mtimes than itself.
+
+                Also remove tiles and directory trees that do exist but
+                shouldn't.
                 
                 This is slower due to stat calls to determine tile mtimes, but
                 safe if the last render was interrupted.
@@ -270,6 +270,7 @@ class TileSet(object):
         self.regionset = regionsetobj
         self.am = assetmanagerobj
         self.textures = texturesobj
+        self.outputdir = os.path.abspath(outputdir)
 
         config = self.am.get_tileset_config(self.options.get("name"))
         self.config = config
@@ -277,17 +278,42 @@ class TileSet(object):
         self.last_rendertime = config.get('last_rendertime', 0)
 
         if "renderchecks" not in self.options:
+            # renderchecks was not given, this indicates it was not specified
+            # in either the config file or the command line. The following code
+            # attempts to detect the most appropriate mode
             if not config:
-                # No persistent config? This is a full render then.
+                # No persistent config?
+                if os.path.exists(self.outputdir):
+                    # Somehow there's no config but the output dir DOES exist.
+                    # That's strange!
+                    logging.warning(
+                        "For render '%s' I couldn't find any persistent config, "
+                        "but I did find my tile directory already exists. This "
+                        "shouldn't normally happen, something may be up, but I "
+                        "think I can continue...", self.options['name'])
+                    logging.info("Switching to --check-tiles mode")
+                    self.options['renderchecks'] = 1
+                else:
+                    # This is the typical code path for an initial render, make
+                    # this a "forcerender"
+                    self.options['renderchecks'] = 2
+                    logging.debug("This is the first time rendering %s. Doing" +
+                            " a full-render",
+                            self.options['name'])
+            elif not os.path.exists(self.outputdir):
+                # Somehow the outputdir got erased but the metadata file is
+                # still there. That's strange!
+                logging.warning(
+                        "This is strange. There is metadata for render '%s' but "
+                        "the output directory is missing. This shouldn't "
+                        "normally happen. I guess we have no choice but to do a "
+                        "--forcerender", self.options['name'])
                 self.options['renderchecks'] = 2
-                logging.debug("This is the first time rendering %s. Doing" +
-                        " a full-render",
-                        self.options['name'])
             elif config.get("render_in_progress", False):
                 # The last render must have been interrupted. The default should be
-                # 1 then, not 0
+                # a check-tiles render then
                 logging.warning(
-                        "The last render for %s didn't finish. I'll be " +
+                        "The last render for '%s' didn't finish. I'll be " +
                         "scanning all the tiles to make sure everything's up "+
                         "to date.",
                         self.options['name'],
@@ -304,9 +330,15 @@ class TileSet(object):
                         )
                 self.options['renderchecks'] = 0
 
-        # Throughout the class, self.outputdir is an absolute path to the
+        if not os.path.exists(self.outputdir):
-        # directory where we output tiles. It is assumed to exist.
+            if self.options['renderchecks'] != 2:
-        self.outputdir = os.path.abspath(outputdir)
+                logging.warning(
+                "The tile directory didn't exist, but you have specified "
+                "explicitly not to do a --fullrender (which is the default for "
+                "this situation). I'm overriding your decision and setting "
+                "--fullrender for just this run")
+                self.options['rednerchecks'] = 2
+            os.mkdir(self.outputdir)
 
         # Set the image format according to the options
         if self.options['imgformat'] == 'png':
@@ -375,6 +407,7 @@ class TileSet(object):
         This method returns an iterator over (obj, [dependencies, ...])
         """
 
+        # The following block of code implementes the changelist functionality.
         fd = self.options.get("changelist", None)
         if fd:
             logging.debug("Changelist activated for %s (fileno %s)", self, fd)
@@ -492,7 +525,6 @@ class TileSet(object):
 
         return d
 
-
     def _find_chunk_range(self):
         """Finds the chunk range in rows/columns and stores them in
         self.minrow, self.maxrow, self.mincol, self.maxcol
@@ -569,6 +601,11 @@ class TileSet(object):
                 logging.warning("Your map seems to have shrunk. Did you delete some chunks? No problem. Re-arranging tiles, just a sec...")
                 for _ in xrange(curdepth - self.treedepth):
                     self._decrease_depth()
+                logging.info(
+                        "There done. I'm switching to --check-tiles mode for "
+                        "this one render. This will make sure any old tiles that "
+                        "should no longer exist are deleted.")
+                self.options['renderchecks'] = 1
 
     def _increase_depth(self):
         """Moves existing tiles into place for a larger tree"""
@@ -928,11 +965,17 @@ class TileSet(object):
             os.utime(tmppath, (max_chunk_mtime, max_chunk_mtime))
 
     def _iterate_and_check_tiles(self, path):
-        """A generator function over all tiles that need rendering in the
+        """A generator function over all tiles that should exist in the subtree
-        subtree identified by path. This yields, in order, all tiles that need
+        identified by path. This yields, in order, all tiles that need
         rendering in a post-traversal order, including this node itself.
 
-        This method actually yields tuples in this form:
+        This method takes one parameter:
+
+        path
+            The path of a tile that should exist
+
+
+        This method yields tuples in this form:
             (path, mtime, needs_rendering)
         path
             is the path tuple of the tile that needs rendering
@@ -945,8 +988,9 @@ class TileSet(object):
             is a boolean indicating this tile does in fact need rendering.
 
         (Since this is a recursive generator, tiles that don't need rendering
-        are not propagated all the way out of the recursive stack, but the
+        are not propagated all the way out of the recursive stack, but are
-        immediate parent call needs to know its mtime)
+        still yielded to the immediate parent because it needs to know its
+        childs' mtimes)
 
         """
         if len(path) == self.treedepth:
@@ -967,7 +1011,16 @@ class TileSet(object):
                 # If a tile has been modified more recently than any of its
                 # chunks, then this could indicate a potential issue with
                 # this or future renders.
-                logging.warning("I found a tile with a more recent modification time than any of its chunks. This can happen when a tile has been modified with an outside program, or by a copy utility that doesn't preserve mtimes. Overviewer uses the filesystem's mtimes to determine which tiles need rendering and which don't, so it's important to preserve the mtimes Overviewer sets. Please see our FAQ page on docs.overviewer.org or ask us in IRC for more information")
+                logging.warning(
+                        "I found a tile with a more recent modification time "
+                        "than any of its chunks. This can happen when a tile has "
+                        "been modified with an outside program, or by a copy "
+                        "utility that doesn't preserve mtimes. Overviewer uses "
+                        "the filesystem's mtimes to determine which tiles need "
+                        "rendering and which don't, so it's important to "
+                        "preserve the mtimes Overviewer sets. Please see our FAQ "
+                        "page on docs.overviewer.org or ask us in IRC for more "
+                        "information")
                 logging.warning("Tile was: %s", imgpath)
 
             if max_chunk_mtime > tile_mtime:
@@ -975,20 +1028,23 @@ class TileSet(object):
                 yield (path, None, True)
             else:
                 # This doesn't need rendering. Return mtime to parent in case
-                # they do need to render
+                # its mtime is less, indicating the parent DOES need a render
                 yield path, max_chunk_mtime, False
         
         else:
             # A composite-tile.
-            # First, recurse to each of our children
             render_me = False
             max_child_mtime = 0
+
+            # First, recurse to each of our children
             for childnum in xrange(4):
                 childpath = path + (childnum,)
 
                 # Check if this sub-tree should actually exist, so that we only
                 # end up checking tiles that actually exist
                 if not self.dirtytree.query_path(childpath):
+                    # Here check if it *does* exist, and if so, nuke it.
+                    self._nuke_path(childpath)
                     continue
 
                 for child_path, child_mtime, child_needs_rendering in \
@@ -1034,6 +1090,31 @@ class TileSet(object):
                     # Nope.
                     yield path, max_child_mtime, False
 
+    def _nuke_path(self, path):
+        """Given a quadtree path, erase it from disk. This is called by
+        _iterate_and_check_tiles() as a helper-method.
+
+        """
+        if len(path) == self.treedepth:
+            # path referrs to a single tile
+            tileobj = RenderTile.from_path(path)
+            imgpath = tileobj.get_filepath(self.outputdir, self.imgextension)
+            if os.path.exists(imgpath):
+                # No need to catch ENOENT since this is only called from the
+                # master process
+                logging.debug("Found an image that shouldn't exist. Deleting it: %s", imgpath)
+                os.remove(imgpath)
+        else:
+            # path referrs to a composite tile, and by extension a directory
+            dirpath = os.path.join(self.outputdir, *(str(x) for x in path))
+            imgpath = dirpath + "." + self.imgextension
+            if os.path.exists(imgpath):
+                logging.debug("Found an image that shouldn't exist. Deleting it: %s", imgpath)
+                os.remove(imgpath)
+            if os.path.exists(dirpath):
+                logging.debug("Found a subtree that shouldn't exist. Deleting it: %s", dirpath)
+                shutil.rmtree(dirpath)
+
 ##
 ## Functions for converting (x, z) to (col, row) and back
 ##