From f51e53c9863526ab2ba0ab7e707183ed0acaafba Mon Sep 17 00:00:00 2001 From: dpwb Date: Tue, 25 Feb 2020 16:02:22 +0000 Subject: [PATCH 1/3] clear up item ownership for dead factions --- src/npc.cpp | 11 +++++++++++ src/pickup.cpp | 5 +++++ 2 files changed, 16 insertions(+) diff --git a/src/npc.cpp b/src/npc.cpp index 8d267cea57006..8587421cdcc35 100644 --- a/src/npc.cpp +++ b/src/npc.cpp @@ -2372,6 +2372,17 @@ void npc::die( Creature *nkiller ) elem->remove_owner(); elem->remove_old_owner(); } + // broadcast a signal to any items this NPC may have dropped nearby to clear their ownership too + // this will also validate when the items are interacted with + // but may as well solve what we can now. + for( const tripoint tri_elem : g->m.points_in_radius( pos(), 60 ) ) { + for( item &elem : g->m.i_at( tri_elem ) ) { + if( !elem.get_owner().is_null() && !g->faction_manager_ptr->get( elem.get_owner(), false ) ) { + elem.remove_owner(); + elem.remove_old_owner(); + } + } + } } my_fac->remove_member( getID() ); } diff --git a/src/pickup.cpp b/src/pickup.cpp index c0abaf506cbf9..38958714a43a3 100644 --- a/src/pickup.cpp +++ b/src/pickup.cpp @@ -517,6 +517,11 @@ void Pickup::pick_up( const tripoint &p, int min, from_where get_items_from ) std::vector> stacked_here; for( item_stack::iterator it : here ) { bool found_stack = false; + // an NPC may have dropped an item, then later died, the faction is now invalid. + if( !it->get_owner().is_null() && !g->faction_manager_ptr->get( it->get_owner(), false ) ) { + it->remove_owner(); + it->remove_old_owner(); + } for( std::list &stack : stacked_here ) { if( stack.front()->display_stacked_with( *it ) ) { stack.push_back( it ); From 4f526ca195f8a3aac4412ec0ad48e211e39ecd99 Mon Sep 17 00:00:00 2001 From: dpwb Date: Tue, 25 Feb 2020 16:53:50 +0000 Subject: [PATCH 2/3] rework the concept - make owner mutable, confine validation to get_owner() --- src/item.cpp | 18 +++++++++++++++--- src/item.h | 9 +++++---- src/npc.cpp | 11 ----------- src/pickup.cpp | 5 ----- 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/item.cpp b/src/item.cpp index f16a4ce6a33d4..72fa7454ba08a 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -1048,7 +1048,7 @@ bool item::is_owned_by( const Character &c, bool available_to_take ) const bool item::is_old_owner( const Character &c, bool available_to_take ) const { - if( old_owner.is_null() ) { + if( get_old_owner().is_null() ) { return available_to_take; } if( !c.get_faction() ) { @@ -1060,11 +1060,11 @@ bool item::is_old_owner( const Character &c, bool available_to_take ) const std::string item::get_owner_name() const { - if( !g->faction_manager_ptr->get( owner ) ) { + if( !g->faction_manager_ptr->get( get_owner() ) ) { debugmsg( "item::get_owner_name() item %s has no valid nor null faction id ", tname() ); return "no owner"; } - return g->faction_manager_ptr->get( owner )->name; + return g->faction_manager_ptr->get( get_owner() )->name; } void item::set_owner( const Character &c ) @@ -1078,14 +1078,26 @@ void item::set_owner( const Character &c ) faction_id item::get_owner() const { + validate_ownership(); return owner; } faction_id item::get_old_owner() const { + validate_ownership(); return old_owner; } +void item::validate_ownership() const +{ + if( !old_owner.is_null() && !g->faction_manager_ptr->get( old_owner, false ) ) { + remove_old_owner(); + } + if( !owner.is_null() && !g->faction_manager_ptr->get( owner, false ) ) { + remove_owner(); + } +} + static void insert_separation_line( std::vector &info ) { if( info.empty() || info.back().sName != "--" ) { diff --git a/src/item.h b/src/item.h index 47eb4085e896f..21eb66e400b08 100644 --- a/src/item.h +++ b/src/item.h @@ -1963,17 +1963,18 @@ class item : public visitable void set_birthday( const time_point &bday ); void handle_pickup_ownership( Character &c ); int get_gun_ups_drain() const; + void validate_ownership() const; inline void set_old_owner( const faction_id &temp_owner ) { old_owner = temp_owner; } - inline void remove_old_owner() { + inline void remove_old_owner() const { old_owner = faction_id::NULL_ID(); } inline void set_owner( const faction_id &new_owner ) { owner = new_owner; } void set_owner( const Character &c ); - inline void remove_owner() { + inline void remove_owner() const { owner = faction_id::NULL_ID(); } faction_id get_owner() const; @@ -2183,9 +2184,9 @@ class item : public visitable */ phase_id current_phase = static_cast( 0 ); // The faction that owns this item. - faction_id owner = faction_id::NULL_ID(); + mutable faction_id owner = faction_id::NULL_ID(); // The faction that previously owned this item - faction_id old_owner = faction_id::NULL_ID(); + mutable faction_id old_owner = faction_id::NULL_ID(); int damage_ = 0; light_emission light = nolight; diff --git a/src/npc.cpp b/src/npc.cpp index 8587421cdcc35..8d267cea57006 100644 --- a/src/npc.cpp +++ b/src/npc.cpp @@ -2372,17 +2372,6 @@ void npc::die( Creature *nkiller ) elem->remove_owner(); elem->remove_old_owner(); } - // broadcast a signal to any items this NPC may have dropped nearby to clear their ownership too - // this will also validate when the items are interacted with - // but may as well solve what we can now. - for( const tripoint tri_elem : g->m.points_in_radius( pos(), 60 ) ) { - for( item &elem : g->m.i_at( tri_elem ) ) { - if( !elem.get_owner().is_null() && !g->faction_manager_ptr->get( elem.get_owner(), false ) ) { - elem.remove_owner(); - elem.remove_old_owner(); - } - } - } } my_fac->remove_member( getID() ); } diff --git a/src/pickup.cpp b/src/pickup.cpp index 38958714a43a3..c0abaf506cbf9 100644 --- a/src/pickup.cpp +++ b/src/pickup.cpp @@ -517,11 +517,6 @@ void Pickup::pick_up( const tripoint &p, int min, from_where get_items_from ) std::vector> stacked_here; for( item_stack::iterator it : here ) { bool found_stack = false; - // an NPC may have dropped an item, then later died, the faction is now invalid. - if( !it->get_owner().is_null() && !g->faction_manager_ptr->get( it->get_owner(), false ) ) { - it->remove_owner(); - it->remove_old_owner(); - } for( std::list &stack : stacked_here ) { if( stack.front()->display_stacked_with( *it ) ) { stack.push_back( it ); From e9c664202982c401828e5dc828db477b6bd0f9de Mon Sep 17 00:00:00 2001 From: dpwb Date: Tue, 25 Feb 2020 17:01:18 +0000 Subject: [PATCH 3/3] use get_owner() in is_owned_by() --- src/item.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/item.cpp b/src/item.cpp index 72fa7454ba08a..1b272c77a2e99 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -1036,7 +1036,7 @@ bool item::is_owned_by( const Character &c, bool available_to_take ) const // owner.is_null() implies faction_id( "no_faction" ) which shouldnt happen, or no owner at all. // either way, certain situations this means the thing is available to take. // in other scenarios we actaully really want to check for id == id, even for no_faction - if( owner.is_null() ) { + if( get_owner().is_null() ) { return available_to_take; } if( !c.get_faction() ) {