lmms
lmms copied to clipboard
LMMS
Restrict Time Signature denominators to 1, 2, 4, 8, 16, and 32
This PR creates a custom TwoPowerModel which inherits from IntModel with the restriction that TwoPowerModel only allows powers of two as its reported value.
It then changes the type of m_denominatorModel in MeterModel to TwoPowerModel.
This eliminates the ability to create irregular (and currently not well handled) time signatures in LMMS.
There are a couple of hackey things here, specifically in the LCD display. I am open to changing it if someone more familiar with the codebase has a better idea, but the GUI widget rendering the time signature denominator was dynamic casting the TwoPowerModel to an IntModel prior to getting the value, as a result of LcdSpinBox inheriting from IntModelView.
@Veratil
There are some changes in the
pluginsdirectory that need to be removed
Whooops, my bad! Pesky -a. I will fix that ASAP.
Style fixes all over...
Working on it.
As to your other points, I can get started on that, but I saw a couple others on Discord, (such as @zonkmachine ) saying that we shouldn't keep backward compatibility at all. LMMS doesn't currently handle irregular denominators very well, as some people were saying in general, so I'm not sure how many projects actually use them.
That said, if you all are in agreement that an option in Settings to allow irregular time signatures is needed, I can start on that as well.
Finally, I can definitely combine the model with the existing IntModel, but I thought that having a separate one would reduce the number of branch steps in get() which would make it ever so slightly faster, since if we added it to IntModel we'd have to have something like this:
if ( m_restrictToPowersOfTwo )
{
// Do what TwoPowerModel currently does
}
else
{
// Do current IntModel behavior
}
I may not be able to get this PR all the way ready today due to other responsibilities, but I'll try to early this week.
When attempting to load a project with an irregular time signature:
LMMS will now prompt if you want to keep it:
As to your other points, I can get started on that, but I saw a couple others on Discord, (such as @zonkmachine ) saying that we shouldn't keep backward compatibility at all.
I don't think I phrased myself that categorically but I am more relaxed than most of the team on backward compatibility. I also follow the wishes of the rest of the team which is to maintain backward compatibility at least when possible. That said I think the solution above to keep irregular time signatures looks really good.
This PR is out of date with the main branch, so I'm going to close it.
There are multiple ways to bring it up to date, there's no reason you need to close it.
Yeah I know, it just seemed like there wasn't any interest in merging it, since it's been sitting here since November. I'd rather close it if it's not going to get merged than leave it open forever and clutter up the PRs tab.
Interest is not lost, just attention has been focused elsewhere. This is still something that we should probably merge, but it just needs more than me reviewing it.
since it's been sitting here since November
I have PRs that go back to 2019, it's fine. :)
@ifndefJOSH sorry for the delay. Well i believe the denominators should be restricted to power of 2. Also @LostRobotMusic stated somewhere that the denominators being this flexible might lead to some breakages somewhere. Also i believe it might be a good idea to convert the denominator part to a drop down box instead of a lcd spinbox. If you could get this PR revived, we might as well merge this.
Also regarding the old projects, i believe it's fine to forcefully change. Just put a drop down on project load.
Also @LostRobotMusic stated somewhere that the denominators being this flexible might lead to some breakages somewhere.
I never said this. I did say there is no valid use case for non-power-of-2 denominators, since the purpose of the denominator is to determine which type of note holds the main pulse, 2 is the smallest prime number, and all available note lengths in LMMS are based around either powers of 2 or triplets of those. I also said we should ensure LMMS can still properly load old projects that make use of other denominators.
