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.

Sign up for GitHub

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

Use colony for map and vehicle item storage #31406

Merged
merged 8 commits into from
Jun 20, 2019
Merged

Use colony for map and vehicle item storage #31406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9d9c13e
Use colony for map and vehicle item storage
ifreund Jun 13, 2019
e241207
Refactor player activites to use item_location
ifreund Jun 15, 2019
50ff098
Use item_location for butchery and disassembly
ifreund Jun 15, 2019
947f1ae
Make changes from code review and refactor
ifreund Jun 15, 2019
a2d8c5e
Remove `item_stack` subscript operator
ifreund Jun 15, 2019
04c1ffd
Remove map and vehicle functions using item indexes
ifreund Jun 16, 2019
cfc8644
Replace item_stack::front() with item_stack::only_item()
ifreund Jun 16, 2019
3922cca
Fix invlet tests
ifreund Jun 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/active_item_cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
#include <algorithm>
#include <utility>

#include "colony.h"
#include "debug.h"
#include "item.h"
#include "item_stack.h"

void active_item_cache::remove( std::list<item>::iterator it, point location )
void active_item_cache::remove( item_stack::iterator it, point location )
{
const auto predicate = [&]( const item_reference & active_item ) {
return location == active_item.location && active_item.item_iterator == it;
Expand All @@ -29,7 +31,7 @@ void active_item_cache::remove( std::list<item>::iterator it, point location )
}
}

