Skip to content

Conversation

@theGreatWhiteShark
Copy link
Contributor

It seems #1482 was merged in a dirty state and is now (at least for most parts) in master because I messed up some git stuff. (But this doesn't mean we need to use this implementation over #1437. Everything it still on the table :) )

I choose to

  • pick the bottom most pattern instead of the one requiring the least amount of scrolling in order to both make things reproducible and ease the recording process
  • unset the pattern in case an empty column is reached during playback in song mode. This make the overall UX consistent and might be helpful to point users to the fact that the column is empty (which is not obvious if one uses a large amount of patterns)
  • the pattern editor is only locked when transport is rolling in song mode. Otherwise, other patterns can be selected by clicking the pattern list in the SongEditor. This behavior is indicated by checking the lock button only if transport is rolling.
  • let the whole logic remain in the core. I thought about moving it into the GUI similar to Follow playing patterns in Song Editor #1437 since it would result in less code and the feature itself does not affect the core. But I decided against it because a) the locking would only take effect in response to a specific event, like the reaching a different column, instead of all related events, especially clicking the lock button. b) Because it would allow to get the pattern notes will be recorded too and the one being displayed out of sync.

@cme do you concur with this?

Implementation details:

  • remove Preferences::bPatternFollowsSong in favour for Song::m_bIsPatternEditorLocked. Whether this one should be stored in the Song or Preferences is not an easy decision. It is more or less determined with whether it should be set via a button in the GUI or an option in the Preferences. I guess there are people who like this feature a lot and will have it turned on the whole time. Those will be probably annoyed as they have to turn it on every single time they open a new song. On the other hand, the feature feels super helpful when playing back a finished song or while fine-tuning it and super annoying composing it. I see myself altering its state at least once during the creation of each song and this sounds way more like a button than an option in the Preferences. (But this might just be me).
  • ensure the selected pattern is updated on all relevant instances in case the editor is locked
  • allow for the pattern to be changed in song mode while transport is not rolling
  • disable the locking buttons in pattern mode in order to indicate they only take effect in song mode

- remove Preferences::bPatternFollowsSong in favour for Song::m_bIsPatternEditorLocked. Whether this one should be stored in the Song or Preferences is not an easy decision. It is more or less determined with whether it should be set via a button in the GUI or an option in the Preferences. I guess there are people who like this feature a lot and will have it turned on the whole time. Those will be probably annoyed as they have to turn it on every single time they open a new song. On the other hand, the feature feels super helpful when playing back a finished song or while fine-tuning it and super annoying composing it. I see myself altering its state at least once during the creation of each song and this sounds way more like a button than an option in the Preferences. (But this might just be me).
- ensure the selected pattern is updated on all relevant instances in case the editor is locked
- allow for the pattern to be changed in song mode while transport is not rolling
- disable the locking buttons in pattern mode in order to indicate they only take effect in song mode
@cme
Copy link
Contributor

cme commented Apr 15, 2022

Sounds good, I think (sorry, I've been busy elsewhere, attempting to catch up!)

One question, the lock button, currently in the song editor's tool bar, would it perhaps be better somewhere near the pattern editor since that's what it visibly affects?

@theGreatWhiteShark
Copy link
Contributor Author

sorry, I've been busy elsewhere, attempting to catch up!

Don't worry. It's quite a lot of PRs I opened recently. But most stuff for 1.2 seems done and just a couple of tabs left on the TODO list.

One question, the lock button, currently in the song editor's tool bar, would it perhaps be better somewhere near the pattern editor since that's what it visibly affects?

I put it there because the panel of the PE is already quite full and overflowing on smaller window widths. Also, in the panel of the SE there are already a number of other buttons affecting only pattern or song mode and being enabled/disabled when switching between them. (TBH this feels like a small mess. At some point in the future we maybe could think about grouping song mode and the pattern mode stuff more elegantly but not now.)

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.

2 participants