Skip to content
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

[DONE] Player Faction Base #24040

Merged
merged 86 commits into from Jul 16, 2018

Conversation

@acidia
Copy link
Contributor

commented Jun 18, 2018


Implementing something that I had intended to do a few years ago, this allows the player to take two NPCs and actually build a base in any field from which you can send countless NPCs on their own adventures/missions. The biggest limiting factor is the rate at which you can recruit people. You could stick with two companions for everything but it isn't practical given the construction totals for the central camp building alone are
-36 days of labor for your carpenter to build everything between all of the missions (meaning he is going to be borrowing your hammer/shovel/wrench while he works if he needs it, you should make 2+ if you are running multiple carpenters or need to do stuff yourself)
-502 2x4s
-1228 nails
-86 pipes
-460 logs
-351 sticks
-40 rocks
-172 rope/makeshift ropes
-traces of other construction materials
You can send your companions to gather materials for the next upgrade so you don't need to gather anything yourself unless you want to speed up the process (a rare item like a well pump might take forever for them to find). Expansions like farms/garages/forges/bakeries/schools will be built in the OM squares around your central camp and will become necessary to produce materials in large enough quantities or trained recruits to fulfill the growing requirements.

Known issues:
[DONE] Doesn't check distances from other camps or the terrain where you ask the camp to be built yet
-Missions have a good deal of redundant code still
[DONE-ish]When you upgrade an OM tile, it wipes items and vehicles since it is pulling a new JSON building in, a priority fix. Will prevent you from upgrading a OM tile when a vehicle is present simply because the engine doesn't support collision mechanics well when merging OM tiles.
[DONE] Time listed for missions is wacky/incomplete
[DIFFERENT PR] Companions don't take your tools with them when they leave on mission yet

Current Missions:
Upgrade camp - take an NPC and have him build the next level OM building consuming the listed materials
Gather Materials - send an NPC to search for basic materials used in the next upgrade
Collect Firewood - just brings in heavy sticks and other material so you don't have to
Menial Labor - FINALLY this has one of your companions sort all material in the surrounding OM tiles so you don't have to... not great at the moment because I used crates to store the items in so you have to be standing next to them to see the stuff to craft with, will fix soon
Distribute Food - convert food into a faction "currency" to pay your companions as they go on missions.

My major goal is to get the farm expansion finished so that I can start using calories as your currency to pay your workers... you know how you can check your faction stats in the '#' menu and it has a "Food Supply: Starving", that is pulling from a simple int that will literally equal the number of total calories/nutrition's that you have given to your followers. For a low danger mission you might be paying them 12 nutrition/hour. I'll let demand for food be high enough that a food surplus won't be as much of an issue.

Forum: https://discourse.cataclysmdda.org/t/spoilers-your-base/9737/32

acidia added some commits Jun 18, 2018

@John-Candlebury

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2018

Bothers me that the base that won is the flimsiest one. But well that's the danger of democracy I guess.

Great job doing this Acidia.

@@ -6,6 +6,7 @@
#include "item.h"
#include "string_formatter.h"
#include "output.h"
#include "overmap.h"

This comment has been minimized.

Copy link
@BevapDin

BevapDin Jun 18, 2018

Contributor

You don't need to include "overmap.h" here, a forward declarations of oter_t and oter_str_id is enough to use the functions of the string_id.

void talk_function::become_overseer( npc &p )
{
add_msg( _( "%s has become a camp manager." ), p.name.c_str() );
p.name = p.name + ", Camp Manager";

This comment has been minimized.

Copy link
@BevapDin

BevapDin Jun 18, 2018

Contributor

Sholdn't the title be translated as well? (Isn't "Camp Manager" supposed to be a title?)

This comment has been minimized.

Copy link
@acidia

acidia Jun 19, 2018

Author Contributor

Yes... but for the moment factions are still determining roles based on title instead of a separate variable. 100% should be fixed so that it is translation compatible and renaming NPCs is cool but that requires going through and ensuring compatibility for all older outstanding missions.


bool talk_function::camp_gathering_return( npc &p, std::string task )
{
//minimum working time is 3 hour

This comment has been minimized.

Copy link
@BevapDin

BevapDin Jun 18, 2018

Contributor

Why are you repeating this over and over? And why is it not in the function talk_function::upgrade_return? Why is it there anyway - the function call already contains the 3 hours information?

} else {
itemlist = "forest" ;
}
std::string output = "Notes: \nSend a companion to gather materials for the next camp upgrade.\n \n"

This comment has been minimized.

Copy link
@BevapDin

BevapDin Jun 18, 2018

Contributor

Needs to be translated.

