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

Fixes activity zone sorting for comestibles in containers. #45453

Merged
merged 5 commits into from
Nov 27, 2020
Merged

Fixes activity zone sorting for comestibles in containers. #45453

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d26710a
Fixes activity zone sorting for comestibles in containers.
Squishums Nov 16, 2020
d194d17
Resolves clang 3.8 compilation error
Squishums Nov 19, 2020
1228e80
Update tests/clzones_test.cpp
kevingranade Nov 25, 2020
afa8160
Update tests/clzones_test.cpp
kevingranade Nov 25, 2020
869f2ba
targeted lambda capture
kevingranade Nov 27, 2020
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
23 changes: 23 additions & 0 deletions data/mods/TEST_DATA/items.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,6 +1269,29 @@
"loudness_modifier": -30,
"flags": [ "DISABLE_SIGHTS", "CONSUMABLE", "REACH_ATTACK" ]
},
{
"type": "COMESTIBLE",
"id": "test_milk",
"name": { "str_sp": "intolerant milk" },
"weight": "258 g",
"color": "white",
"spoils_in": "1 d",
"container": "jug_plastic",
"comestible_type": "DRINK",
"symbol": "~",
"quench": 25,
"healthy": 1,
"calories": 132,
"description": "You can only push a drink so far before it spoils.",
"price": 38,
"price_postapoc": 50,
"material": [ "milk" ],
"volume": "250 ml",
"phase": "liquid",
"flags": [ "EATEN_COLD" ],
"vitamins": [ [ "vitA", 10 ], [ "vitB", 6 ], [ "vitC", 4 ], [ "calcium", 25 ] ],
"fun": 1
},
{
"type": "COMESTIBLE",
"id": "test_brew_wine",
Expand Down
31 changes: 27 additions & 4 deletions src/clzones.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,17 +839,40 @@ zone_type_id zone_manager::get_near_zone_type_for_item( const item &it,
}

if( cat.get_id() == item_category_food ) {
// skip food without comestible, like MREs
if( const item *it_food = it.get_food() ) {
const item *it_food = nullptr;
bool perishable = false;
// Look for food, and whether any contents which will spoil if left out.
// Food crafts and food without comestible, like MREs, will fall down to LOOT_FOOD.
it.visit_items( [&]( const item * node, const item * parent ) {
kevingranade marked this conversation as resolved.
Show resolved Hide resolved
if( node && node->is_food() ) {
it_food = node;

if( node->goes_bad() ) {
float spoil_multiplier = 1.0f;
if( parent ) {
const item_pocket *parent_pocket = parent->contained_where( *node );
if( parent_pocket ) {
spoil_multiplier = parent_pocket->spoil_multiplier();
}
}
if( spoil_multiplier > 0.0f ) {
perishable = true;
}
}
}
return VisitResponse::NEXT;
} );

if( it_food != nullptr ) {
if( it_food->get_comestible()->comesttype == "DRINK" ) {
if( it_food->goes_bad() && has_near( zone_type_id( "LOOT_PDRINK" ), where, range ) ) {
if( perishable && has_near( zone_type_id( "LOOT_PDRINK" ), where, range ) ) {
return zone_type_id( "LOOT_PDRINK" );
} else if( has_near( zone_type_id( "LOOT_DRINK" ), where, range ) ) {
return zone_type_id( "LOOT_DRINK" );
}
}

if( it_food->goes_bad() && has_near( zone_type_id( "LOOT_PFOOD" ), where, range ) ) {
if( perishable && has_near( zone_type_id( "LOOT_PFOOD" ), where, range ) ) {
return zone_type_id( "LOOT_PFOOD" );
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/item.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6821,6 +6821,11 @@ item_pocket *item::contained_where( const item &contained )
return contents.contained_where( contained );
}

const item_pocket *item::contained_where( const item &contained ) const
{
return const_cast<item *>( this )->contained_where( contained );
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this const_cast does anything here. The method is already marked const so this is const already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually casting this to a mutable item * so that we can use the mutable contained_where implementation. We can't call it without the cast, because the mutable version returns a mutable reference, which is a no-go from a const object. It's a common C++ trick to avoid having to duplicate the entire method body for these const/mutable method pairs, and it's safe as long as your mutable version doesn't have side effects.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see; thanks!

}

bool item::is_watertight_container() const
{
return contents.can_contain_liquid( true );
Expand Down
1 change: 1 addition & 0 deletions src/item.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ class item : public visitable<item>
// for pocket update stuff, which pocket is @contained in?
// returns a nullptr if the item is not contaiend, and prints a debug message
item_pocket *contained_where( const item &contained );
const item_pocket *contained_where( const item &contained ) const;
/** Whether this is a container which can be used to store liquids. */
bool is_watertight_container() const;
/** Whether this item has no contents at all. */
Expand Down
235 changes: 235 additions & 0 deletions tests/clzones_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
#include "catch/catch.hpp"

#include "clzones.h"
#include "game_constants.h"
#include "item.h"
#include "item_category.h"
#include "map.h"
#include "map_helpers.h"

static const zone_type_id zone_type_LOOT_UNSORTED( "LOOT_UNSORTED" );
static const zone_type_id zone_type_LOOT_FOOD( "LOOT_FOOD" );
static const zone_type_id zone_type_LOOT_PFOOD( "LOOT_PFOOD" );
static const zone_type_id zone_type_LOOT_DRINK( "LOOT_DRINK" );
static const zone_type_id zone_type_LOOT_PDRINK( "LOOT_PDRINK" );

static void create_tile_zone( const std::string &name, const zone_type_id &zone_type, tripoint pos )
{
zone_manager &zm = zone_manager::get_manager();
zm.add( name, zone_type, faction_id( "your_followers" ), false, true, pos, pos );
}

// Comestibles sorting is a bit awkward. Unlike other loot, they're almost
// always inside of a container, and their sort zone changes based on their
// shelf life and whether the container prevents rotting.
TEST_CASE( "zone sorting comestibles ", "[zones][items][food][activities]" )
{
clear_map();
zone_manager &zm = zone_manager::get_manager();

const tripoint origin_pos;
Squishums marked this conversation as resolved.
Show resolved Hide resolved
kevingranade marked this conversation as resolved.
Show resolved Hide resolved
create_tile_zone( "Food", zone_type_LOOT_FOOD, tripoint_east );
create_tile_zone( "Drink", zone_type_LOOT_DRINK, tripoint_west );

SECTION( "without perishable zones" ) {
GIVEN( "a non-perishable food" ) {
item nonperishable_food( "test_bitter_almond" );
REQUIRE_FALSE( nonperishable_food.goes_bad() );

WHEN( "sorting without a container" ) {
THEN( "should put in the food zone" ) {
CHECK( zm.get_near_zone_type_for_item( nonperishable_food, origin_pos ) == zone_type_LOOT_FOOD );
}
}
}

GIVEN( "a non-perishable drink" ) {
item nonperishable_drink( "test_wine" );
REQUIRE_FALSE( nonperishable_drink.goes_bad() );

WHEN( "sorting without a container" ) {
THEN( "should put in the drink zone" ) {
CHECK( zm.get_near_zone_type_for_item( nonperishable_drink, origin_pos ) == zone_type_LOOT_DRINK );
}
}
}

GIVEN( "a perishable food" ) {
item perishable_food( "test_apple" );
REQUIRE( perishable_food.goes_bad() );

WHEN( "sorting without a container" ) {
THEN( "should put in the food zone" ) {
CHECK( zm.get_near_zone_type_for_item( perishable_food, origin_pos ) == zone_type_LOOT_FOOD );
}
}
}

GIVEN( "a perishable drink" ) {
item perishable_drink( "test_milk" );
REQUIRE( perishable_drink.goes_bad() );

WHEN( "sorting without a container" ) {
THEN( "should put in the drink zone" ) {
CHECK( zm.get_near_zone_type_for_item( perishable_drink, origin_pos ) == zone_type_LOOT_DRINK );
}
}
}
}

SECTION( "with perishable zones" ) {
create_tile_zone( "PFood", zone_type_LOOT_PFOOD, tripoint_north );
create_tile_zone( "PDrink", zone_type_LOOT_PDRINK, tripoint_south );

GIVEN( "a non-perishable food" ) {
item nonperishable_food( "test_bitter_almond" );
REQUIRE_FALSE( nonperishable_food.goes_bad() );

WHEN( "sorting without a container" ) {
THEN( "should put in the food zone" ) {
CHECK( zm.get_near_zone_type_for_item( nonperishable_food, origin_pos ) == zone_type_LOOT_FOOD );
}
}

WHEN( "sorting within an unsealed container" ) {
item container( "test_watertight_open_sealed_container_250ml" );
REQUIRE( container.put_in( nonperishable_food, item_pocket::pocket_type::CONTAINER ).success() );
REQUIRE( container.contents.get_sealed_summary() == item_contents::sealed_summary::unsealed );

THEN( "should put in the food zone" ) {
CHECK( zm.get_near_zone_type_for_item( container, origin_pos ) == zone_type_LOOT_FOOD );
}
}

WHEN( "sorting within a sealed container" ) {
item container( "test_watertight_open_sealed_container_250ml" );
REQUIRE( container.put_in( nonperishable_food, item_pocket::pocket_type::CONTAINER ).success() );
REQUIRE( container.seal() );
REQUIRE( container.contents.get_all_contained_pockets().value().front()->spoil_multiplier() ==
0.0f );
REQUIRE( container.contents.get_sealed_summary() == item_contents::sealed_summary::all_sealed );

THEN( "should put in the food zone" ) {
CHECK( zm.get_near_zone_type_for_item( container, origin_pos ) == zone_type_LOOT_FOOD );
}
}
}

GIVEN( "a non-perishable drink" ) {
item nonperishable_drink( "test_wine" );
REQUIRE_FALSE( nonperishable_drink.goes_bad() );

WHEN( "sorting without a container" ) {
THEN( "should put in the drink zone" ) {
CHECK( zm.get_near_zone_type_for_item( nonperishable_drink, origin_pos ) == zone_type_LOOT_DRINK );
}
}

WHEN( "sorting within an unsealed container" ) {
item container( "test_watertight_open_sealed_container_250ml" );
REQUIRE( container.put_in( nonperishable_drink, item_pocket::pocket_type::CONTAINER ).success() );
REQUIRE( container.contents.get_sealed_summary() == item_contents::sealed_summary::unsealed );

THEN( "should put in the drink zone" ) {
CHECK( zm.get_near_zone_type_for_item( container, origin_pos ) == zone_type_LOOT_DRINK );
}
}

WHEN( "sorting within a sealed container" ) {
item container( "test_watertight_open_sealed_container_250ml" );
REQUIRE( container.put_in( nonperishable_drink, item_pocket::pocket_type::CONTAINER ).success() );
REQUIRE( container.seal() );
REQUIRE( container.contents.get_all_contained_pockets().value().front()->spoil_multiplier() ==
0.0f );
REQUIRE( container.contents.get_sealed_summary() == item_contents::sealed_summary::all_sealed );

THEN( "should put in the drink zone" ) {
CHECK( zm.get_near_zone_type_for_item( container, origin_pos ) == zone_type_LOOT_DRINK );
}
}
}


GIVEN( "a perishable food" ) {
item perishable_food( "test_apple" );
REQUIRE( perishable_food.goes_bad() );

WHEN( "sorting without a container" ) {
THEN( "should put in the perishable food zone" ) {
CHECK( zm.get_near_zone_type_for_item( perishable_food, origin_pos ) == zone_type_LOOT_PFOOD );
}
}

WHEN( "sorting within an unsealed container" ) {
item container( "test_watertight_open_sealed_container_250ml" );
REQUIRE( container.put_in( perishable_food, item_pocket::pocket_type::CONTAINER ).success() );
REQUIRE( container.contents.get_sealed_summary() == item_contents::sealed_summary::unsealed );

THEN( "should put in the perishable food zone" ) {
CHECK( zm.get_near_zone_type_for_item( container, origin_pos ) == zone_type_LOOT_PFOOD );
}
}

WHEN( "sorting within a sealed container" ) {
item container( "test_watertight_open_sealed_container_250ml" );
REQUIRE( container.put_in( perishable_food, item_pocket::pocket_type::CONTAINER ).success() );
REQUIRE( container.seal() );
REQUIRE( container.contents.get_all_contained_pockets().value().front()->spoil_multiplier() ==
0.0f );
REQUIRE( container.contents.get_sealed_summary() == item_contents::sealed_summary::all_sealed );

THEN( "should put in the food zone" ) {
CHECK( zm.get_near_zone_type_for_item( container, origin_pos ) == zone_type_LOOT_FOOD );
}
}
}

GIVEN( "a perishable drink" ) {
item perishable_drink( "test_milk" );
REQUIRE( perishable_drink.goes_bad() );

WHEN( "sorting without a container" ) {
THEN( "should put in the perishable drink zone" ) {
CHECK( zm.get_near_zone_type_for_item( perishable_drink, origin_pos ) == zone_type_LOOT_PDRINK );
}
}

WHEN( "sorting within an unsealed container" ) {
item container( "test_watertight_open_sealed_container_250ml" );
REQUIRE( container.put_in( perishable_drink, item_pocket::pocket_type::CONTAINER ).success() );
REQUIRE( container.contents.get_sealed_summary() == item_contents::sealed_summary::unsealed );

THEN( "should put in the perishable drink zone" ) {
CHECK( zm.get_near_zone_type_for_item( container, origin_pos ) == zone_type_LOOT_PDRINK );
}
}

WHEN( "sorting within a sealed container" ) {
item container( "test_watertight_open_sealed_container_250ml" );
REQUIRE( container.put_in( perishable_drink, item_pocket::pocket_type::CONTAINER ).success() );
REQUIRE( container.seal() );
REQUIRE( container.contents.get_all_contained_pockets().value().front()->spoil_multiplier() ==
0.0f );
REQUIRE( container.contents.get_sealed_summary() == item_contents::sealed_summary::all_sealed );

THEN( "should put in the drink zone" ) {
CHECK( zm.get_near_zone_type_for_item( container, origin_pos ) == zone_type_LOOT_DRINK );
}
}
}


// MREs are under the food category but are not directly edible.
GIVEN( "a non-comestible food" ) {
item noncomestible_food( "mre_dessert" );
REQUIRE( noncomestible_food.get_category_shallow().get_id() == item_category_id( "food" ) );
REQUIRE_FALSE( noncomestible_food.is_comestible() );

WHEN( "sorting" ) {
THEN( "should put in the food zone" ) {
CHECK( zm.get_near_zone_type_for_item( noncomestible_food, origin_pos ) == zone_type_LOOT_FOOD );
}
}
}
}
}
15 changes: 15 additions & 0 deletions tests/map_helpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "npc.h"
#include "point.h"
#include "type_id.h"
#include "clzones.h"

// Remove all vehicles from the map
void clear_vehicles()
Expand Down Expand Up @@ -85,13 +86,27 @@ void clear_items( const int zlevel )
}
}

void clear_zones()
{
zone_manager &zm = zone_manager::get_manager();
for( auto zone_ref : zm.get_zones( faction_id( "your_followers" ) ) ) {
if( !zone_ref.get().get_is_vehicle() ) {
// Trying to delete vehicle zones fails with a message that the zone isn't loaded.
// Don't need it right now and the errors spam up the test output, so skip.
continue;
}
zm.remove( zone_ref.get() );
}
}

void clear_map()
{
// Clearing all z-levels is rather slow, so just clear the ones I know the
// tests use for now.
for( int z = -2; z <= 0; ++z ) {
clear_fields( z );
}
clear_zones();
wipe_map_terrain();
clear_npcs();
clear_creatures();
Expand Down
1 change: 1 addition & 0 deletions tests/map_helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ void clear_creatures();
void clear_npcs();
void clear_fields( int zlevel );
void clear_items( int zlevel );
void clear_zones();
void clear_map();
void clear_map_and_put_player_underground();
monster &spawn_test_monster( const std::string &monster_type, const tripoint &start );
Expand Down