osara
osara copied to clipboard
jcsteh
Add project and take markers action
This improves support for take markers, by making adding them context sensitive. Navigation of them will be added to the key map in the next commit.
Build succeeded! Build osara pr1133-1569,e50f98de completed (commit https://github.com/jcsteh/osara/commit/e50f98dedd by @ScottChesworth) Downloads:
Build succeeded! Build osara pr1133-1570,ee47c18f completed (commit https://github.com/jcsteh/osara/commit/ee47c18ff6 by @ScottChesworth) Downloads:
Gotcha, I can refactor that. Did you have specific fakeFocus states in mind? I started off trying to use FOCUS_ITEM and FOCUS_RULER because I figured it would be nice to Page through long takes dropping markers, but it felt like the lesser used marker type was taking precedence too often that way, so I was gonna tell people to clear selection with Shift+Escape instead. Maybe I should try removing item selection from the conditionals, rely entirely on fakeFocus instead? Hmm.
I was thinking take markers would only be FOCUS_ITEM, but I guess I can see why that would make this action far less useful, precisely because you can't drop take markers while moving through the project. I guess the guidance could be that you need to select the item under the cursor to force focus back to the item when you want to drop a take marker, but that seems quite annoying too. Ug.
I'm not really sure what to suggest here. I'm not sure we can really make this context sensitive without encountering problems like this. I don't love the idea of having to clear selection to drop a project marker; it feels a bit unintuitive. I'm probably a bit biased though because I use project markers all the time, but I've never used take markers and I don't see that changing any time soon. That said, I can just keep using the existing actions, so if I'm the odd one out here, that's fine.
I don't think you'll be the odd one out, using it more tonight, I'm not liking it yet either. Will keep trying stuff. I'm exploring the context sensitive approach as there's nowhere logical left to put adding and editing take markers. T, A, K, E, M and R are all crammed.
Refactored with a lot of help from @TimTam, changed to using fakeFocus instead of item selection because while not ideal, it makes the context sensitivity less weighted toward take which feels better in situ. I'll come back tomorrow and fix the key map conflict.
Build succeeded! Build osara pr1133-1585,549d0a14 completed (commit https://github.com/jcsteh/osara/commit/549d0a140f by @ScottChesworth) Downloads:
Wew, resolved conflicts. Can you take a gander at this approach when you get time @jcsteh?
Build succeeded! Build osara pr1133-1587,a2ba8695 completed (commit https://github.com/jcsteh/osara/commit/a2ba869513 by @ScottChesworth) Downloads:
Build succeeded! Build osara pr1133-1588,819a7b5e completed (commit https://github.com/jcsteh/osara/commit/819a7b5e41 by @ScottChesworth) Downloads:
Just FYI @jcsteh, we tested explicitly with items that don't have takes and didn't cause a crash, probably because REAPER is smart enough to just return 0 if you hand it an invalid take. Its fair to ensure we don't cause unexpected behaviour, but REAPER is smart enough to don't crash too. An extra safety layer doesn't hurt though.
Yeah, I did wonder whether REAPER might be smart enough to handle that. I'm fairly sure I've seen a crash by my failure to null check takes in the past, but maybe that wasn't a REAPEr function I was calling; I can't recall.
If you've explicitly tested it and it doesn't crash, I can live without that change.
The commit I'm talking about is 8944a12bc3be0d16ba3d68c1a4fb89c1daa30b05, but maybe that was caused by something else I was doing with a null take there. My commit comment is a bit vague on the details of the actual crash. :)
Hmm I remember thinking context sensitivity wasn't working how I wanted yet last time I used the test build in this PR. It's been so long I can't recall what I meant to change though. Might end up closing it. I'll try running it for a while when I'm working tomorrow, then decide.
Eventually retested this. It's too finicky to know what type of marker is going to be made so I'm closing. Will try to come up with something else.