Income Simulation programifier

Question

http://pastebin.com/7UqD0gwF

I'm looking for serious feedback on my first serious program.

This is a program to calculate PvZ income over time of sunflowers and sunshrooms on nighttime pool levels.

Shrooms might have an obvious advantage but this program was borne over the idea that once all space allocated to income plants fills up, they are operating at lower rates until they are two minutes old and thus it, might be possible that sunflowers end up earning more over the course of a few minutes if they fill up the same area and when shrooms scale worse with lily pad costs

The hypothesis was false :( but they're so close that there is no functional gameplay difference

The program assumes 16 income plants, 3 rows on land and 2 in water because water is harder to defend. Changes in maximum number of income plants changed the results by little to nothing. Attempts to use both plants gave no profits.

time = 0

sun_f = 50                                              # spendable sun for sunflower simulation
sun_f_total = 0                                         # total earned sun for sunflowers
sun_s = 50                                              # spendable sun for sunshroom simulation
sun_s_total = 0                                         # total earned sun for sunshrooms

cooldownflower = 0
cooldownshroom = 0

sunflowers = 0
sunshrooms = 0

ageflower = []                                          # lists for calculating income timing and shroom evolving
ageshroom = []

Just the variables

def buildsunflower():
    global sun_f
    global sunflowers
    global cooldownflower
    if sunflowers < 12:
        sun_f -= 50                           # normal cost
    elif sunflowers > 11:
        sun_f -= 75                           # adjusted lily pad cost for water placement after 12 land spots fill
    sunflowers += 1                           # running total for sunflowers
    ageflower.append(12)                      # the first income packet only requires 12 instead of 24 seconds
    cooldownflower = 9                        # sets sunflower build cooldown to 8 + 1 second of human error


def buildsunshroom():
    global sun_s
    global sunshrooms
    global cooldownshroom
    if sunshrooms < 12:
        sun_s -= 25
    elif sunshrooms > 11:
        sun_s -= 50
    sunshrooms += 1
    ageshroom.append(12)
    cooldownshroom = 9

These are the functions that simulate building the plants. I'm not sure if the global thing is ideal here.

def update():
    global time
    global cooldownflower
    global cooldownshroom
    global ageflower
    global ageshroom
    print('\t', time, '\t\t', sun_f, '/', sun_s, '\t\t', sun_f_total, '/', sun_s_total,
      '\t\t', sunflowers, '/', sunshrooms)                                         # per second readout
    time += 1                                                                          # running total of seconds
    cooldownflower -= 1                                                                # reduces build cooldowns from 9
    cooldownshroom -= 1
    ageflower = [x + 1 for x in ageflower]                                # magical aging code I don't understand yet
    ageshroom = [x + 1 for x in ageshroom]

Figuring out how to program the income was the hardest part for me by far as the exact [average] moment you get the income depends on when you first build the plant. I solved it by appending a list after each sunflower/shroom was planted and by having my loop age the list with +1 to every value, and then adding income whenever the loop finds a value that's a multiple of 24. It works swell but I feel there are much better ways to accomplish this. I'm not even entirely sure how the list aging code works, I've never seen it like that before.

for n in range(300):
    if sun_f >= 50 and cooldownflower <= 0 and sunflowers < 16:               # plants flower if currency,
        buildsunflower()                                                      # cooldown, and room allow

    for x in ageflower:
        if x % 24 == 0:                                                       # adds income in multiples of 24 seconds
            sun_f += 25
            sun_f_total += 25

    if sun_s >= 25 and cooldownshroom <= 0 and sunshrooms < 16:
        buildsunshroom()

    for x in ageshroom:
        if x % 24 == 0 and x > 120:                                           # the 120 is to check for evolved shroom
            sun_s += 25
            sun_s_total += 25
        elif x % 24 == 0:
            sun_s += 15
            sun_s_total += 15

    update()                                                                   # recalculates before new loop

The loop tying it all together.

Answer 1

First off global is almost always a red flag. Normally you can change this to use a class.

If we look at your first 15 lines you should see:

time = 0

sun_f = 50
sun_f_total = 0
sun_s = 50
sun_s_total = 0

cooldownflower = 0
cooldownshroom = 0

sunflowers = 0
sunshrooms = 0

ageflower = []
ageshroom = []

If you change the grouping:

time = 0

sun_f = 50
sun_f_total = 0
cooldownflower = 0
sunflowers = 0
ageflower = []

sun_s = 50
sun_s_total = 0
cooldownshroom = 0
sunshrooms = 0
ageshroom = []

You should notice that these are almost exact duplicates. And so you can make a class.

class Plant:
    def __init__(self):
        self.sun = 50
        self.sun_total = 0
        self.cooldown = 0
        self.suns = 0
        self.age = []

This makes the above:

time = 0
flower = Plant()
shroom = Plant()

You should notice that age seems wrong for a list, and may be better with ages or plant_ages. I'll go with ages. You should also see that self.suns == len(self.ages). And finally I'd change sun_total so you don't have to manually add to it.

This can result in:

class Plant:
    def __init__(self):
        self._sun = 50
        self._sun_total = 0
        self.cooldown = 0
        self.ages = []

    @property
    def sun(self):
        return self._sun

    @sun.setter
    def sun(self, amount):
        diff = amount - self._sun
        if diff > 0:
            self._sun_total += diff
        self._sun = amount

    @property
    def sun_total(self):
        return self._sun_total

You also have buildsunflower and buildsunshroom which you should change. The only numbers that change are 50 and 75 and 25 and 50. This means you can use a single variable to merge these functions. You can also add it to the above class. You should also see that in your main for loop you do the following check, if plant.sun >= plant.cost and plant.cooldown <= 0 and len(plant.ages) < 16:, just before you call this function, and so I'd add it into the function.

This should result in something like:

class Plant:
    def __init__(self, cost, build_amounts):
        self._sun = 50
        self._sun_total = 0
        self.cooldown = 0
        self.ages = []
        self._cost = cost
        self._amounts = build_amounts

    def build():
        if not (self.sun >= self._cost and self.cooldown <= 0 and len(self.ages) < 16):
            return
        self.sun -= self._amounts[0 if len(self.ages) < 12 else 1]
        self.ages.append(12)
        self.cooldown = 9

    @property
    def sun(self):
        return self._sun

    @sun.setter
    def sun(self, amount):
        diff = amount - self._sun
        if diff > 0:
            self._sun_total += diff
        self._sun = amount

    @property
    def sun_total(self):
        return self._sun_total

Finally there is one (two) more functions that I'd make. Each loop of your main loop you do:

for x in ageflower:
    if x % 24 == 0:
        sun_f += 25
        sun_f_total += 25

And an equivalent for shrooms as these can't just be merged with a simple variable, I'd make a subclass to Plant for Flower and Shroom that implement this function. Which should result in:

class Flower(Plant):
    def __init__(self):
        super().__init__(50, (50, 75))

    def generate_sun(self):
        for plant in self.ages:
            if plant % 24 == 0:
                self.sun += 25

class Shroom(Plant):
    def __init__(self):
        super().__init__(25, (25, 50))

    def generate_sun(self):
        for plant in self.ages:
            if plant % 24 == 0:
                self.sun += 25 if plant > 120 else 15

Finally you're left with your main loop and update. I'd merge these together the changes at the bottom of update don't affect the print, and so you can do all the functions above the print. Looping through all the plants allows you to simplify your duplicate logic. And so could result in:

if __name__ == '__main__':
    time = 0
    flower, shroom = plants = (Flower(), Shroom())
    for n in range(300):
        time += 1
        for plant in plants:
            plant.build()
            plant.generate_sun()
            plant.cooldown -= 1
            plant.ages = [x + 1 for x in plant.ages]

        print('\t {} \t\t {} / {} \t\t {} / {} \t\t {} / {}'.format(
                time,
                flower.sun,
                shroom.sun,
                flower.sun_total,
                shroom.sun_total,
                len(flower.ages),
                len(shroom.ages)))