refactor!: give items identity

doc/GAME_OBJECTS.md

@@ -0,0 +1,70 @@
+# Game Objects
+
+Many of the things that physically exist within the game world are **game objects(GO)**. Currently
+this only covers items, but creatures and vehicles are coming soon and then probably furniture at
+some point. GOs have a small common interface with methods like name and position that you can find
+in [`game_object.h`](../src/game_object.h). GOs use private constructors, this means **your
+variables must always be references or pointers**. Trying to assign a variable without a layer of
+indirection like that will cause a compile error. Likewise trying to copy one object over another
+will cause a similar error.
+
+```c++
+item& it_ref = ...; // Good
+item* it_pointer; // Good
+
+item it = ...; // Compile error
+it_ref = other; // Compile error
+*it_pointer = other; // Compile error
+```
+
+New GOs can be created via `::spawn` static methods that return a `detached_ptr` to the newly
+created game object. A `detached_ptr` represents an object that does not currently exist in the game
+world. There can only be one `detached_ptr` to an object at a time (those who know
+[`std::unique_ptr`](https://en.cppreference.com/w/cpp/memory/unique_ptr) will be familiar with this
+behavior). Functions that add an object to the world will require that you `std::move` in a
+`detached_ptr`. In general `detached_ptr`s must be `std::move`'d when they're passed into a
+function. Note that you should never use any variable after it has been moved. Likewise functions
+that remove an object from the world will return a `detached_ptr`. This ensures that you can't add
+the same thing to the world twice.
+
+Functions that don't add things to the world just accept normal pointers. You can turn a
+`detached_ptr` into a normal pointer by doing this. Note that you can use this to keep a valid
+pointer to something even after you `std::move` it, but you must do this before the `std::move`
+happens.
+
+```c++
+&*detached
+```
+
+And you can go the other way using this. Though note that it removes the object from the game world
+in the process and will cause errors if called on an object that isn't in the game world.
+
+```c++
+detached_ptr<item> as_detached=normal_ptr->detach();
+```
+
+Trying to access an invalid `detached_ptr` (for instance one that has been `std::move`'d from) will
+cause a debugmsg and give you the null version of that object.
+
+## Safe References
+
+Game objects support safe references. `safe_reference<item>` will refuse access to an object that
+has been destroyed or is outside of the reality bubble without losing track of it, and they can be
+saved and loaded. You must check them as a boolean (e.g. `if( ref )`) to see if they're valid.
+Trying to access them when they're not will cause debugmsgs and give you a null object instead. They
+have a small interface that lets you check if they're destroyed or unloaded etc. If they were
+destroyed or unloaded this turn, they have a method that will allow you to access the object in a
+const fashion, for instance to display an error message.
+
+If you're moving objects around you need to use `detached_ptr`s, but otherwise when choosing which
+reference to use the most important thing to consider is how long you want to hold onto it. If
+you're just using something temporarily, for instance most arguments and variables, you should use a
+normal reference or pointer. If you need to store it for longer you should use a safe reference and
+this means it can be easily stored in the save file. In the rare case that you do want to save it
+across turns but don't want to store it in the save file, which means caches, there's also a fast
+`cache_reference<item>`, which does last across turns but can't be saved.
+
+Game objects can sometimes be split into pieces or merged together. Item stacks are the main example
+of this but there are others like vehicles being split or dissoluted devourers merging. When a stack
+of items is split, the stack that stays in place is the one that safe references will follow. When
+they are merged safe references to either half of the merge will now point to the merge result.

msvc-full-features/stdafx.h

@@ -74,7 +74,6 @@
 #include <vector>
 
 #include "../src/platform_win.h"
-#include "../src/colony.h"
 
 #if defined(TILES)
 #   if defined(_MSC_VER) && defined(USE_VCPKG)

pch/main-pch.hpp

@@ -55,4 +55,3 @@
 #include <unordered_set>
 #include <utility>
 #include <vector>
-#include "../src/colony.h"

src/action.cpp

@@ -556,12 +556,12 @@ bool can_butcher_at( const tripoint &p )
     bool has_corpse = false;
 
     const inventory &crafting_inv = you.crafting_inventory();
-    for( item &items_it : items ) {
-        if( items_it.is_corpse() ) {
+    for( item *&items_it : items ) {
+        if( items_it->is_corpse() ) {
             if( factor != INT_MIN  || factorD != INT_MIN ) {
                 has_corpse = true;
             }
-        } else if( crafting::can_disassemble( you, items_it, crafting_inv ).success() ) {
+        } else if( crafting::can_disassemble( you, *items_it, crafting_inv ).success() ) {
             has_item = true;
         }
     }

src/active_item_cache.cpp

@@ -9,62 +9,58 @@
 void active_item_cache::remove( const item *it )
 {
     for( auto &list : active_items ) {
-        list.second.remove_if( [it]( const item_reference & active_item ) {
-            item *const target = active_item.item_ref.get();
+        list.second.erase( std::remove_if( list.second.begin(),
+        list.second.end(), [it]( const cache_reference<item> &active_item ) {
+            if( !active_item ) {
+                return true;
+            }
+            item *const target = &*active_item;
             return !target || target == it;
-        } );
+        } ), list.second.end() );
     }
     if( it->can_revive() ) {
-        special_items[ special_item_type::corpse ].remove_if( [it]( const item_reference & active_item ) {
-            item *const target = active_item.item_ref.get();
-            return !target || target == it;
-        } );
+        std::vector<cache_reference<item>> &corpse = special_items[ special_item_type::corpse ];
+        corpse.erase( std::remove( corpse.begin(), corpse.end(), it ), corpse.end() );
     }
     if( it->get_use( "explosion" ) ) {
-        special_items[ special_item_type::explosive ].remove_if( [it]( const item_reference &
-        active_item ) {
-            item *const target = active_item.item_ref.get();
-            return !target || target == it;
-        } );
+        std::vector<cache_reference<item>> &explosive = special_items[ special_item_type::explosive ];
+        explosive.erase( std::remove( explosive.begin(), explosive.end(), it ), explosive.end() );
     }
 }
 
-void active_item_cache::add( item &it, point location )
+void active_item_cache::add( item &it )
 {
     // If the item is alread in the cache for some reason, don't add a second reference
-    std::list<item_reference> &target_list = active_items[it.processing_speed()];
-    if( std::find_if( target_list.begin(),
-    target_list.end(), [&it]( const item_reference & active_item_ref ) {
-    return &it == active_item_ref.item_ref.get();
-    } ) != target_list.end() ) {
+    std::vector<cache_reference<item>> &target_list = active_items[it.processing_speed()];
+    if( std::find( target_list.begin(), target_list.end(), it ) != target_list.end() ) {
         return;
     }
     if( it.can_revive() ) {
-        special_items[ special_item_type::corpse ].push_back( item_reference{ location, it.get_safe_reference() } );
+        special_items[ special_item_type::corpse ].push_back( it );
     }
     if( it.get_use( "explosion" ) ) {
-        special_items[ special_item_type::explosive ].push_back( item_reference{ location, it.get_safe_reference() } );
+        special_items[ special_item_type::explosive ].push_back( it );
     }
-    target_list.push_back( item_reference{ location, it.get_safe_reference() } );
+    target_list.push_back( it );
 }
 
 bool active_item_cache::empty() const
 {
-    for( std::pair<int, std::list<item_reference>> active_queue : active_items ) {
+    for( std::pair<int, std::vector<cache_reference<item>>> active_queue : active_items ) {
         if( !active_queue.second.empty() ) {
             return false;
         }
     }
     return true;
 }
 
-std::vector<item_reference> active_item_cache::get()
+std::vector<item *> active_item_cache::get()
 {
-    std::vector<item_reference> all_cached_items;
-    for( std::pair<const int, std::list<item_reference>> &kv : active_items ) {
-        for( std::list<item_reference>::iterator it = kv.second.begin(); it != kv.second.end(); ) {
-            if( it->item_ref ) {
-                all_cached_items.emplace_back( *it );
+    std::vector<item *> all_cached_items;
+    for( std::pair<const int, std::vector<cache_reference<item>>> &kv : active_items ) {
+        for( std::vector<cache_reference<item>>::iterator it = kv.second.begin(); it != kv.second.end(); ) {
+            if( *it ) {
+                all_cached_items.push_back( & **it );
                 ++it;
             } else {
                 it = kv.second.erase( it );
@@ -74,16 +70,21 @@ std::vector<item_reference> active_item_cache::get()
     return all_cached_items;
 }
 
-std::vector<item_reference> active_item_cache::get_for_processing()
+std::vector<item *> active_item_cache::get_for_processing()
 {
-    std::vector<item_reference> items_to_process;
-    for( std::pair<const int, std::list<item_reference>> &kv : active_items ) {
+    std::vector<item *> items_to_process;
+    for( std::pair < const int, std::vector<cache_reference<item>>> &kv : active_items ) {
         // Rely on iteration logic to make sure the number is sane.
         int num_to_process = kv.second.size() / kv.first;
-        std::list<item_reference>::iterator it = kv.second.begin();
+        std::vector<cache_reference<item>>::iterator it = kv.second.begin();
+
+        //TODO!: This is a bit of a hack, at the very least check the maths
+        //Skip the entries that were processed last turn.
+        it += num_to_process * ( to_turn<int>( calendar::turn ) % kv.first );
+
         for( ; it != kv.second.end() && num_to_process >= 0; ) {
-            if( it->item_ref ) {
-                items_to_process.push_back( *it );
+            if( *it ) {
+                items_to_process.push_back( & **it );
                 --num_to_process;
                 ++it;
             } else {
@@ -93,34 +94,18 @@ std::vector<item_reference> active_item_cache::get_for_processing()
         }
         // Rotate the returned items to the end of their list so that the items that weren't
         // returned this time will be first in line on the next call
-        kv.second.splice( kv.second.end(), kv.second, kv.second.begin(), it );
+        // kv.second.splice( kv.second.end(), kv.second, kv.second.begin(), it );
     }
     return items_to_process;
 }
 
-std::vector<item_reference> active_item_cache::get_special( special_item_type type )
-{
-    std::vector<item_reference> matching_items;
-    for( const item_reference &it : special_items[type] ) {
-        matching_items.push_back( it );
-    }
-    return matching_items;
-}
-
-void active_item_cache::subtract_locations( point delta )
-{
-    for( std::pair<const int, std::list<item_reference>> &pair : active_items ) {
-        for( item_reference &ir : pair.second ) {
-            ir.location -= delta;
-        }
-    }
-}
-
-void active_item_cache::rotate_locations( int turns, point dim )
+std::vector<item *> active_item_cache::get_special( special_item_type type )
 {
-    for( std::pair<const int, std::list<item_reference>> &pair : active_items ) {
-        for( item_reference &ir : pair.second ) {
-            ir.location = ir.location.rotate( turns, dim );
+    std::vector<item *> matching_items;
+    for( const cache_reference<item> &it : special_items[type] ) {
+        if( it ) {
+            matching_items.push_back( &*it );
         }
     }
+    return matching_items;
 }

src/active_item_cache.h

@@ -12,12 +12,6 @@
 
 class item;
 
-// A struct used to uniquely identify an item within a submap or vehicle.
-struct item_reference {
-    point location;
-    safe_reference<item> item_ref;
-};
-
 enum class special_item_type : int {
     none,
     corpse,
@@ -37,8 +31,8 @@ struct hash<special_item_type> {
 class active_item_cache
 {
     private:
-        std::unordered_map<int, std::list<item_reference>> active_items;
-        std::unordered_map<special_item_type, std::list<item_reference>> special_items;
+        std::unordered_map<int, std::vector<cache_reference<item>>> active_items;
+        std::unordered_map<special_item_type, std::vector<cache_reference<item>>> special_items;
 
     public:
         /**
@@ -52,7 +46,7 @@ class active_item_cache
          * Adds the reference to the cache. Does nothing if the reference is already in the cache.
          * Relies on the fact that item::processing_speed() is a constant.
          */
-        void add( item &it, point location );
+        void add( item &it );
 
         /**
          * Returns true if the cache is empty
@@ -63,7 +57,7 @@ class active_item_cache
          * Returns a vector of all cached active item references.
          * Broken references are removed from the cache.
          */
-        std::vector<item_reference> get();
+        std::vector<item *> get();
 
         /**
          * Returns the first size() / processing_speed() elements of each list, rounded up.
@@ -73,15 +67,12 @@ class active_item_cache
          * the cache.
          * Relies on the fact that item::processing_speed() is a constant.
          */
-        std::vector<item_reference> get_for_processing();
+        std::vector<item *> get_for_processing();
 
         /**
          * Returns the currently tracked list of special active items.
          */
-        std::vector<item_reference> get_special( special_item_type type );
-        /** Subtract delta from every item_reference's location */
-        void subtract_locations( point delta );
-        void rotate_locations( int turns, point dim );
+        std::vector<item *> get_special( special_item_type type );
 };
 
 #endif // CATA_SRC_ACTIVE_ITEM_CACHE_H

src/active_tile_data.cpp

@@ -256,8 +256,8 @@ void charger_tile::update_internal( time_point to, const tripoint_abs_ms &p,
     }
     std::int64_t power = this->power * to_seconds<std::int64_t>( to - get_last_updated() );
     // TODO: Make not a copy from map.cpp
-    for( item &outer : sm->get_items( p_within_sm.raw() ) ) {
-        outer.visit_items( [&power, &grid]( item * it ) {
+    for( item *const outer : sm->get_items( p_within_sm.raw() ) ) {
+        outer->visit_items( [&power, &grid]( item * it ) {
             item &n = *it;
             if( !n.has_flag( flag_RECHARGE ) && !n.has_flag( flag_USE_UPS ) ) {
                 return VisitResponse::NEXT;