Dn-FamiTracker icon indicating copy to clipboard operation
Dn-FamiTracker copied to clipboard

Add envelopes to VRC7 instruments

Open eulyderg opened this issue 2 years ago • 2 comments

eulyderg avatar Nov 17 '21 02:11 eulyderg

I tested this branch a bit. Fortunately, so far ASAN didn't find any new memory errors, aside from this branch saving corrupted files which cause master to crash when loading, and this PR to trip ASAN when loading.

  • The Pitch and Patch envelopes have nearly the same name, which threw me off for a moment. Is this acceptable, and is there a better solution or not?
  • It's hard for me to use the patch envelope, because I have to constantly flip back and forth to the VRC7 tab to look at the list of patches, and back to the Envelopes tab to try out different patch sequences (but there's no list of patches there, or ability to listen to them individually). Maybe this would be less of an issue for someone who actively uses VRC7 and isn't rusty like me.
    • The two-tab UI feels slightly awkward to use in general, for switching between default patches. Note that I didn't test switching between default and custom patches, nor between custom patches.
  • When a patch envelope is active, the VRC7 tab has no visual indication that the "static patch" you pick has no effect. (Maybe Patch 0's custom settings still have an effect, I'm not sure). And I can't preview pure patches by picking them in the VRC7 tab and playing notes.
    • Maybe there should be a visual indication that the static patch index has no effect. Maybe there should be some alternative way to preview pure patches even with a patch envelope active. But having the static patch override the patch envelope when you're on the VRC7 tab isn't a good idea either, I think.

Will users want to switch between custom patches? AFAIK in this branch, an instrument can't switch from one custom patch to another, and the user will have to switch between custom instruments in the pattern editor (limited by the pattern speed).

nyanpasu64 avatar Nov 23 '21 03:11 nyanpasu64

This PR resolves #22.

Gumball2415 avatar Aug 06 '22 04:08 Gumball2415

closing, as it cannot be merged as-is due to too much changes to the module document API. i will have to re-create this pull request based on the next release.

Gumball2415 avatar Mar 10 '23 08:03 Gumball2415