Skip to content

Commit

Permalink
refactor!: give items identity (#2250)
Browse files Browse the repository at this point in the history
* Temp commit, now with compiling

* More refactoring

* vehicles fix

* More bugfixes and the start of the test suite

* More bugfixes

* More bugfixes

* Beginning of better types

* More new stuff, no compiling

* More moving to smart pointers

* Bit more

* More more more

* MOAR

* More

* even more

* style: typo `item_spawn` -> `item::spawn`

* style: remove unnecessary `std::move`, use `->` for activity

applied clang-tidy to fix `-Wpessimizing-move`, and activiy being changed to pointer

Co-authored-by: jove <[email protected]>

* fix:  legacy null item reference

Co-authored-by: jove <[email protected]>

* docs: return type of reduce_charges

* style: astyle

* Now with compiling

* More fixes

* More fixes

* Pre-merge commit, fairly stable now

* Round out interfaces

* Better vehicles and cleanup

* Remove traces and better destruction

* Attempt to appease gcc with vectors

* Pretty sure this wasn't me but ok

* ty clang tidy

* Oh yeah sounds exist

* bug fix

* Remove unused overmap constructors as they fail on some compilers

* Backwards compat

* Test suite compiles now

* MOAR

* More more more

* Test suite passes

* Bad move

* Fix missing super character constructors

* Update init.cpp

* Update src/init.cpp

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update src/init.cpp

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update init.cpp

* Remove unused stuff

* And the missed use

* Remove execinfo.h

* Bugs

* Fix map rotation

* Fix failed wrecks

* More bugfixes

* Various bugs

* More bugfixes

* Fix disassembly

* Fix dead removals

* Back to late delete for now

* Missing property in constructor

* Activity fix

* More safety

* style: format with code blocks

* ups

* Update crafting.cpp

* More fixes

* More fixes

* Forgot to add a file

* Fix rotation offsets

* Appease gcc

* Fixup android

* minor fixes

* weapon charges fix

* Fix monsters

* Remove old rotation offsets

* Fixes

* minor safety, needs investigating

* Fixes

* cleanup

* pch fix and more cleanup

* merge

* Fix msvc

* Fix spacing, remove unused includes

* Remove npc.h include from locations.h

* Remove leftovers from item_location

* Remove old commented code

* Remove player_activity.h from json.h

* Split off activity_ptr into its own header

* Fix typos

* Remove commented out or dead code

* Fix tidy warnings

* Remove colony from update-pch.sh

* Hopefully this fixes the compile

* build fixes

* Wrap long comments

* Fix typo

* Delete submodule

* Alternative build fix

* Fix dereferencing nullptr

* Remove extra space

* Remove dead code

* Update src/detached_ptr.cpp

Co-authored-by: Olanti <[email protected]>

* Update src/activity_actor.cpp

Co-authored-by: Olanti <[email protected]>

* Update src/active_tile_data.cpp

Co-authored-by: Olanti <[email protected]>

* Update src/handle_liquid.h

Co-authored-by: Olanti <[email protected]>

* Update src/active_tile_data.cpp

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Fix takeoff deleting items

* Fixes

* Liquid fix

* Fix book crash

* fix holsters

* missed a spot

* fix: make it compile

* Mess with lua iterators

* Fixes

* fix: iexamine_elevator

* fix: `vehicle_part.h` headers

* fix: salvage by weight PR

* docs: game objects

* fix: salvage test

* style(autofix.ci): automated formatting

* submap rotate fixes

* Shouldn't use uint

* And the rest

* Fix destroyed things being processed

* Fix armor relayering

* fix gcc

* style(autofix.ci): automated formatting

* Fix filthy clothes

* Fix butchering

* Random iterator safety fixes

* refactor: trailing return goes brrrrr

* refactor: apply clang-tidy suggestions

* perf: remove obsoleted colony tests

* perf: apply all clang-tidy performance fixes

* perf: use `emplace_back`

* fix: position loopback segfaulting

* fix: keep location info on `attempt_detach`

* perf: remove flag usage for temperature display

* feat: give morale bonus for items `EATEN_COLD`

* Better position through attempt_detach

* Update game_object.cpp

* Update rot.cpp

* style(autofix.ci): automated formatting

* Fix things rotting away

* Fix null cache references being moved badly

* style(autofix.ci): automated formatting

* Fix ub when active item list is empty

* Prevents possible use after free in sounds

* Appease windows compilers noexcept

* Bunch of cleanup

* Proper move for lambda arg/return

* style(autofix.ci): automated formatting

* tidy

* style(autofix.ci): automated formatting

* Fix or silence misc lints

* Fix accidental copy

* Use somewhat safer deallocation on return

* Fix readability-inconsistent-declaration-parameter-name

* Delete doc/GAME_OBJECT.md

It's been moved to doc/src/content/docs/en/dev/explanation/game_objects.md

* Use id as debug name to avoid it being translated

* Fix nullptr access

* Add blocks in switch cases

* Silence false positive use-after-move

* Reorder some code to avoid use-after-move

* Reorder some code to avoid bugprone-if-assignment

* Fix moving of references

* Remove noop detached_ptr<item>::release() calls

* Fix legit (?) cases of use-after-move

* Fix crash in inventory dumping

* Fix bug in spells

---------

Co-authored-by: scarf <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Olanti <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
  • Loading branch information
5 people authored Nov 4, 2023
1 parent daddaec commit 7ed3776
Show file tree
Hide file tree
Showing 362 changed files with 17,569 additions and 20,573 deletions.
2 changes: 0 additions & 2 deletions LICENSE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ Terminus Font (data/font/terminus.ttf) is licensed under the SIL Open Font Licen

CATCH unit-test framework (tests/catch/catch.hpp) is licensed under the Boost Software License. Visit https://github.com/philsquared/Catch/blob/master/LICENSE.txt to read the license.

PLF List and PLF Colony (src/list.h, src/colony.h) are licensed under the zLib license (https://www.zlib.net/zlib_license.html).

getpost (tools/json_tools/format/getpost.h) is licensed under the MIT license, see file for text of license.

Lua (src/lua/*) is licensed under the MIT license, see src_lua/LICENSE.md for text of license.
Expand Down
2 changes: 1 addition & 1 deletion data/json/items/comestibles/frozen.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
"//": "not technically a drink, but is quasi-liquid enough that it'd get all over everything if you carried it",
"charges": 2,
"vitamins": [ [ "vitA", 9 ], [ "calcium", 9 ], [ "vitB", 11 ] ],
"flags": [ "MELTS" ]
"flags": [ "MELTS", "EATEN_COLD" ]
},
{
"type": "COMESTIBLE",
Expand Down
10 changes: 10 additions & 0 deletions data/json/morale_types.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@
"type": "morale_type",
"text": "Enjoyed a hot meal"
},
{
"id": "morale_food_cold",
"type": "morale_type",
"text": "Enjoyed a cold treat"
},
{
"id": "morale_food_very_cold",
"type": "morale_type",
"text": "Enjoyed an icy treat"
},
{
"id": "morale_chat",
"type": "morale_type",
Expand Down
71 changes: 71 additions & 0 deletions doc/src/content/docs/en/dev/explanation/game_objects.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
---
title: 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`. 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.

```cpp
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.

```cpp
&*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.

```cpp
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.
18 changes: 18 additions & 0 deletions doc/src/content/docs/en/dev/guides/items.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
title: Items Cookbook
---

Here are some common tasks that you might want to do with items.
[For more on item(game objects), check here.](../explanation/game_objects.md)

## Moving items from one tripoint to another

```cpp
auto move_item( map &here, const tripoint &src, const tripoint &dest ) -> void
{
map_stack items_src = here.i_at( src );
map_stack items_dest = here.i_at( dest );

items_src.move_all_to( &items_dest );
}
```
1 change: 0 additions & 1 deletion msvc-full-features/stdafx.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion pch/main-pch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,3 @@
#include <unordered_set>
#include <utility>
#include <vector>
#include "../src/colony.h"
4 changes: 2 additions & 2 deletions src/achievement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ void achievement::add_kill_requirements( const JsonObject &jo, const std::string
}
}

void achievement::add_kill_requirement( const JsonObject inner, const std::string & )
void achievement::add_kill_requirement( const JsonObject &inner, const std::string & )
{
if( inner.has_string( "monster" ) && inner.has_string( "species" ) ) {
inner.throw_error( "Cannot have both id and species identifiers" );
Expand Down Expand Up @@ -268,7 +268,7 @@ void achievement::add_skill_requirements( const JsonObject &jo, const std::strin
}
}

void achievement::add_skill_requirement( const JsonObject inner, const std::string & )
void achievement::add_skill_requirement( const JsonObject &inner, const std::string & )
{
const skill_id skill = static_cast<skill_id>( inner.get_string( "skill" ) );
const achievement_comparison compare = inner.get_enum_value<achievement_comparison>( "is" );
Expand Down
4 changes: 2 additions & 2 deletions src/achievement.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ class achievement
/** Retrieves kill requirement JsonObjects and feeds it to add_skill_requirement*/
void add_kill_requirements( const JsonObject &jo, const std::string &src );
/** Organizes variables provided and adds kill_requirements to achievements*/
void add_kill_requirement( const JsonObject inner, const std::string &src );
void add_kill_requirement( const JsonObject &inner, const std::string &src );
/** Retrieves skill requirement JsonObjects and feeds it to add_skill_requirement*/
void add_skill_requirements( const JsonObject &jo, const std::string &src );
/** Organizes variables provided and adds skill_requirements to achievements*/
void add_skill_requirement( const JsonObject inner, const std::string &src );
void add_skill_requirement( const JsonObject &inner, const std::string &src );
};

template<>
Expand Down
6 changes: 3 additions & 3 deletions src/action.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,12 +562,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;
}
}
Expand Down
138 changes: 74 additions & 64 deletions src/active_item_cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,115 +9,125 @@
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();
return !target || target == it;
} );
int count = 0;
list.second.second.erase( std::remove_if( list.second.second.begin(),
list.second.second.end(), [it, &count, &list]( const cache_reference<item> &active_item ) {
if( !active_item ) {
count++;
return true;
}
item *const target = &*active_item;
if( !target || target == it ) {
if( count >= list.second.first ) {
list.second.first = std::max( 0, list.second.first - 1 );
}
count++;
return true;
}
count++;
return false;
} ), list.second.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()].second;
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 ].emplace_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 ].emplace_back( it );
}
target_list.push_back( item_reference{ location, it.get_safe_reference() } );
target_list.emplace_back( it );
}