output = output + "> " + e.second + "\n";
}
std::vector<std::shared_ptr<npc>> npc_list = companion_list( p, "_faction_camp_gathering" );
output = output + " \nRisk: Very Low\n"

This comment has been minimized.

Copy link
@BevapDin

BevapDin Jun 18, 2018

Contributor

Needs to be translated.

comp = comp + elem + "\n";
}
std::vector<std::shared_ptr<npc>> npc_list = companion_list( p, "_faction_upgrade_camp" );
comp = _("Notes:\n" +

This comment has been minimized.

Copy link
@BevapDin

BevapDin Jun 18, 2018

Contributor

The _ only applies to C-strings and is usually not working on generated strings, only on string literals.


std::string talk_function::om_upgrade_description( npc &p, std::string bldg ){
const recipe *making = &recipe_id( bldg ).obj();
if( making == nullptr ) {

This comment has been minimized.

Copy link
@BevapDin

BevapDin Jun 18, 2018

Contributor

This will never be true. making has been assigned the address of something (the result of applying the "address of" operator &). The address of something can never be null.

Note that recipe_id::obj will show a debug message if the recipe id is not valid (see string_id<recipe>::obj in "recipe_dictionary.cpp").

@@ -1370,6 +1371,7 @@ class player : public Character
int radiation;
unsigned long cash;
int movecounter;
bool death_drops;// Turned to false for simulating NPCs on distant missions so they don't drop all their gear in sight

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 10, 2018

Contributor

If only NPCs use it and players don't, it shouldn't be in player.
Instead, you should mark the place_corpse function as virtual (as in, virtual bool place_corpse, only in header), then create an override in npc, like this:

// npc.h
class npc {
    ...
    void place_corpse() override;
    ...
}
// npc.cpp
void npc::place_corpse() {
    if( death_drops ) {
        player::place_corpse();
    }
}

Also, you forgot to save and load it in savegames.

This comment has been minimized.

Copy link
@acidia

acidia Jul 10, 2018

Author Contributor

Ah, ok. Yup, don't need it to be saved since it is turned on and off in the method that utilizes it so I don't accidentally leave it on for NPCs or monsters.

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 10, 2018

Contributor

don't need it to be saved since it is turned on and off in the method that utilizes it

Then you should have it in the function, not pass it around in the object itself.
Have a set or vector of NPCs that need death processing at specific spot. Or a vector of std::pair<tripoint, std::shared_ptr<npc>> if they can die at different positions.

This comment has been minimized.

Copy link
@acidia

acidia Jul 10, 2018

Author Contributor

Hmmm... so it's an interrupter or whatever you call it, for combat deaths. The only thing I call for actual combat is:
const auto att = random_entry( attacker );
monster *def = random_entry( group );
att->melee_attack( *def, false);
Everything is handled by melee_attack() including dropping the corpse in the player om without the interrupter. Would it be best to add another bool argument to melee_attack() that would get pushed to the monster getting hit which would go though whatever else till it arrived to its die() function?

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 11, 2018

Contributor

This won't work well: NPCs and monsters still have local coords and melee_attack will trigger local-coord-dependent behaviors.
Special casing every single event isn't feasible. You'll need to drop the actual combat mechanics and replace them with approximations.

Otherwise there will be a lot of strange bugs like

  • Appearing items from destroyed NPC weapons and armor
  • Acid bursts from acid defense when hitting some zombies
  • Blood splatters from critters that NPCs are fighting

The actual proper solution would be to drop the g pointer and have NPCs and monsters carry their own game and map pointers and simulate their actions on those "virtual" maps. But this would be a lot of work.

There is no easy way of splitting off common sections of complex combat sections, so approximations are the only sensible way of doing this at the moment.

std::vector<std::pair<std::string, tripoint>> om_region = om_building_region( p, 1 );
for( const auto &om_near : om_region ){
if ( om_near.first != "field" && om_near.first != "forest" && om_near.first != "forest_thick" &&
om_near.first != "forest_water" && om_near.first.find("river_") == std::string::npos ){

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 10, 2018

Contributor

Don't compare strings, this is risky. Especially don't ever use string::find to compare strings - this may randomly fail without any warning, since the suffix is added automatically and may change in the future.

For fields and forests, use the hardcoded values in set_oter_ids(), in overmap_cpp. They are all in the form of ot_forest.
For rivers, use is_river( ter_id ).

/// Takes all the food from the point set in camp_menial_sort_pts() and increases the faction food_supply
bool camp_distribute_food( npc &p );
/// Returns the OM tiles surrounding the camp, @ref purge removes all tiles that aren't expansions
std::vector<std::pair<std::string, tripoint>> om_building_region( npc &p, int range, bool purge = false );

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 10, 2018

Contributor

This shouldn't return raw strings but oter_str_id.

}

return true;
}
if ( cur_key.find("Recover Ally, [") != std::string::npos && cur_key.find("] Expansion") != std::string::npos ){

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 10, 2018

Contributor

Avoid string::find like the plague it is.
Instead store mission type and description separately and only compare the type here, not the entire description.
You can store them as pairs or only store the type and have a function that adds some suffix to the type.

c_mission.mission_id == id && c_mission.role_id == p.companion_mission_role_id ){

available.push_back( elem );
} else if( contains && c_mission.mission_id.find( id ) != std::string::npos ){

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 10, 2018

Contributor

Don't store this as a substring. Add this to the struct, as a totally separate field.
Same for direction - it certainly shouldn't be encoded as a string. A best candidate for direction type would be om_direction::type from omdata.h - it already has all the functions like getting name, offsetting a point by it, opposite direction etc.

return available;
}

bool companion_sort_compare( npc* first, npc* second ){

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 10, 2018

Contributor

Both npcs should be const npc &, not npc *.

if( scores[0].combat > scores[1].combat ){
return true;
}
return false;

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jul 10, 2018

Contributor

return scores[0].combat > scores[1].combat;

//tmpmap.save();
}

overmap &om = overmap_buffer.get( om_tgt.x, om_tgt.y );

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Jul 12, 2018

Contributor

Variable is not being used.

);
popup( "%s", slide_overview );
slide_overview = _("Faction Camp Tutorial:\n \n"
"UPGRADING: After you pick a site you will need to find or make materials to upgrade the camp further to access new "

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Jul 12, 2018

Contributor

Can you color this headers in tutorial? with <color_light_red>UPGRADING</color> or <good>UPGRADING</good>?

std::string buffer = _("Warning, you have selected a region with the following issues:\n \n");
if( forests < 3 ){
display = true;
buffer = buffer + _("There are few forests. Wood is your primary construction material.\n");

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Jul 12, 2018

Contributor

Maybe we should color these messages to show which is bad?

And also make some pivot table showing location stats?

Forests: <good>enough</good>
Plains: <good>enough</good>
Rivers: <bad>low</bad>
Swamps: <critical>none</critical>
@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2018

Bug:

I've started first phase of Upgrade camp with single companion using large tent then I talked to Camp Manager and cycled through direction tabs, then Recover Ally from Upgrading tab was duplicated. Restarting conversation removes duplicate line until I cycle through tabs again.

image

image

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2018

Below is the link to compiled Windows x64 tiles version for those who want to test, but cannot compile by themselves (it does not include gfx folder, but you can copy tilesets from current main version):

cdda_acidia_base_camp_2018-07-12.zip

@acidia

This comment has been minimized.

Copy link
Contributor Author

commented Jul 13, 2018

Got rid of the double listing of priority missions bug if you tabbed until you looped. I made the subtitle in the tutorial green and the reference to menial labor mission yellow. Any other color suggestions?

acidia added some commits Jul 14, 2018

@kevingranade kevingranade merged commit 9752dd7 into CleverRaven:master Jul 16, 2018

1 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
gorgon-ghprb Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@UristLikot

This comment has been minimized.

Copy link

commented Jul 16, 2018

@acidia
Found a bug with translations:
fylp0dchbnm
baccdemlw0g
Skill level check only works on English version of the game.

@acidia

This comment has been minimized.

Copy link
Contributor Author

commented Jul 16, 2018

How do I even change the language? I go to options, interface, and picked the only Cyrillic option and saved but everything is still in English. Tried restarting and it remembered the language that I picked but nothing is translated still.

I think it is a simple solution of changing the hand full of occurrences of
making->skill_used.obj().name()
to
making->skill_used.obj().ident().c_str()

@UristLikot

This comment has been minimized.

Copy link

commented Jul 16, 2018

Are you using launcher? I think it might reset game to system locale.

@acidia

This comment has been minimized.

Copy link
Contributor Author

commented Jul 16, 2018

Nah, I'm compiling it from scratch. Can you compile?

@UristLikot

This comment has been minimized.

Copy link

commented Jul 16, 2018

Sure, but not right now.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2018

Nah, I'm compiling it from scratch. Can you compile?

You need to compile translations from .pot to .mo files with make languages=all command.

@acidia

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2018

Tried using make languages=all as a build argument in code blocks, rebuilt, and nothing changed?

EDIT: NVM, that was pushing those arguments to the .exe after compiling. For project build options ->
"make" commands tab, I can't edit anything because the project is setup without a custom Makefile.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jul 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.