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.
1 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)))
global
calls. \$\endgroup\$