Implement escape characters for lyrics and lyrics code refactor#575
Conversation
In song editor if you edited the lyrics of a note that has a space before it, the note would split. The fix is to not consider the first character in a note for splitting.
During editing of a note, the "visible whitespace" replacement characters were put into the actual lyrics data and replaced with normal whitespace only when editing ended. This is a very minor bug but it looked weird in the lyrics text area.
The word separator ' ' and syllable separator ';' can now be used in lyrics by adding an escape character '\' before them. The escape character itself can also be escaped. These escapes work in the lyrics editor, sentence lyrics editor and note lyrics editor. If a song file contains one of these characters, an escape character is automatically shown in the editor.
Moves contents of old LyricsUtils and EditLyricsUtils into a single common LyricsUtils class, with common functions for getting viewable and editable lyrics text and applying edits to voices/sentences/notes, with note splitting support. All the parsing and formatting of edit mode lyrics is now done in two generic functions: FormatAsEditable and ParseEditable. Other files were adjusted to make use of this new utility class.
The caret position finding code was counting word and syllable separators regardless if they were escaped. Now it's using ParseEditable to handle escapes like the rest of the code.
KeyDownEvents were not being fired on the lyrics text field (maybe a Unity upgrade issue). The whitespace replacement functions now explicitly support "fixing" newline replacements on text with partially replaced newlines. This means they can be used directly when adding or removing newlines in the lyrics text field and we don't need KeyDownEvents.
|
Thank you for this PR. I think it is a useful feature. I only have few questions:
I will try to resolve merge conflicts with MM in the next integrate. Don't worry about that. |
|
The existing tests pass. Play mode tests seem to be very clumsy to write so I'm not sure if I want to write any. |
Ok, I will give it a try. |
|
Maybe edit mode tests would be useful for the LyricsUtils. |
|
I will try to integrate this change in USPlay, then Melody Mania, and then merge back Melody Mania into USPlay. This should align existing tests and code base. |
| /// Splits the text into sentences and syllables and applies it to the notes of the voice, | ||
| /// padding with empty strings if necessary. | ||
| /// </summary> | ||
| /// <param name="editModeText"></param> |
There was a problem hiding this comment.
I prefer JavaDoc resp. JsDoc format, because I think the XML stuff is rather verbose. Especially because it repeats <summary> always.
Furthermore, empty parameter descriptions are not helpful and tend to diverge with code over time. Please remove. The empty ones.
There was a problem hiding this comment.
Anyway, overall this works well and I do not want to be nitpicky with the comments.
Still in the future, prefer to use JavaDoc (I know this is not very C#-ish) and avoid empty parameter descriptions.
Support for backslash as lyrics escape character was merged in UltraStar-Deluxe/Play#575. Update the webpage to mention this feature.
Support for backslash as lyrics escape character was merged in UltraStar-Deluxe/Play#575. Update the webpage to mention this feature.
2 participants
What does this PR do?
As mentioned in #552, the current lyrics editor does not support spaces inside syllables, because space has a special meaning as a word separator. This PR implements escape characters for the lyrics editor, so that you can write
К\ теinstead ofК теif you want a literal space character inside the syllable. Because;is also a special syllable separator and adding\as an escape character makes it a special character too,;and\can also be escaped. The full list of escape sequences are therefore:\becomes\;becomes;\\becomes\Newline
\nhas no escape sequence because it can't be escaped. Word and syllable separators are special characters in the US Play editor but newline is a special character in the UltraStar TXT format and it can't appear in lyrics.While implementing this feature, I noticed that the same escape processing logic had to be repeated in at least 5 different locations in the code, as you can see in the commit d46842d. I refactored the lyrics editing code so that all lyrics text processing now happens in a common file
LyricsUtils.cswhich has common utility functions for parsing and formatting lyrics text (commit eb98813). This utility class is then used by the big lyrics text field, the sentence level and note level text fields as well as the lyrics importer and hyphenation codes.As usual when refactoring code, a number of small bugs were also found and fixed, such as
More information in commit messages and code comments.
Closes Issue(s)
#552
More
Additional Notes
I have understood that in Melody Mania 1.5.2 there is a new option to change the word separator character. I don't have access to Melody Mania 1.5.2 and I don't know how that's implemented, but these changes will probably conflict with that one. At least the code in this PR assumes that the real word separator and the one that the user writes on their keyboard is the same. If the user changes their word separator to pipe
|they probably don't want the pipe to end up in their song files. Now that spaces can be escaped, I don't know if there's really a need for that option anymore. Please tell me if this feature still needs to be implemented.These changes were thoroughly tested by editing lyrics in the big lyrics text field, sentence level fields and note level fields, as well as loading song files with special characters. They were however not tested with an imported LRC file or the speech recognition feature but just assumed to work as before.