Skip to content

Fix follow for multiple cursors#1111

Merged
sschmidTU merged 2 commits into
opensheetmusicdisplay:developfrom
rvilarl:fixFollowCursor
Dec 13, 2021
Merged

Fix follow for multiple cursors#1111
sschmidTU merged 2 commits into
opensheetmusicdisplay:developfrom
rvilarl:fixFollowCursor

Conversation

@rvilarl

@rvilarl rvilarl commented Dec 12, 2021

Copy link
Copy Markdown
Contributor

@sschmidTU testing with the migration I have realised that the independent control of follow for multiple cursors was not working.

Do you agree with the approach?

  • the general one, if set, forces to follow all the cursors.
  • the independent one, if set, forces to follow this particular cursor

@sschmidTU

Copy link
Copy Markdown
Contributor

Why would you ever want to follow multiple cursors?
Don't you just want to always follow the first cursor?
As far as I saw, multiple cursor were used as a utility to show additional things, not to be followed.
But I have to admit I haven't really seen a use case for multiple cursors in action from other people.

@rvilarl

rvilarl commented Dec 12, 2021

Copy link
Copy Markdown
Contributor Author

I do not know. In my app I just want to follow one of them but we did allow to configure several each with its follow config and I guess we should be consistent. Another possibility is to remove the independent configuration and state that we just follow the first one.

@rvilarl

rvilarl commented Dec 12, 2021

Copy link
Copy Markdown
Contributor Author

The current implementation follows all of them. Setting the focus to the next one that moves.

@rvilarl

rvilarl commented Dec 12, 2021

Copy link
Copy Markdown
Contributor Author

There are posible solutions but the current one makes no sense. All cursors are followed when the global option is set.
The first question is: do we want to allow more than one cursor to be follow? In my app the current play cursor is follow and the tempo cursor is not follow.

@sschmidTU

sschmidTU commented Dec 12, 2021

Copy link
Copy Markdown
Contributor

Yeah, if it's possible to follow any cursor other than the main cursor, it makes sense to let the developer choose which cursor to follow. So the .follow boolean looks good to me, simple and hopefully working without issues.
Maybe make it && .follow instead of or? Otherwise it would always follow every cursor if openSheetMusicDisplay.FollowCursor is true, right?

@rvilarl

rvilarl commented Dec 12, 2021

Copy link
Copy Markdown
Contributor Author

Yeah, if it's possible to follow any cursor other than the main cursor, it makes sense to let the developer choose which cursor to follow. So the .follow boolean looks good to me, simple and hopefully working without issues. Maybe make it && .follow instead of or? Otherwise it would always follow every cursor if openSheetMusicDisplay.FollowCursor is true, right?

You are right, and makes more sense. But in order to make does work I had to change the default to true when options are not provided (case of one cursor). Done and ready to merge!!!!

@sschmidTU sschmidTU merged commit 37f9002 into opensheetmusicdisplay:develop Dec 13, 2021
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.

2 participants