Expand and improve randomly-generated music descriptions #34053
Conversation
It would actually make translation relatively easier; previously three fragments were simply concatenated together, disregarding grammar variation among languages...
We already have replacement tags in the form |
ZhilkinSerg
added
[C++]
|
It looks like currently the |
|
There's still a bit of refactoring opportunity to have the npc talk tag replacement logic re-use the parsing from the snippet logic—e.g. passing a lambda to |
|
Some snippets-related tests are failing now. |
| @@ -119,7 +141,7 @@ std::string snippet_library::random_from_category( const std::string &cat ) cons | |||
| const int index = rng( 0, count - 1 ); | |||
| auto iter = iters.first; | |||
| std::advance( iter, index ); | |||
| return get( iter->second ); | |||
| return expand( get( iter->second ) ); | |||
There was a problem hiding this comment.
It seems that the unit test wants this function to return the unexpanded snippet.
There was a problem hiding this comment.
I've updated the test to check something slightly different, and in the process I discovered:
- it was testing two tags that were unused,
<drop_weapon>and<catch_up> - there was a snippet using the nonexistent tag,
<catch_up>.
I've removed the relevant snippet, and removed the old tags from the test.
As a follow-up, I'd like to write a test that iterates through every snippet and checks that any tags it references are either a special tag (like <yrwp>) or exist in the snippets library. That should prevent any undefined references from sneaking in.
| @@ -410,7 +410,6 @@ | |||
| "Stay with me, <name_g>!", | |||
| "Catch up, <name_g>!", | |||
| "Keep up!", | |||
| "Come on, <catch_up>!", | |||
There was a problem hiding this comment.
Perhaps it was intended to use <keep_up> here.
There was a problem hiding this comment.
Yep, but I decided to delete it instead because the capitalization would be wrong, e.g. Come on, Keep up!!
| "<let_me_pass>", "<done_mugging>", "<happy>", | ||
| "<drop_weapon>", "<swear>", "<lets_talk>", |
There was a problem hiding this comment.
<drop_it> was probably the intended value.
|
I've updated the PR description to reflect the new code, and I've addressed all review comments. Please take another look when you can :) |
Summary
SUMMARY: Content "Expand randomly-generated music descriptions"Purpose of change
The randomly-generated music descriptions were both inconsistent (generating descriptions with syntax errors like "neo -tune"), and were missing a period at the end of the description.
Describe the solution
I've extended the system of replacements in the snippets system so that all snippets can use the
<other_snippet_id>syntax, not just NPC text, and then converted the music description logic to use it.Describe alternatives you've considered
Perhaps this system isn't really necessary, and we'd rather stick to hand-authored lists of snippets rather than more complex generators such as this one.
The rules I've added to the JSON for describing music could be special-cased and written in C++ directly, rather than building a system for expanding text described in JSON.
Instead of reusing the snippets system for this sort of "expanding text", we could introduce a new JSON type ("snippet_generator"?) which could contain all the rules for generating a given type of snippet, like this music generator. This would prevent potential namespace overlap issues, since all the rules would be self-contained, but would prevent re-using generators in multiple places.
These more complex and more fragmented strings might present challenges for translators.
We could expand the system with additional capabilities, such as pluralization, article agreement (a/an), and state tracking, all of which Tracery implements. I've chosen to avoid those and write around these limitations for now to keep things simple for a first cut.
Additional context
Improved music descriptions:
