Installing "mods" with Python A little disclaimer: you're going to have a field day with this. It's horrible. Looking at it makes me want to vomit, and don't ask me to explain it, because I've long forgotten what I was thinking when I wrote it. That being said, I can't really find a way to shorten it of make it make sense. Perhaps someone could help me out?

A bit of a background:

This program is meant to install mods for a game called Supreme Commander 2. It is meant to install ANY mod. The following is true of all the mods:


The only archive type is the .scd. SCD's have the same encoding as .zip's, but with a 0% compression rate.
The way to install the mod is simple: There will always be at least one scd inside the first, and there may be more scd's inside that one. Any scd with folders inside it should be moved to


  C:\Program Files (x86)\Steam\steamapps\common\supreme commander 2\gamedata


There may be more than one scd to be moved per mod.


And that's it. This is something of a "programming challenge," but I am really looking for improvements upon the following code:

import os, zipfile
def installfromdownload(filename):
    global scdlist
    targetfolder = r"C:\Program Files (x86)\Mod Manager\Mods\f_" + filename[:-4]
    lentf = len(targetfolder)
    unzip(r"C:\Program Files (x86)\Mod Manager\Mods\z_" + filename + ".scd", targetfolder)
    if checkdirs(targetfolder) == False:
        os.system("copy C:\Program Files (x86)\Mod Manager\Mods\z_" + filename + ".scd" " C:\Program Files (x86)\Steam\steamapps\common\supreme commander 2\gamedata")
    else:
        newfolderonelist = []
        getscds(targetfolder)
        for file in scdlist:
            newfolderone = targetfolder + "\f_" + file[:-4]
            filepath = targetfolder + "\\" + file
            unzip(filepath, newfolderone)
            newfolderonelist.append(newfolderone)
        for newfolders in newfolderonelist:
            if os.checkdirs(newfolders) == False:
                newfolders = newfolders[lentf + 1:]
                for scds in scdlist:
                    if newfolder in scds == True:
                        scdpath = targetfolder + "\\" + scds
                        os.system("copy " + scdpath + " C:\Program Files (x86)\Steam\steamapps\common\supreme commander 2\gamedata")


Note that there is no error handling here, that is done when I call the function. The function unzip() is here:

def unzip(file, target):
    modscd = zipfile.ZipFile(file)
    makepath(target)
    modscd.extractall(target)


The function checkdirs() is here (note this is pretty bad, it only checks for periods in the file name, so please submit suggestions):

def checkdirs(target):
    isfile = 0
    targetlist = os.listdir(target)
    for files in targetlist:
        for letters in files:
            if letters == ".":
                isfile = isfile + 1
    if isfile == len(os.listdir(target)):
        return False
    else:
        return True


The getscds() function just checks a filename for ".scd".
Firstly, your installfromdownload function should be install_from_download (and have a docstring to go with it explaining what it does).

This line:

targetfolder = r"C:\Program Files (x86)\Mod Manager\Mods\f_" + filename[:-4]


should probably check and make sure that the file actually ends with .zip (or whatever format you're actually expecting it to come in):

if filename.endswith('.zip'):
    target_folder = ...


Something doesn't seem quite right with:

unzip(r"C:\Program Files (x86)\Mod Manager\Mods\z_" + filename + ".scd", targetfolder)


You're stripping the last 4 characters off filename before, and here you're appending '.scd'. I assumed you were stripping .scd off above, but then realized you're unzipping it, so it's actually a .zip (or something like that). Either way, it's confusing and something like:

target_folder = r"..." + filename.split('.zip')[0] 


would make it much more clear what you're actually doing (commenting would help a bit too).

Don't explicitly check against False:

if checkdirs(targetfolder) == False:


This should be:

if not checkdirs(targetfolder):


Don't join paths together with "\\" (which is OS specific anyway):

 filepath = targetfolder + "\\" + file


Use os.path.join instead:

 filepath = os.path.join(target_folder, file)


Some of the variable names here make your code really, really hard to follow (newfolderonelist, newfolderone). 

This line should make your script crash:

if os.checkdirs(newfolders) == False:


As you're trying to call your checkdirs function (os.checkdirs doesn't exist).

Let's look at the checkdirs function:

def checkdirs(target):
    isfile = 0
    targetlist = os.listdir(target)
    for files in targetlist:
        for letters in files:
            if letters == ".":
                isfile = isfile + 1
if isfile == len(os.listdir(target)):
    return False
else:
    return True


Again following Python naming convention, this should be check_dirs (although the name needs a lot of work - what's it checking them for?). Further, a lot of the work here can replaced with os.path.isfile. From reading it, it looks like you're checking that everything in a given directory is not a file, so I'm going to call it not_all_files:

def not_all_files(directory):
    '''Checks the given directory, returning False if it contains only files,
    and True otherwise. If the argument given is not a directory, 
    raises IOError.
    '''

    if not os.path.isdir(directory)
        # Oops, passed in something that wasn't a directory
        # Handle this error
        raise IOError('....')
    return not all((os.path.isfile(f) for f in os.listdir(directory)))


Given how you're using it above, I'd rather swap around what it returns: have it return True if everything in the directory is a file, False otherwise. This would then be:

def all_files(directory):
    '''Checks the given directory, returning True if it contains only files,
    and False otherwise. If the argument given is not a directory, 
    raises IOError.
    '''

    if not os.path.isdir(directory)
        # Oops, passed in something that wasn't a directory
        # Handle this error
        raise IOError('....')
    return all((os.path.isfile(f) for f in os.listdir(directory)))


Your code in install_from_download would then become:

 if all_files(target_folder):


which I think is much easier to read.