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

Manually merged #6090 to master #6393

Closed
wants to merge 1 commit into from
Closed

Manually merged #6090 to master #6393

wants to merge 1 commit into from

Conversation

anatoly-os
Copy link
Contributor

@anatoly-os anatoly-os commented Jul 30, 2020

@dmitrio95 could you please take a look at the compilation error? That was pretty complex merging process that broke compilation, unfortunately.

Screenshot 2020-07-30 at 15 48 40

@dmitrio95
Copy link
Contributor

Well, it looks like after 36f3298#diff-0b6703a7aa4a05e91daa6a65e267b37aR62 Staff class is now inherited from Element class instead of ScoreElement, like it was the case in 3.x branch. Therefore Staff wrapper class, being inherited directly from ScoreElement wrapper, does no longer resemble inheritance hierarchy for Staff class in libmscore, which causes this error to happen.

The simplest solution would be to make inheritance of Staff classes in libmscore and plugin API consistent — either by inheriting Staff wrapper from Element (and changing non-template versions of wrap() functions accordingly) or by reverting the change in inheritance in 36f3298.

Alternatively, we can ignore the difference in inheritance trees between libmscore and plugin API and fix that compilation error in this statement instead: either by using C++17's if constexpr there (untested, but should likely work) or by reorganizing that QmlListAccess::at() function splitting it to two template functions with the corresponding std::enable_if conditions.

I would say it would probably be better to keep inheritance tree of wrappers and libmscore objects in sync, so one the former solutions above would be more preferable. I am not sure whether the new inheritance of Staff from Element is semantically correct. Element class represents something potentially visible in a score, with its own set of coordinates, bounding box, proper relationships in parent-child hierarchy of score elements etc., while Staff represents rather a structural unit of a score, not directly mapped to any visible object (Systems are responsible for visual representation of staves and staff groups instead). However if it happens to be necessary to inherit Staff from Element in libmscore it would be better to do so with plugin API classes either.

Therefore the actual choice of the solution to this error depends on some design decisions regarding both libmscore structure and its relationship with plugin API wrappers hierarchy.

@vpereverzev
Copy link
Member

Duplicate of #7081

@vpereverzev vpereverzev marked this as a duplicate of #7081 Dec 29, 2020
@igorkorsukov igorkorsukov deleted the port_pr6090 branch September 30, 2021 08:19
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.

None yet

3 participants