Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Model class visitable #4902

Merged
merged 6 commits into from
Apr 27, 2019
Merged

Conversation

JohannesLorenz
Copy link
Contributor

This makes the different AutomatableModels visitable and adds a visitor based cast function for cleaner code and more type safety. Currently only used in the Lv2 branch, but it's a generic feature.

@JohannesLorenz
Copy link
Contributor Author

This has no impact, so I'll merge without review in 5 days.

@JohannesLorenz JohannesLorenz added needs code review A functional code review is currently required for this PR needs style review A style review is currently required for this PR labels Apr 17, 2019
Target* dcast(bool doThrow = false)
{
DCastVisitor<Target> vis; accept(vis);
if(doThrow && !vis.result) Q_ASSERT(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide an example where the cast fails?
Also, please use if ( instead of if(.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, you have an AutomatableModel* pointing to a ComboBoxModel or IntModel, but try converting the pointer to FloatModel* (using FloatModel for the Target parameter). Should I add this to the comment?

Notes to self: Document doThrow, and correct the current statement that nullptr is only returned if !doThrow. And add something like see non const version.

@JohannesLorenz JohannesLorenz removed needs code review A functional code review is currently required for this PR needs style review A style review is currently required for this PR labels Apr 18, 2019
* document `dcast`
* make `dcast` not only cast exact, but also upwards
* add `dcast` test
* rename `dcast` -> `dynamicCast`
@JohannesLorenz
Copy link
Contributor Author

I just added review commits. The old dcast could only cast to exactly the same class. The new dcast will cast to exactly the same class or any parent class.

There's also a test now documenting how it works.

Check whether returned pointers from the cast are equal to the original
pointers, rather than just checking wether they are not `nullptr`.
@PhysSong
Copy link
Member

I'm not sure if it's worth handling this, but InlineAutomation inherits FloatModel as well. There are also SfxrZeroToOneFloatModel and SfxrNegPosOneFloatModel, but I don't think we should take care of them.

@JohannesLorenz
Copy link
Contributor Author

I don't think it's necessary now. Currently, I only need this to handle widgets of different types, like knobs, LEDs, ComboBoxes, for dialogs with LADSPA or Lv2 ports. InlineAutomation seems to only be used for note detuning, so I guess this can not be used for any of those widgets? The sfxr models can't be used in ModelVisitor.cpp, because that file can not see classes defined in plugins.

@JohannesLorenz JohannesLorenz merged commit c9ed6fc into LMMS:master Apr 27, 2019
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants