shoc71 / Keep-On-Going

making a game on this website
Apache License 2.0
2 stars 0 forks source link

Level Zero Revisions #422

Open slysnivy opened 11 months ago

slysnivy commented 11 months ago

This is just a series of suggestions and my review of the LevelZero code now knowing more.

  1. Why do we need to change the FPS here? I'm pretty sure since we're referring to "pygame.time.Clock()", it refers to the same one as the one referenced in main and will change the game's FPS to 30. image

  2. Change morse_dict's keys to be a series of s and l to represent short and long. This would reduce the memory load in level_zero by a lot. Considering right now there are 37 items multiplied by 6-30 (a single short to short 5 times being the longest string) char's compared to that divided by 6 if they're shortened to 1 char, resulting in 37 x 1-6 memory usage. image

  3. We don't need to declare a self.morse_code_count to check length like in the image. It would be better to use len(self.str_morse_code) image

  4. Agreed mainly due to it not being used for the if statement below image image

  5. I would redo this method because display as a parameter is not being used since you can get away with using a dummy variable (no need to pass it). Additionally, the line for g_string = str(f_string) is not needed (extra unnecessary step). image

  6. Your first if is if the count is < 5, and the second is if it's == 5. If there's a scenario that you're able to get past 5, then the check/safety measures in == 5 are avoided. image

In this case, the self.action_timer (delay before next button pressed) is not used, and without it the player can hit the buttons twice in an instant. This causes letters to be added 2 or more at a time, (2, 4, 6, always even), avoiding 5 since it's odd and causing the issue below: image

In the worst scenario, it can cause lag due to the sheer amount of text. The lag causes the player to collide with the convert_code box longer, meaning more text is generated for each jump, compounding the issue you see here: image

These issues don't have to be fixed immediately as we're still early in development, but should be done within the Long Term Development Goal deadline as stated in this issue's Milestone.

shoc71 commented 11 months ago

Why do we need to change the FPS here? I'm pretty sure since we're referring to "pygame.time.Clock()", it refers to the same one as the one referenced in main and will change the game's FPS to 30.

This wasn't an intentional change as I was trying to fix the frame rate issue so that multiple inputs wouldn't be inputted (which still hasn't been resolved) and it is almost being used and recalled for the text timing.

Change morse_dict's keys to be a series of s and l to represent short and long. This would reduce the memory load in level_zero by a lot. Considering right now there are 37 items multiplied by 6-30 (a single short to short 5 times being the longest string) char's compared to that divided by 6 if they're shortened to 1 char, resulting in 37 x 1-6 memory usage.

Memory wasn't even being considered as an issue here. We can change it, however, I only kept it that way because it was easier for the player to see what inputting they have put on so far and from there I called the dict to translated everything easier. If memory is a serious issue, we can change it.

We don't need to declare a self.morse_code_count to check length like in the image. It would be better to use len(self.str_morse_code)

Oh, that works too. We can change it to that.

I would redo this method because display as a parameter is not being used since you can get away with using a dummy variable (no need to pass it). Additionally, the line for g_string = str(f_string) is not needed (extra unnecessary step).

g_string was used for testing purposes but you're right. It should be removed.

Your first if is if the count is < 5, and the second is if it's == 5. If there's a scenario that you're able to get past 5, then the check/safety measures in == 5 are avoided.

Thanks for catching that mistake. I'll change that as well.

These issues don't have to be fixed immediately as we're still early in development, but should be done within the Long Term Development Goal deadline as stated in this issue's Milestone.

All of these issues are noted and thanks for bringing them up.