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

WIP: Feature/duet edit #35

Closed
wants to merge 4 commits into from
Closed

WIP: Feature/duet edit #35

wants to merge 4 commits into from

Conversation

nieknooijens
Copy link
Contributor

@nieknooijens nieknooijens commented Jun 9, 2019

added in a feature to support dual track edit.

closes #30

@@ -470,7 +493,7 @@
</property>
<widget class="QMenu" name="menuPreferences">
<property name="title">
<string>P&amp;references</string>
<string>Pr&amp;eferences</string>
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm i think the ampersand shouldn't be here?

@@ -625,7 +648,7 @@
</action>
<action name="actionLyricsToFile">
<property name="text">
<string>Lyrics to fi&amp;le...</string>
<string>L&amp;yrics to file...</string>
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm i think the ampersand shouldn't be here?

@@ -758,7 +781,7 @@
</action>
<action name="actionAboutQt">
<property name="text">
<string>About Qt...</string>
<string>About &amp;Qt...</string>
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm i think the ampersand shouldn't be here?

@@ -768,7 +791,7 @@
</action>
<action name="actionLyricsFromLRCFile">
<property name="text">
<string>Timed lyrics from LRC/Soramimi file...</string>
<string>&amp;Timed lyrics from LRC/Soramimi file...</string>
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm i think the ampersand shouldn't be here?

@Baklap4
Copy link
Member

Baklap4 commented Jun 11, 2019

Will take a look at the other files once i'm back home since i can compile it then etc..

Copy link
Member

@Baklap4 Baklap4 left a comment

Choose a reason for hiding this comment

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

Looking good
Some deprecation warnings but they were not introduced in this pr.

I'm missing one feature for this which is the 'current phrase' feature. Once loading a song and playing it it'll automaticly update, however if you switch the lyrics track, this won't get updated anymore resulting in: Current phrase: -

I think this should be done in the on_comboBoxTrack_currentIndexChanged() method.

Speaking of that method, i'm missing a default case statement within the switch. It should do the same as case 0 also break it.

Hmm also another note, what happens if both players need to sing the same sentence? should you define it twice or is there some other key for it?

@nieknooijens
Copy link
Contributor Author

nieknooijens commented Jun 12, 2019 via email

@Baklap4 Baklap4 changed the title Feature/duet edit WIP: Feature/duet edit Jun 12, 2019
@Baklap4 Baklap4 marked this pull request as draft January 6, 2023 12:01
@Baklap4
Copy link
Member

Baklap4 commented Jan 6, 2023

Converted to Draft PR as it's not finished yet :)

@Baklap4
Copy link
Member

Baklap4 commented Jan 8, 2024

Going to close the PR as it's been left abandoned. Please reopen when new changes are ready

@Baklap4 Baklap4 closed this Jan 8, 2024
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.

Option to create duet/multi vocal songs
2 participants