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

Add swng, spin, slsh to serial commands #716

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions props/prop_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,18 @@ class PropBase : CommandParser, Looper, protected SaberBase, public ModeInterfac
}
return true;
}
if (!strcmp(cmd, "swing")) {
hybrid_font.PlayPolyphonic(&SFX_swng);
Copy link
Owner

Choose a reason for hiding this comment

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

swing is a lot more complicated than this, as it can be monophonic or polyphonic, swng.wav or swing.wav
Perhaps we should even consider breaking the function in hybrid font into a swing-detection function and a swing playing function, and then we could just call the playing function from here.

Or even better, have the detection function emit an event, and then do the playing in SB_Event2...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. Does this small "perk" feature warrant all that?
Would it suffice to just check if swng OR swing exist and then play one that does with PlayCommon()?

Copy link
Owner

Choose a reason for hiding this comment

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

If it's worth doing, it's worth doing right....

I think what you suggest would be the minimum needed though.

Splitting it out in the code has other advantages though, because sending events means that blades and displays get to react to swing events, if they want to.

Dropping "swing" command for now and just checking in the other two is also a valid solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated things to a rough start that works. Monophonic stuff needs to be considered next, but how does it look so far now?

return true;
}
if (!strcmp(cmd, "slash")) {
hybrid_font.PlayPolyphonic(&SFX_slsh);
return true;
}
if (!strcmp(cmd, "spin")) {
Copy link
Owner

Choose a reason for hiding this comment

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

This needs to be in #ifdef ENABLE_SPINS

Copy link
Contributor Author

@NoSloppy NoSloppy Dec 11, 2024

Choose a reason for hiding this comment

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

done

hybrid_font.PlayPolyphonic(&SFX_spin);
return true;
}

#ifdef ENABLE_AUDIO

Expand Down