-
Notifications
You must be signed in to change notification settings - Fork 8
Turn in the Mini Project 4 #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…ort.py can implement the function inside it.
branchwelder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall your functionality is good, but I think you could have refactored a lot of your code out into different functions to make it easier to read and modify in the future.
| other.infected_pop = 1 | ||
|
|
||
|
|
||
| background_color = (255,255,255) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually, global variables are defined before any other code is written - but this doesn't affect the rest of your code in any way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By convention, global variables are capitalized e.g. BACKGROUND_COLOR.
|
|
||
| intro = True | ||
|
|
||
| while intro: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to put the game intro, main loop, and ending is separate functions that you can call at the appropriate times. This helps with code organization.
|
|
||
| upgrades = 0 | ||
|
|
||
| """ Pressing C will officially start the game running our |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main game loop should be inside its own function.
| time = 0 | ||
| Upgrade_Point = 0 | ||
| infectionindex = 1 | ||
| """ This is the counter to allow you to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need multiline comments here - inline comments will suffice.
| pygame.init() | ||
| font = pygame.font.SysFont('Consolas', 20) | ||
|
|
||
| class Country: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class could be put into its own file and imported.
|
|
||
| pygame.display.update() | ||
|
|
||
| screen = pygame.display.set_mode((640, 480)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this setup code could go into a setup() function.
| country.infected_pop = country.infected_pop + 1 | ||
| infectionindex = infectionindex - 1 | ||
| country_pop_index = country | ||
| """now our pathogen embarks on infection!""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use inline comments (like this: # this is a comment) instead of multiline comments in this case .
| print (country.infected_rate) | ||
|
|
||
| #increase kill rate | ||
| if event.key == pygame.K_k: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of this code could be refactored so it's easier to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One guideline is the number of lines of a section of code, and how many different things it's doing. When you find yourself labeling a subsection as to what it does, this is a sign that this section of code could be pulled out into a separate function. Doing this gives you something to name, and document, and potentially test.
| Subeen Kim | ||
| """ | ||
|
|
||
| class country: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By convention, class names are capitalized (Country).
| """return integer part of infected population""" | ||
| # return int(self.infected_pop) | ||
|
|
||
| """return probability""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For comments that are inside a function and describe the inside view how some lines of code work (as opposed to docstrings, that are at the top and describe the outside view of what the whole function does), use # comments instead of """doc strings""".
|
|
||
| Time = 0 | ||
| time = 0 | ||
| Upgrade_Point = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By convention, variables whose values change (variables that are not constants) are all lowercase.
| alive_pop = self.max_pop | ||
| if self.infected_ratio() > 0.10: | ||
| if self.infected_pop > 10: | ||
| if self.infected_ratio() > 0.10: #ratios are arbitrarily selected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's recommended that you give constants a name and capitalize them. self.infected_ratio() > THRESHOLD.
Also, it's common practice to make a config class that contains all these arbitrary values
| if all(country.max_pop == 0 for country in countries) == True: | ||
| running = False | ||
| endscreen = True | ||
| #print ('For each country: (infected ratio, total population)', (country1.infected_ratio(),country1.max_pop), (country2.infected_ratio(),country2.max_pop), (country3.infected_ratio(),country3.max_pop)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest removing print statements and logs before submitting pull requests (in the future)
| """Modify Time + XXXX to modify the speed of the game.""" | ||
| if pygame.time.get_ticks() > (Time + 1000): | ||
| Time = pygame.time.get_ticks() | ||
| """ If population == 0 then game is over""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest using inline comments # if the population is zero, then game is over if the comments can be kept brief.
SeunginLyu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, good improvements regarding documentation. I made some stylistic suggestions. Congratulations on finishing your final miniproject for softdes!
Sorry for late pull request
Please review our country.py file.