void active_item_cache::add( std::list<item>::iterator it, point location )
void active_item_cache::add( item_stack::iterator it, point location )
{
if( has( it, location ) ) {
return;
Expand All @@ -38,7 +40,7 @@ void active_item_cache::add( std::list<item>::iterator it, point location )
active_item_set[ &*it ] = false;
}

bool active_item_cache::has( std::list<item>::iterator it, point ) const
bool active_item_cache::has( item_stack::iterator it, point ) const
{
return active_item_set.find( &*it ) != active_item_set.end();
}
Expand Down
10 changes: 6 additions & 4 deletions src/active_item_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
#include <list>
#include <unordered_map>

#include "colony.h"
#include "enums.h"
#include "item.h"
#include "item_stack.h"

// A struct used to uniquely identify an item within a submap or vehicle.
struct item_reference {
point location;
std::list<item>::iterator item_iterator;
item_stack::iterator item_iterator;
// Do not access this from outside this module, it is only used as an ID for active_item_set.
item *item_id;
};
Expand All @@ -25,9 +27,9 @@ class active_item_cache
std::unordered_map<item *, bool> active_item_set;

public:
void remove( std::list<item>::iterator it, point location );
void add( std::list<item>::iterator it, point location );
bool has( std::list<item>::iterator it, point ) const;
void remove( item_stack::iterator it, point location );
void add( item_stack::iterator it, point location );
bool has( item_stack::iterator it, point ) const;
// Use this one if there's a chance that the item being referenced has been invalidated.
bool has( const item_reference &itm ) const;
bool empty() const;
Expand Down
140 changes: 72 additions & 68 deletions src/activity_handlers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,33 +332,11 @@ static void butcher_cbm_group( const std::string &group, const tripoint &pos,

static void set_up_butchery( player_activity &act, player &u, butcher_type action )
{
if( !act.values.empty() ) {
act.index = act.values.back();
act.values.pop_back();
} else {
debugmsg( "Invalid butchery item index %d", act.index );
act.set_to_null();
return;
}

const int factor = u.max_quality( action == DISSECT ? quality_id( "CUT_FINE" ) :
quality_id( "BUTCHER" ) );
auto items = g->m.i_at( u.pos() );
if( static_cast<size_t>( act.index ) >= items.size() ) {
// Let it print a msg for lack of corpses
act.index = INT_MAX;
return;
}

item corpse_item = items[act.index];
const mtype *corpse_ptr = corpse_item.get_mtype();
if( corpse_ptr == nullptr ) {
debugmsg( "Tried to butcher a non-corpse item, %s",
corpse_item.tname( corpse_item.count() ) );
act.set_to_null();
return;
}
const mtype &corpse = *corpse_ptr;
const item &corpse_item = *act.targets.back();
const mtype &corpse = *corpse_item.get_mtype();

if( action != DISSECT ) {
if( factor == INT_MIN ) {
Expand Down Expand Up @@ -422,32 +400,32 @@ static void set_up_butchery( player_activity &act, player &u, butcher_type actio

if( !u.has_quality( quality_id( "CUT" ) ) ) {
u.add_msg_if_player( m_info, _( "You need a cutting tool to perform a full butchery." ) );
act.index = -1;
act.targets.pop_back();
return;
}
if( big_corpse ) {
if( has_rope && !has_tree_nearby && !b_rack_present ) {
u.add_msg_if_player( m_info,
_( "You need to suspend this corpse to butcher it. While you have a rope to lift the corpse, there is no tree nearby to hang it from." ) );
act.index = -1;
act.targets.pop_back();
return;
}
if( !has_rope && !b_rack_present ) {
u.add_msg_if_player( m_info,
_( "To perform a full butchery on a corpse this big, you need either a butchering rack or both a long rope in your inventory and a nearby tree to hang the corpse from." ) );
act.index = -1;
act.targets.pop_back();
return;
}
if( !has_table_nearby ) {
u.add_msg_if_player( m_info,
_( "To perform a full butchery on a corpse this big, you need a table nearby or something else with a flat surface. A leather tarp spread out on the ground could suffice." ) );
act.index = -1;
act.targets.pop_back();
return;
}
if( !( u.has_quality( quality_id( "SAW_W" ) ) || u.has_quality( quality_id( "SAW_M" ) ) ) ) {
u.add_msg_if_player( m_info,
_( "For a corpse this big you need a saw to perform a full butchery." ) );
act.index = -1;
act.targets.pop_back();
return;
}
}
Expand All @@ -457,39 +435,39 @@ static void set_up_butchery( player_activity &act, player &u, butcher_type actio
corpse_item.has_flag( "FIELD_DRESS_FAILED" ) ) ) {
u.add_msg_if_player( m_info,
_( "It would be futile to search for implants inside this badly damaged corpse." ) );
act.index = -1;
act.targets.pop_back();
return;
}

if( action == F_DRESS && ( corpse_item.has_flag( "FIELD_DRESS" ) ||
corpse_item.has_flag( "FIELD_DRESS_FAILED" ) ) ) {
u.add_msg_if_player( m_info, _( "This corpse is already field dressed." ) );
act.index = -1;
act.targets.pop_back();
return;
}

if( action == SKIN && corpse_item.has_flag( "SKINNED" ) ) {
u.add_msg_if_player( m_info, _( "This corpse is already skinned." ) );
act.index = -1;
act.targets.pop_back();
return;
}

if( action == QUARTER ) {
if( corpse.size == MS_TINY ) {
u.add_msg_if_player( m_bad, _( "This corpse is too small to quarter without damaging." ),
corpse.nname() );
act.index = -1;
act.targets.pop_back();
return;
}
if( corpse_item.has_flag( "QUARTERED" ) ) {
u.add_msg_if_player( m_bad, _( "This is already quartered." ), corpse.nname() );
act.index = -1;
act.targets.pop_back();
return;
}
if( !( corpse_item.has_flag( "FIELD_DRESS" ) || corpse_item.has_flag( "FIELD_DRESS_FAILED" ) ) ) {
u.add_msg_if_player( m_bad, _( "You need to perform field dressing before quartering." ),
corpse.nname() );
act.index = -1;
act.targets.pop_back();
return;
}
}
Expand All @@ -516,12 +494,16 @@ static void set_up_butchery( player_activity &act, player &u, butcher_type actio
}
} else {
u.add_msg_if_player( m_good, _( "It needs a coffin, not a knife." ) );
act.index = -1;
act.targets.pop_back();
return;
}
}

act.moves_left = butcher_time_to_cut( u, corpse_item, action );

// We have a valid target, so preform the full finish function
// instead of just selecting the next valid target
act.index = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IS act.index a bool? It seems unlikely...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see later that you are treating it as a bool. I can't decide whether it's clearer to assign 0 or false here. Hmm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree that it's not super clean. I considered adding either a bool or an std::vector<bool> to player_activity but decided that was outside the scope of this PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an aspiration to make accessing things inside player_activity cleaner in general, with some vague ideas of how to go about it. Looking at your code here makes me want to get to that, but I probably won't soon.

}

int butcher_time_to_cut( const player &u, const item &corpse_item, const butcher_type action )
Expand Down Expand Up @@ -593,6 +575,7 @@ int butcher_time_to_cut( const player &u, const item &corpse_item, const butcher
time_to_cut = time_to_cut * ( 1 - ( g->u.get_num_crafting_helpers( 3 ) / 10 ) );
return time_to_cut;
}

// The below function exists to allow mods to migrate their content fully to the new harvest system. This function should be removed eventually.
static harvest_id butchery_flags_deprecate( const mtype &mt )
{
Expand Down Expand Up @@ -1021,6 +1004,21 @@ static void butchery_quarter( item *corpse_item, player &p )

void activity_handlers::butcher_finish( player_activity *act, player *p )
{
// No targets means we are done
if( act->targets.empty() ) {
act->set_to_null();
return;
}

item_location &target = act->targets.back();

// Corpses can disappear (rezzing!), so check for that
if( !target || !target->is_corpse() ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The operator bool on item_location is definitely doing the wrong thing in general. That's not really your fault, but this can totally end up pointing at the wrong item (I think there have been bug reports about butchery doing that already). I think the only way to fix this sensibly is with the weak_ptr idea Kevin has discussed. But that's orthogonal to your changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what it's doing is actually OK here for now. It just searches at its stored location for an item matching its stored pointer, returning true if such an item is found.

I agree theweak_ptr plan is definitely a lot cleaner and more robust and may look at implementing that once this get wrapped up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bear in mind that colony deliberately tries to reuse memory, so there is a good chance that there will be some different item at the same address if one is removed and another added, and it has no way to tell the difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but I don't think there's anything we can do about it yet without the weak_ptr trick aside from hope it's not a corpse.

p->add_msg_if_player( m_info, _( "There's no corpse to butcher!" ) );
act->set_to_null();
return;
}

butcher_type action = BUTCHER;
if( act->id() == activity_id( "ACT_BUTCHER" ) ) {
action = BUTCHER;
Expand All @@ -1038,34 +1036,21 @@ void activity_handlers::butcher_finish( player_activity *act, player *p )
action = DISMEMBER;
}

//Negative index means try to start next item
if( act->index < 0 ) {
//No values means no items left to try
if( act->values.empty() ) {
act->set_to_null();
return;
}
// index is a bool that determines if we are ready to start the next target
if( act->index ) {
set_up_butchery( *act, *p, action );
return;
}
// Corpses can disappear (rezzing!), so check for that
auto items_here = g->m.i_at( p->pos() );
if( static_cast<int>( items_here.size() ) <= act->index ||
!( items_here[act->index].is_corpse() ) ) {
p->add_msg_if_player( m_info, _( "There's no corpse to butcher!" ) );
act->set_to_null();
return;
}

item &corpse_item = items_here[act->index];
auto contents = corpse_item.contents;
item &corpse_item = *target;
std::list<item> contents = corpse_item.contents;
const mtype *corpse = corpse_item.get_mtype();
const field_id type_blood = corpse->bloodType();
const field_id type_gib = corpse->gibType();

if( action == QUARTER ) {
butchery_quarter( &corpse_item, *p );
act->index = -1;
act->index = true;
return;
}

Expand Down Expand Up @@ -1115,7 +1100,11 @@ void activity_handlers::butcher_finish( player_activity *act, player *p )
_( "You made so many mistakes during the process that you doubt even vultures will be interested in what's left of it." ) );
break;
}
g->m.i_rem( p->pos(), act->index );

// Remove the target from the map
target.remove_item();
act->targets.pop_back();

g->m.add_splatter( type_gib, p->pos(), rng( corpse->size + 2, ( corpse->size + 1 ) * 2 ) );
g->m.add_splatter( type_blood, p->pos(), rng( corpse->size + 2, ( corpse->size + 1 ) * 2 ) );
for( int i = 1; i <= corpse->size; i++ ) {
Expand All @@ -1124,7 +1113,9 @@ void activity_handlers::butcher_finish( player_activity *act, player *p )
g->m.add_splatter_trail( type_blood, p->pos(), random_entry( g->m.points_in_radius( p->pos(),
corpse->size + 1 ) ) );
}
act->index = -1;

// Ready to move on to the next item, if there is one
act->index = true;
return;
}
// function just for drop yields
Expand Down Expand Up @@ -1157,11 +1148,17 @@ void activity_handlers::butcher_finish( player_activity *act, player *p )
p->add_msg_if_player( m_good,
_( "You apply few quick cuts to the %s and leave what's left of it for scavengers." ),
corpse_item.tname() );
g->m.i_rem( p->pos(), act->index );
break; //no set_to_null here, for multibutchering

// Remove the target from the map
target.remove_item();
act->targets.pop_back();
break;
case BUTCHER_FULL:
p->add_msg_if_player( m_good, _( "You finish butchering the %s." ), corpse_item.tname() );
g->m.i_rem( p->pos(), act->index );

// Remove the target from the map
target.remove_item();
act->targets.pop_back();
break;
case F_DRESS:
if( roll_butchery() < 0 ) { // partial failure
Expand Down Expand Up @@ -1250,15 +1247,22 @@ void activity_handlers::butcher_finish( player_activity *act, player *p )
case 3:
p->add_msg_if_player( m_good, _( "You cleave the %s into pieces." ), corpse_item.tname() );
}
g->m.i_rem( p->pos(), act->index );

// Remove the target from the map
target.remove_item();
act->targets.pop_back();
break;
case DISSECT:
p->add_msg_if_player( m_good, _( "You finish dissecting the %s." ), corpse_item.tname() );
g->m.i_rem( p->pos(), act->index );

// Remove the target from the map
target.remove_item();
act->targets.pop_back();
break;
}
// multibutchering
act->index = -1;

// Ready to move on to the next item, if there is one (for example if multibutchering)
act->index = true;
}

