supertux
supertux copied to clipboard
SuperTux
Display music metadata in the level editor
Fixes #2617.
When navigating or hovering over menu items in the level editor music object settings file browser, information about a song's author, license, and title will now be displayed in the menu item help text at the bottom of the screen.
This pull request adds:
- A software implementation
- author, license and title data for most SuperTux music files in data/music
- Updated authorship data in the game credits and data/AUTHORS
- A realignment of the display of menu item help text that fixes cropping of long (>4 line) menu item help texts that occur for some songs. (this is in a separate commit)
The author, license and title fields are option for music files. SuperTux simply omits the display of any fields that are not present in music files, and if none of these fields are defined for a music file (like christmas_theme.music, currently), then help text does not display for that song.
Thanks for all your feedback @tobbi and @Vankata453! I pushed commits that fix all the easy code style changes, I will work on resolving your remaining feedback tomorrow.
What is the current state of this PR?
@mrkubax10 I am resolving remaining feedback on this PR. In particular, right now I am working on supporting translations.
Looking good! My only style complaint would be that m_generate_help_text_for_file sounds like it's a bool parameter, not a function. But that's not important.
May I suggest something like help_text_for_file_generator or help_text_generator
What still needs to get done here?
To Do
- WIP: Finish refactor to use item_processor
- Plan how to remove English phrases from the author field to support translation
- Add music metadata for music added to SuperTux master recently (since December)
This feature was functionally complete, but refactoring it to use the Vankata's item_processor parameter (instead of the separate specific generate_help_text_for_file) and rebasing it on the latest master is still a work in progress. Also, there are English phrases within the author field data that should be translated (some author values include a lot of info, including roles different contributors had for a given song). Finally, there are a few new music files added to SuperTux since December that need the metadata fields added to their .music files.
Current Progress on Refactoring
Last week I worked on the refactoring, and I am planning on continuing the refactor this week. UPDATE March 4: I fixed the illegal instruction exception by fixing return statements in the item_processor function. Now I am continuing to debug other issues until the refactored code works. ~~My current work is held up by a tricky illegal instruction exception issue when the item_processor callback returns in my local refactored code, which I am going to try solving this week by rebuilding and updating my development container to fix potential ABI compatibility issues. Last week I tried using Google's AddressSanitizer to search for memory corruption, but it didn't report any issues in my testing.~~
Is this done?
This is work in progress. All functionality and translations are complete. Still left to do are updating music metadata, like title, author and license, for recent music added to the game, and refactor to extract music help text item processor function out of object_settings.cpp. I'm planning on resuming my work for this PR this week.
This pull request is ready for code review and merging! I completed all the remaining work that I mentioned in my last status update 3 weeks ago. The code now uses m_item_processor, resolving feedback provided by @Vankata453. My testing passed for the level editor, as well as regression tests for story mode. It looks good to merge by me, but it would be good for this to get reviewed by other contributors.
Works great! You can see all important information about a music file:
Left 2 minor code-related comments. If you agree with any of them, I am also up to make the changes listed (if you want me to, of course). Offering that, so you don't have to do any code-related work on this no more if you don't want to, seeing the effort you put in throughout all this time, encouraged by all the previous comments.
Thanks for the feedback: I fixed both of them! I enabled "Allow edits by maintainers" for this PR so you are welcome to add commits to the PR repo if you prefer. However, I am happy to fix any additional feedback you mention here on GitHub.