TASagent / TASagentTwitchBotCore

Other
7 stars 1 forks source link

bit cheer sound effect is hard coded #11

Open inkydom opened 3 years ago

inkydom commented 3 years ago

Line 304 of FullActivityProvider.cs:

Audio.SoundEffect cheerSoundEffect = soundEffectSystem.GetSoundEffectByName("FF7 Purchase");

I think this should be moved to the config file (or database if we made a UI for it, but the config file seems easier). I'm guessing other sound effects for other alerts are like this? I can probably work on this later this week or next.

This isn't a huge issue, since I just renamed my sound effect to "FF7 Purchase". Alternatively for a fast and dirty fix, we could hardcode more generic names like "Bit Cheer".

TASagent commented 3 years ago

Yeah, customization is a tricky problem. I'd actually initially envisioned customization being done in the source code rather than config files, since I pictured this as a framework for programmers to be able to tinker and with relatively little friction implement new and bizarre things. That's why, for instance, classes like the FullActivityProvider.cs have modularized so many of their behaviors into protected virtual methods. So I'd pictured customization looking more like CustomActivityProvider.cs in the TASagentTwitchBot.SimpleDemo project, especially since the unbounded number of different solutions, even just simple ones, would require vastly different configuration specifications. Since it's not really a framework built to support a lot of behavior customization through configuration, I figured it may be misleading to start with a system that kind of suggested configuration files were how you were meant to inject customization.

The upside of customization through configuration is obviously that you reduce the need to understand/modify the code, but the downside is that it comes with huge coupled constraints on flexibility and clarity. Like, two possible example approaches on the opposite end of the flexibility scale are "moving the existing hard-coded sfx names into a configuration file" and "defining all notification behaviors with a scripting language". The former only lets you remap the sound effects, while accumulating vestigial components and actually becoming more annoying for people who try to customize some of the behaviors through inheritance, while the latter is both a tremendous amount of work, as well as raises customization to once again inaccessible levels to most people.

I'm curious what thoughts you might have on this problem.

TASagent commented 3 years ago

Consider the case of someone wanting to add a different sound effect for each tier and for a couple special durations for sub notification. In the case of the config file approach, they'd start by inheriting form FullActivityProvider.cs and overriding GetSubscriberAudioRequest, adding in some branch to select the sound effect. They'd also need to inherit from the configuration object and add in new fields. They'd either ignore the existing SubNotificationSFX field and add in a bunch of new ones, or they'd use the existing field for one of the conditions, and add in new fields for the rest. Or they'd add a new object that just holds the configuration for their customizations. None of these cases, though, really feel ideal.