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

Sound added! #51

Open
wants to merge 42 commits into
base: dev
Choose a base branch
from
Open

Sound added! #51

wants to merge 42 commits into from

Conversation

schielb
Copy link
Contributor

@schielb schielb commented Sep 1, 2023

No description provided.

emilyk19 and others added 30 commits August 21, 2023 16:18
made the beginning of the move function. finished with the button input, but have not tested.
…hen they hold down the button, but stop when they let go. still buggy jump function.
…but just moves it to a game over screen. still need to add a reset button to start the game again, and also the score.
Getting recent updates, specifically video demo
creating a place for bryson to work his magic
Run game ready to add sound
Copy link
Contributor

@christopolise christopolise left a comment

Choose a reason for hiding this comment

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

Lots of great work! Look at the comments I left and address accordingly. I think there are some serious conversations we will need to agree on before we can merge the current broadcaster outputqueue handling logic.

@@ -31,10 +31,32 @@ def start_outputs(system_queue, demo_output_queue):
logger.warning("Unable to import modules necessary to run MQTT output.")
logger.warning("Program will continue to run without this output.")

try:
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be done in a later branch, but we need to meticulously check and make sure the exceptions to this block will prevent the sss from crashing (i.e.. include all possible exceptions)

try:
for item in utils.get_all_from_queue(pygame_mixer_q):
logger.debug("pygame_mixer: {}", item)
if str(item).startswith("SOUND "):
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentinel value should be re-evaluated and a better protocol should be devised to ignore unwanted communication

@@ -0,0 +1,276 @@
import random
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this game from the this commit, doesn't fit within the scope of your branch

@@ -0,0 +1,91 @@
class TwoCircle:
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove from the commit, not relevant to PR topic

Copy link
Contributor

@apal6981 apal6981 left a comment

Choose a reason for hiding this comment

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

We need to have a conversation about how we want to want to handle sound names directly in demos. I don't like that we have to import something from another module. I think we should have our operating system take care of the sound names and we just output the sound name through the system queue from within the demo itself. In side of the demo, we probably should add a new member to the class that lists the sound names. In addition, i don't think we need a huge list listing what sounds we have available, we should dynamically load and create a list that a demo can request.

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.

6 participants