-
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
[CR] remake of the refugee center shopkeeper NPC #52374
Conversation
Hmm, this looks really good. I would have suggested keeping the random NPC element in, previously, but really since little else in the center does that I guess there's no reason to keep the legacy stuff. I will review your dialogue and offer some feedback in a bit. Glad to see someone else givin the center some love. |
Thanks for the kind words! If you like, I'm fine changing them to a random name. I'd just have to change some of the lines referencing their name in particular, and give them a random appearance. I only went with the static name because I've seen it repeated elsewhere, ala Rubik. In other words, this should be ready for review. The new NPC should offer everything that the current one does, in addition to some bugfixes. I'm particularly interested in if there's an accepted nomenclature for dialogue IDs and variables, as right now I just used what felt right. I'm open to change pretty much any of the internal IDs. |
data/json/npcs/refugee_center/surface_staff/Smokes/free_merchant_shopkeep_missions.json
Show resolved
Hide resolved
Sorry for the bump, but this has been up for a good while and I wanted to make sure that it wasn't like, held up for something I needed to address. Is there anything I should be doing, or is this just waiting for review once someone gets the time? |
This is good stuff. I feel like the refugee center has a lot of potential to be a major quest hub, even more than it already is. |
For my own angle this is just delayed by me not being around for much of anything lately, I've been reading it in bits here and there and will merge it (or suggest changes) soon unless someone beats me to it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still reviewing, posting this as I'm going back to work. Starting up on merchant_shopkeep_talk next.
data/json/npcs/refugee_center/surface_staff/Smokes/free_merchant_shopkeep_itemlist.json
Outdated
Show resolved
Hide resolved
"om_special": "hub_01", | ||
"reveal_radius": 1, | ||
"random": true, | ||
"search_range": 240 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will probably be too short of a search range. Just to check - this mission is outside the usual mission chain, correct? It can be very tough if the search doesn't find hub01.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's outside the normal mission chain, yeah. This is actually cut-and-paste from the existing quest def except for the dialogue, which is different - it has a search range of 240 in the current iteration of the game as well. What should I change it to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with 400 with the teamster in the back room and we still get "can't find it" errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn. What should I use in the meantime, then? 400, something higher...
data/json/npcs/refugee_center/surface_staff/Smokes/free_merchant_shopkeep_missions.json
Outdated
Show resolved
Hide resolved
data/json/npcs/refugee_center/surface_staff/Smokes/free_merchant_shopkeep_missions.json
Show resolved
Hide resolved
@Ilysen : Just here to say thank you for this! The shopkeeper had been feeling stiff, and it's great to see them getting an update! |
appreciated! I had the same opinion, which is why I'm working on it - the writing of the game feels like it's been getting better with time, and so older parts felt a lot more out of place as a result. the merchant is definitely the most egregious existing example, at least to me, although I'm sure someone else could write them better than I could. I'd like to follow up on this PR with further additions over time, as long as everything goes okay. |
Fingers crossed this was ready. I'll put out a feeler on the subreddits for input in a few days and decide next steps from there. Worst case scenario, if something breaks, a hotfix or revert is fine. If there ends up being something gamebreaking or wrong, give me a holler here. |
Nothing's ever ready, we fix the details as they are caught |
This is so good, I felt the need to find my creds to log in to Github just to thank you. |
Hey nice to see you again faefux |
Remove duplicated EXO_ flags. Rename two EOCs to prevent one from overwriting the other. Remove old duplicate mission definitions (duplicated in CleverRaven#52374) Remove the old mine entrance overmap special (the new one was correctly overwriting it) Remove some duplicate overmap terrain defs. Remove some duplicate basecamp recipe groups. Remove a duplicated widget.
Remove duplicated EXO_ flags. Rename two EOCs to prevent one from overwriting the other. Remove old duplicate mission definitions (duplicated in CleverRaven#52374) Remove the old mine entrance overmap special (the new one was correctly overwriting it) Remove some duplicate overmap terrain defs. Remove some duplicate basecamp recipe groups. Remove a duplicated widget.
Remove duplicated EXO_ flags. Rename two EOCs to prevent one from overwriting the other. Remove old duplicate mission definitions (duplicated in CleverRaven#52374) Remove the old mine entrance overmap special (the new one was correctly overwriting it) Remove some duplicate overmap terrain defs. Remove some duplicate basecamp recipe groups. Remove a duplicated widget.
Remove duplicated EXO_ flags. Rename two EOCs to prevent one from overwriting the other. Remove old duplicate mission definitions (duplicated in CleverRaven#52374) Remove the old mine entrance overmap special (the new one was correctly overwriting it) Remove some duplicate overmap terrain defs. Remove some duplicate basecamp recipe groups. Remove a duplicated widget.
This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there: https://discourse.cataclysmdda.org/t/i-am-missing-the-npc-named-smokes/28586/6 |
Summary
Content "The refugee center merchant has been remade into a new NPC named Smokes."
Purpose of change
The Free Merchants are among one of the oldest factions in the game, and a lot of the dialogue from the "front" NPCs in particular (who represent the faction's interactions with the player) can feel stiff. With the advent of Rubik showing that fleshed-out and believable NPCs can serve as fronts for the player, I really wanted to try my hand at updating the refugee center some more to fit that.
Describe the solution
The old merchant has been completely replaced by a new NPC named Smokes. His actual name is Gavin Wheeler, but the Free Merchants have given him the nickname 'Smokes' because he's an ex-firefighter and wears his helmet. Smokes offers the same features as the current merchant, including the same missions and shop inventory. However, the character of Smokes has been completely rewritten from the old merchant, with an aim to make someone that feels legitimate and has distinct personality traits.
Compared to the old shopkeeper, Smokes is a lot more empathetic about the situation people are in. They're also quite happy to talk about things and explain how things are going at the center, as well as giving far more details about the rationale for why the Free Merchants are doing certain things, information about how their setup is handled, and so on. These things aren't massive mechanical changes, but I feel like they go a long way towards making the character feel more believable.
Many more player dialogue options were also added, with the goal of letting people talk to Smokes about certain things in detail, like asking why they react to certain things or giving unique options for certain traits, as well as causing dialogue to change in response to different events. Ultimately, I'd like for Smokes to be a good way to introduce a new player to the NPC system, as well as a way for those players to learn more about the world with his dialogue.
For compatibility purposes, right now, Smokes is a completely different NPC in the json than the old shopkeeper. Existing saves should be able to interact with the old merchant in the exact same way as normal; only newly generated refugee centers will have Smokes. I figure this is good for making sure it doesn't break peoples' saves, and could probably stay until 0.G or thereabouts.
All existing refugee center missions haven't been changed, except for the breadcrumb to Hub 01, which has had the merchant dialogue updated to match Smokes's personality. They've also been moved to their own file, instead of being in a giant file lumped together with the shopkeeper.
This NPC should be complete for this stage. I'd like to follow up the PR by updating some of the evac center's missions as well, but that's out of scope for this one.
Describe alternatives you've considered
Replacing the old shopkeeper's code with the new NPC. This was problematic because only newly-generated NPCs will have the correct name, appearance, and loadout, even if they're defined in the json. For now, I've decided to split the two, with the goal of eventually removing the old merchant code once Smokes is in a complete state. I can still replace the code for the old merchant if need be, though.
Having a different name. This NPC went through several iterations while I was developing him - it started as "Galvin", moved to "Gavin Wheeler", then became "Smokestack", and finally "Smokes". All of these things are viable; I kept Gavin Wheeler as his actual name, but Smokes felt like the most natural nickname for him to have, and I felt like having their name as a distinct single word catches the player's eye and hints to them that this person is important.
Testing
I visited Smokes and went through their dialog trees with specific traits, items, etc. to make sure that all subjects worked (unless I missed any). I accepted the first mission, completed it with debug commands, and then handed it in, which gave me the correct reward and offered the second mission. I used Debug Mind Control to make sure that their traits, stats, and skills were right.
Additional context
I'd really like to hear input on the dialogue and consistency with the character. This is also my first time touching NPC code, so I may have gotten something wrong. Please give me a holler if you have any concerns.
This is probably to #51320 and #30579.
Fixes #49233.
Fixes #49488.