Add swng, spin, slsh to serial commands #716
Conversation
| hybrid_font.PlayPolyphonic(&SFX_slsh); | ||
| return true; | ||
| } | ||
| if (!strcmp(cmd, "spin")) { |
There was a problem hiding this comment.
This needs to be in #ifdef ENABLE_SPINS
props/prop_base.h
Outdated
| @@ -1376,6 +1376,18 @@ class PropBase : CommandParser, Looper, protected SaberBase, public ModeInterfac | |||
| } | |||
| return true; | |||
| } | |||
| if (!strcmp(cmd, "swing")) { | |||
| hybrid_font.PlayPolyphonic(&SFX_swng); | |||
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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()?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
| @@ -320,9 +320,15 @@ class HybridFont : public SaberBase, public Looper { | |||
| float s = (rss - font_config.ProffieOSMinSwingAcceleration) / font_config.ProffieOSMaxSwingAcceleration; | |||
| effect->SelectFloat(s); | |||
| } | |||
| swing_player_ = PlayPolyphonic(effect); | |||
| current_swing_effect_ = effect; | |||
There was a problem hiding this comment.
I don't want to pass the effect in a global variable like this.
There was a problem hiding this comment.
Hmmm. Are you thinking a singleton something instead?
Maybe another class needs to be created, or somehow add a parameter to SB_Effect?
I am not sure what to do instead.
Is there a problem with the use of the global variable?
| @@ -320,9 +320,15 @@ class HybridFont : public SaberBase, public Looper { | |||
| float s = (rss - font_config.ProffieOSMinSwingAcceleration) / font_config.ProffieOSMaxSwingAcceleration; | |||
| effect->SelectFloat(s); | |||
There was a problem hiding this comment.
This should probably happen in SB_Effect() now.
There was a problem hiding this comment.
well wouldn't that mean it would need to be repeated for both EFFECT_ACCENT_SWING and EFFECT_ACCENT SLASH cases? Is it not neater to keep it in just one occurrence where it is?
Or again, is this lending itself to all of these changes somehow in a new class or function?
This is starting to grow out of my ability to brain this.
| @@ -39,6 +39,8 @@ extern SaberBase* saberbases; | |||
| DEFINE_EFFECT(POWERSAVE) \ | |||
| DEFINE_EFFECT(BATTERY_LEVEL) \ | |||
| DEFINE_EFFECT(VOLUME_LEVEL) \ | |||
| DEFINE_EFFECT(ACCENT_SWING) \ | |||
| DEFINE_EFFECT(ACCENT_SLASH) \ | |||
There was a problem hiding this comment.
Probably want to add one for spins too.
This not only adds good functionality for testing sounds and such, but facilitates access to trigger these sounds via remote control apps.