Skip to content
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

Large Refactor #317

Merged
merged 14 commits into from
Mar 11, 2024
Merged

Large Refactor #317

merged 14 commits into from
Mar 11, 2024

Conversation

mchirlin
Copy link

@mchirlin mchirlin commented Feb 5, 2024

Refactoring

  • Moving all games to use a common class: games.game
  • Reusing as much code as possible across games
  • Added logging to file throughout
  • Removed a lot of duplicate type variables - moved all game_opts to a single variable

Bug Fixes

Enhancements

update.py Outdated
@@ -51,7 +51,8 @@ def check_for_update(voice):
#print(os.getlogin())
process = run_command("sudo runuser -l {} -c 'cd {};pwd'".format(homename,os.getcwd()))
process = run_command("sudo runuser -l {} -c 'cd {};git fetch'".format(homename,os.getcwd()))
diff_files = run_command("sudo runuser -l {} -c 'cd {};git diff origin/master --name-only --cached'".format(homename,os.getcwd())).split()
diff_files = run_command("sudo runuser -l {} -c 'cd {};git diff origin/debugging-error --name-only --cached'".format(homename,os.getcwd())).split()
# TODO - Replace with this: diff_files = run_command("sudo runuser -l {} -c 'cd {};git diff origin/master --name-only --cached'".format(homename,os.getcwd())).split()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will revert this before merging

@mchirlin mchirlin changed the title Debugging error Large Refactor Feb 5, 2024
@adangert
Copy link
Owner

adangert commented Feb 7, 2024

Thanks! Next time that I have some spare time, I will test out the code changes, are you using bookworm or bullseye for the raspberry pi OS btw?

@mchirlin
Copy link
Author

mchirlin commented Feb 7, 2024

I think it's bullseye.

move.set_leds(random.randrange(100, 200),0,0)
# Set moves to random colors
elif game_mode == Games.Ninja:
if move_num <= 0: # TODO - How does this happen?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is just for the first controller (move 0) to be a different random color. i.e. one controller is a random value, the rest are red for this game mode.

# If trigger is no longer being held, reset the start
if menu_opts[Opts.HOLDING.value] == True and move_button == Button.NONE and move.get_trigger() <= 100:
menu_opts[Opts.HOLDING.value] = False
# TODO - do you want to reset the force_start_timer?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should reset once we start holding again, on L243

move_opt[Opts.random_start.value] = Alive.on.value
for serial in self.move_opts.keys():
for move_opt in self.menu_opts.values():
move_opt[Opts.RANDOM_START.value] = False
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -654,7 +691,7 @@ def game_loop(self):
self.check_for_new_moves()
if len(self.tracked_moves) > 0:
self.check_new_admin()
self.check_change_mode()
self.check_change_mode() # TODO - do we want to make this so only admins can change mode?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not a bad idea (especially for larger groups, hitting the buttons can disrupt things) but for a smaller group, let's say 4 players, it's easier than going into admin mode every time. This probably should be a setting that can lock switching modes to admins only, so for conventions it would make sense.

games/zombie.py Outdated
super().before_game_loop()

# Kill first humans
for random_human in random.sample(set(self.humans), 2):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getting the error here:
Traceback (most recent call last):
File "/home/aaron/JoustMania/piparty.py", line 1215, in
piparty.game_loop()
File "/home/aaron/JoustMania/piparty.py", line 697, in game_loop
self.check_start_game()
File "/home/aaron/JoustMania/piparty.py", line 960, in check_start_game
self.start_game()
File "/home/aaron/JoustMania/piparty.py", line 1125, in start_game
zombie.Joust(moves=game_moves, command_queue=self.command_queue, ns=self.ns, red_on_kill=self.red_on_kill,
File "/home/aaron/JoustMania/games/zombie.py", line 61, in init
self.game_loop()
File "/home/aaron/JoustMania/games/game.py", line 388, in game_loop
self.before_game_loop()
File "/home/aaron/JoustMania/games/zombie.py", line 104, in before_game_loop
for random_human in random.sample(set(self.humans), 2):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/random.py", line 439, in sample
raise TypeError("Population must be a sequence. "
TypeError: Population must be a sequence. For dicts or sets, use sorted(d).

piparty.py Show resolved Hide resolved
@adangert
Copy link
Owner

Fantastic work!! Just went through the code, and was able to test all the game modes,
There were a couple of bug fixes that I was able to find and update as well. But all the new code is much cleaner with way less DRY code everywhere. Thanks so much for all the help!

@adangert adangert merged commit 90c0d46 into adangert:master Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants