-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
NPC snippets overhaul, part 1. talk_tags.json
#77372
Conversation
What's your reasoning for moving the file? |
All these snippets were split into 5 files. It is more convenient to work with them in a separate directory. |
I don't fully understand the rationale. For example, you're removing the I think you should provide a complete rationale for every tag you're deprecating. |
The thing is, the removed snippets didn't create any particular diversity by replacing a couple words somewhere in the NPC's backstory. They caused more pain than benefit, it's hard to tell if a replacement will be appropriate without knowing which line will be randomly selected, and it's hard to edit a snippet to fit all of its occurrences without knowing the context in which it might be used. ( Sometimes snippets exist for few dialog strings, like To summarize some rationale, perhaps it's to reduce bloated and complicated infrastructure, hopefully it makes sense. |
please remove snippets that no longer exist from this test Cataclysm-DDA/tests/npc_test.cpp Lines 205 to 217 in 92f70f2
|
it seems you missed something, the test is still failing |
I'll add the new snippets to the test back later. And if this PR will be merged, I assume the tests for the other PRs will also be successful after the restart? |
If you resolves the test for all snippets removed in the rest of PRs, then yes, merging this PR and then updating the branch for the rest would solve the problem |
now the error is due to a ‘bad tag’, I'll take a look at what's wrong a little later. um, I moved |
Spell checker encountered unrecognized words in the in-game text added in this pull request. See below for details. Click to expand
This alert is automatically generated. You can simply disregard if this is inaccurate, or (optionally) you can also add the new words to Hints for adding a new word to the dictionary
|
Summary
Infrastructure "NPC snippets overhaul"
Purpose of change
Some short snippets (
<very>
,<ill_die>
) could create strange sentences out of place. Or be used ambiguously (<swear>
). Or the snippet became a mishmash and lost its original meaning (okay
).Some snippets were rarely used or not used at all, hanging around as dead weight.
Some snippets, on the contrary, could be expanded or slightly reworked for a better result.
Describe the solution
Move talk_tags* to a separate folder (which is why PR counts so many changes).
Deprecate the following snippets:
<shitty>
,<dumb>
,<very>
,<really>
,<monster>
,<happy>
,<sad>
,<get_lost>
,<ill_kill_you>
,<ill_die>
,<move>
,<fuck_you>
<okay>
- in favor of<acknowledged>
<swear>
- renamed to<freaking>
to eliminate ambiguity, because the snippet was used both as an adjective and as a stand-alone curse word. Now it is used only as an adjective before a noun.The occurrences in the text of these snippets have been replaced, and the definition itself has been moved to a separate file in
data/json/obsoletion_and_migration_0.I/
. Just in case, so that simple replacement of snippets does not cause errors in third-party mods or anywhere else.Some snippets have been merged into one pool, creating a hierarchy. For example,
<general_danger_h>
uses snippets from<general_danger>
in addition to its own unique ones. Or<kill_player_h>
is now the same as<kill_npc_h>
.Some snippets have been split into multiple categories that will be concatenated to create more variations:
<heal_self>
,<kill_npc_h>
,<greet>
, noise warnings.<swear!>
is now a complete sentence:For crying out loud!
,Damn<punc…!>
,Are you kidding me?!
.3 new snippets with punctuation, chosen randomly:
<punc.!>
,<punc…!>
,<punc.…>
.Describe alternatives you've considered
Testing
Ummm... all these changes are split into 5 parts, but they should be tested together, merging separately will most likely not even allow the game to load the world. And it is possible that its will break the build, since one of the header files has a removed snippet, which was replaced in... part 5?
Wait, I actually created a patch of all the changes before breaking it, here it is:snip.zip. Nah, it's a bit outdated without a couple of minor fixes.Nothing much has changed from the player's point of view, but here are some screenshots:
Details
^^^All snippets, including deprecated ones^^^
Additional context
It would be fun to have snippet categories for factions instead neutral-friendly and hostile.