-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
More npc locations revived #57483
More npc locations revived #57483
Conversation
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 |
Finally, the PR looks ok at first glance :) |
It doesn't quite to me - looks like line endings are changed in one file, and there are several other large files diffs here, which I can't look at much on phone. Also, the filenames contain spaces. |
But at least there are no extraneous commits, so a step in the right direction ;) |
It's a lot better than before! I feel like there are 3-4 things going on here that could/should still be broken up though for easier review and testing, but it'll be easier to see once line endings are fixed. |
Shelter updates
I merged your PR, and I don't see no problem with the spawn adjustments. Don't worry about the reviews, I just hope you enjoy your week. |
I resolved the conflict listed in the scenario file. |
Ok so to review a gigantic mapgen PR with 4k lines I need you to take screenshots of the locations in game, please. I'll review the non mapgen parts now and come back and review the maps once I can compare them to the screenshots. Thanks! |
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.
OK I've stopped reviewing this because there are numerous places where the tests have told you that there is a mistake and how to fix it and you haven't fixed them. That's what is stalling out this PR. I've corrected them up to where I stopped but there may be more. Please correct the errors that are causing the tests to fail and let me know and I'll review.
data/json/npcs/other/homeless/group_camp/TALK_homeless_broker.json
Outdated
Show resolved
Hide resolved
data/json/npcs/other/homeless/group_camp/TALK_homeless_broker.json
Outdated
Show resolved
Hide resolved
data/json/npcs/other/homeless/group_camp/TALK_homeless_broker.json
Outdated
Show resolved
Hide resolved
Co-authored-by: Maleclypse <54345792+Maleclypse@users.noreply.github.com>
@Maleclypse I'm sorry I didn't get to this after we spoke on Discord, I was on vacation. |
No stress. :) |
@Maleclypse I fixed the errors in the altered JSON files, the only errors now come from the Clang 12 build matrix, giving some errors on item density compared to max density ( CHECK( item_density <= max_density ) ). For how little the PR deals in items, I don't know why the matrix finds that unacceptable. I also don't know how to fix 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.
These are great additions. I appreciate you fixing the errors that were blocking this.
* origin/master: More npc locations revived (CleverRaven#57483) Add FBI Paranormal Investigator profession to Xedra Evolved (CleverRaven#59316) allow using nearby items to start or quench fires (CleverRaven#59317) Fix summon time (CleverRaven#59318) Self aware sidebar health widgets (CleverRaven#59319) Add test to prune item density known bad list (CleverRaven#59324) Optional widget color scale `breaks` (CleverRaven#59336) Sidebar widgets rel. weight carried / overburdened (CleverRaven#59330) [DinoMod] stegoceras and egg and dung rebalance (CleverRaven#59343) Add FIREWOOD to twigs and leaves, adjust twig volume (CleverRaven#59345) fix: guns spawning with double shoulder straps (CleverRaven#59346) Appease clang-tidy: move cata::mdarray to heap (CleverRaven#59335) Playtest fixes + Shaman doesn't override medical resident Miner, fisher & four hunters Shaman & Backpacker
Summary
Content "Add more NPC locations"
Purpose of change
The purpose of this change is to add more places where NPC's can spawn, either alone or in groups; providing a greater variety within the world of Cataclysm.
This adds more NPC's in various places. Currently, two random lone NPC's will spawn at evac shelters and homeless camps. This also adds a homeless camp with a group of survivors; so far including a chain of missions and an ability to trade. Additionally, this adds several new varieties of evac shelters.
Essentially, this is a revival of pull request #55196, with great thanks to onura46 for resurrecting it.
Describe the solution
This branch contains the same content as #55196, adding various NPCs to the world of Cataclysm. onura46 added cooldown timers for certain rare products sold by the chemist, fleshed out dialogue, and had the player spawn in the basement of the evac shelter for the “Infested Shelter” challenge start.
Describe alternatives you've considered
None
Testing
According to onura46, all code runs smoothly and works as intended.
Additional context
The original content was inspired by two Discourse boards, seen below:
https://discourse.cataclysmdda.org/t/more-evac-shelter-varieties/27336/9
https://discourse.cataclysmdda.org/t/npc-s-what-do-you-want-changed-and-improved/27102/18