bool active_item_cache::empty() const
{
return std::all_of( active_items.begin(), active_items.end(), []( const auto & active_queue ) {
return active_queue.second.empty();
return active_queue.second.second.empty();
} );
}

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::pair<int, std::vector<cache_reference<item>>>> &kv : active_items ) {
for( std::vector<cache_reference<item>>::iterator it = kv.second.second.begin();
it != kv.second.second.end(); ) {
if( *it ) {
all_cached_items.push_back( & **it );
++it;
} else {
it = kv.second.erase( it );
it = kv.second.second.erase( it );
}
}
}
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::pair<int, std::vector<cache_reference<item>>>> &kv :
active_items ) {
//The algorithm here is a bit weird. We're going to process a fraction of the list at a time, keeping track of where we are in the list with a simple int.
//But, the list could change between each run. As such the number will be reduced when items are removed from it (in ::remove) to prevent skips.

if( kv.second.second.empty() ) { //Prevents a div by 0 in the modulo operations
kv.second.first = 0; //May as well reset the position
continue;
}

// 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();
for( ; it != kv.second.end() && num_to_process >= 0; ) {
if( it->item_ref ) {
items_to_process.push_back( *it );
int num_to_process = kv.second.second.size() / kv.first;
std::vector<cache_reference<item>>::iterator it = kv.second.second.begin();


kv.second.first = kv.second.first %
kv.second.second.size(); //Make sure the key isn't larger than the array
std::advance( it, kv.second.first );

while( num_to_process >= 0 ) {
if( *it ) {
items_to_process.push_back( & **it );
--num_to_process;
++it;
} else {
// The item has been destroyed, so remove the reference from the cache
it = kv.second.erase( it );
it = kv.second.second.erase( it );
if( kv.second.second.empty() ) {
break;
}
}
if( it == kv.second.second.end() ) {
it = kv.second.second.begin();
}
}
// 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.first += num_to_process + 1;
}
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;
}
Loading

0 comments on commit 7ed3776

Please sign in to comment.