void activity_handlers::fill_liquid_do_turn( player_activity *act, player *p )
Expand All @@ -1269,7 +1273,7 @@ void activity_handlers::fill_liquid_do_turn( player_activity *act, player *p )
vehicle *source_veh = nullptr;
const tripoint source_pos = act_ref.coords.at( 0 );
map_stack source_stack = g->m.i_at( source_pos );
std::list<item>::iterator on_ground;
map_stack::iterator on_ground;
monster *source_mon = nullptr;
item liquid;
const auto source_type = static_cast<liquid_source_type>( act_ref.values.at( 0 ) );
Expand Down Expand Up @@ -2561,9 +2565,9 @@ void activity_handlers::pickup_do_turn( player_activity *, player * )
activity_on_turn_pickup();
}

void activity_handlers::wear_do_turn( player_activity *, player * )
void activity_handlers::wear_do_turn( player_activity *act, player *p )
{
activity_on_turn_wear();
activity_on_turn_wear( *act, *p );
}

// This activity opens the menu (it's not meant to queue consumption of items)
Expand All @@ -2587,9 +2591,9 @@ void activity_handlers::consume_meds_menu_do_turn( player_activity *, player * )
g->eat( game_menus::inv::consume_meds );
}

void activity_handlers::move_items_do_turn( player_activity *, player * )
void activity_handlers::move_items_do_turn( player_activity *act, player *p )
{
activity_on_turn_move_items();
activity_on_turn_move_items( *act, *p );
}

void activity_handlers::move_loot_do_turn( player_activity *act, player *p )
Expand Down
5 changes: 2 additions & 3 deletions src/activity_handlers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,11 @@ int butcher_time_to_cut( const player &u, const item &corpse_item, const butcher

// activity_item_handling.cpp
void activity_on_turn_drop();
void activity_on_turn_move_items();
void activity_on_turn_move_items( player_activity &act, player &p );
void activity_on_turn_move_loot( player_activity &act, player &p );
void activity_on_turn_blueprint_move( player_activity &, player &p );
void activity_on_turn_pickup();
void activity_on_turn_wear();
void activity_on_turn_stash();
void activity_on_turn_wear( player_activity &act, player &p );
void try_fuel_fire( player_activity &act, player &p, const bool starting_fire = false );

enum class item_drop_reason {
Expand Down
Loading