bug: range selection lost after range-playback started
esaruoho opened this issue · 4 comments
it seems that range selection is lost after range-playback is started.
Steps to replicate.
- Select 4 rows of song ( all 3 channels of the song )
- Click on Play Range
Expected result:
Selection stays
Current result:
selection lost
This really has to do with how the follower works. For instance, you'll notice that the selection in the sequencer is slightly different during playback (only the column is selected, while the row is controlled by the follower).
...but I agree that it would be nice if the selection was reset after playback. Also, the pattern position should be reset (e.g. to the top of the pattern).
@mbitsnbites for your information, I went pretty deep in solving this and #51. I finally concluded that trying to fix the current mSeqRow logic is just not worth it. The design is just too shaky: as the mSeqRow, mSeqCol etc. variables can be changed anywhere, the code base is full of updateXXX calls and it's difficult to make sure that no ill values are placed into them. This is very bug prone!
So I encapsulated all changes to GUI, OOP-style. I have one class, CTableEditor, that shares all the common code base between sequencer, pattern editor and fxtrack editor. It takes care of copy, paste, updating the table etc...
I have three instances: mSeq, mPattern and mFxTrack. Each of them tracks the location of the cursor and selected area, and these two by the way are now separated, so the user can keep the selection even when the cursor is moving with the song. It's maybe difficult to say everything that I ended up changing, but check the branch at: https://github.com/vsariola/soundbox/tree/feature/new_gui_logic
It solves several issues simultaneously (at least #50 and #51), saves already some 300 lines of code without any obvious loss of functionality, and seems more "future proof" than trying to remember to call proper "UpdateXXX" functions in different places. I'm not even nearly done here; I didn't even touch the instrument update part yet, so I think it'll save some 500 lines of code at least.
I just got a copy working somehow, but I'll continue to working on this in the following days and do some testing and putting out bugs. Let me know what you think of the approach taken.
This looks awesome! It's a long overdue change, so it's very welcome. Over the years I have just copy-pasted code when I've added functionality, knowing that it's an error prone, ugly solution, but I've never had the time/motivation to refactor it properly. Looking forward to your PR 👍