Auto-eat sorts by spoilage

[chaosvolt]

Summary

SUMMARY: Balance "Auto-eat now prioritizes items based on shelf life instead of first-dropped unsorted order"

Purpose of change

And this is the other half of the adjustments to auto-eat zones that Viss requested, they asked that someone look into seeing if auto-eat/drink can be reordered to favor whatever will spoil first, much like how the Eat menu orders things by spoilage.

Describe the solution

previous solution by chaosvolt

1. Added a call to stable_sort to find_auto_consume in activity_item_handling.cpp, that sorts the items found by std::vector<item *> items_here; so that the item closest to going bad gets favored. That way it behaves more like the sorting used by the Eat menu instead of a plain unsorted order, which just takes whather was placed in that tile first.
2. Per Coolthulhu's request, left in an add_msg that was used to test and confirm that one of the behaviors observed while testing was due to one of the filtration if statements excluding items that're too filling to currently eat. This is to be deleted (or converted to debug-only if one really wants it) prior to merging.

1. extracted logic to check if player should eat the food to ok_to_consume lambda.
2. made an unnamed struct which stores item pointer, tripoint and item_location.
3. the code loops thru items in each location, and replace the struct if item has shorter shelf life.
4. unnested eating and vehicle iteration.

Describe alternatives you've considered

Screaming.

Testing

1. Compiled and load-tested.
2. Set character to be decently hungry () and placed an auto-eat zone.
3. Placed 20 hardtack, then 20 cooked meat, then 20 pine nuts into the zone. From prior experience, I know that the hardtack will be the first item in unsorted order due to being placed first, but having the cooked meat in the middle idiotproofs my testing by ensuring that even if I forget which is at the actual top of unsorted order, the answer is still not cooked meat.
4. Waited a brief bit. With the level of kcal I'd set, the character consistently prefers to eat 3 cooked meat, then the added messages indicate that they skip over cooked meat (because too filling now) in favor of pine nuts.

Cooked meat being the first choice in this instance confirms it's working as expected, as under the previous unsorted logic the hardtack would normally be chosen first.

It choosing pine nuts also indicates it's favoring them because they spoil sooner than hardtack, and are still being picked despite being lower on the list according to unsorted order.

8. Repeated this test, instead dropping in 20 hardtack, then 20 pine nuts, then 20 cooked meat, then 20 vegetable aspic. This time the PC favored eating 3 cooked meat, then 1 vegetable aspic. Vegetable aspic has the second shortest shelf life of the 4 items used, so it being picked as the second choice after the PC fills up most of the way on cooked meat, despite being dead last in unsorted order, further indicates it's working right.
9. Astyled affected file.

Additional context

Showing initial drop order and cooked meat being prioritized instead:

Same but with skip-over messages added:

Showing the above items in unsorted order, where hardtack would normally take precedence due to being the first item added:

Showing the results of the other test where vegetable aspic was added:

[Lamandus]

if the message is to be deleted before merging, why is it still in? for BFN testing?

[chaosvolt]

BFN asked for it on the discord for some reason.

[Coolthulhu]

BFN asked for it on the discord for some reason.

I meant, it was supposed to be there to help you debug and only for that.

[chaosvolt]

Deleted the message since it's served its purpose. I don't see any way to pretty up the added function though, not without losing the use of stable_sort which all seems to be similarly ugly (and just regular sort seems like it won't be adequate?).

[Firestorm01X2]

@chaosvolt I have reports that player drink vinegar on auto eat.

[Firestorm01X2]

I have reports that player drink vinegar on auto eat.

@chaosvolt Can't reproduce the bug. Maybe mod case

[chaosvolt]

Vinegar has zero quench value (https://github.com/cataclysmbnteam/Cataclysm-BN/blob/upload/data/json/items/comestibles/drink_other.json#L167) so it should be excluded by this:

            if( !food && comest.get_comestible()->quench < 15 ) {
                // not quenching enough
                continue;
            }

[Lamandus]

Vinegar has zero quench value (https://github.com/cataclysmbnteam/Cataclysm-BN/blob/upload/data/json/items/comestibles/drink_other.json#L167) so it should be excluded by this:

            if( !food && comest.get_comestible()->quench < 15 ) {
                // not quenching enough
                continue;
            }

is it considered food or drink?

[chaosvolt]

It should be telling it that the food boolean is false, in whichever function passes that bool to find_auto_consume?

[Coolthulhu]

From what I can tell, it will only sort items in a randomly picked pile, not out of all of them. The PR should say so.

It would be possible to sort ALL piles and pick the most rotten item out of them, but that would be more expensive. Not a lot of work to implement, but potentially expensive to run, with cost scaling with nearby item count.

[chaosvolt]

Ah, hmm. Yeah I'm not sure how to ensure it does that, I'd assumed it would already do that from how it seemed to be sorting things.

[Coolthulhu]

I'd assumed it would already do that

It has a return inside the loop, meaning that it can't be accumulating them all. To have it go through everything, it would need to store all the item_locations (the item_loc, given as argument to eat) of potentially-edible items in one vector in one loop, then once all tiles are visited, pick the item with highest "rot percentage".

[chaosvolt]

Ah, that'd explain it. I'll have to look at this when I get home later today.

[scarf005]

alright, rebased to latest upload. been reading the code, and i think i could refactor based on BFN's comment.

@joveeater, would it be possible to improve the performance for batch-sorting? i think they could be parallelized or cached.

[chaosvolt]

Good luck, I can still look at it when I get time but for now still a bit in over my head. ^^"

[scarf005]

hmm, got an idea, create a vector of each tiles' shortest shelf life items, then find shortest among them all
no need to sort

from discord

[scarf005]

new commits work well with food but returns ACTIVITY_NULL on drinks. will investigate when i get home.

[Coolthulhu]

Code looks good.
How's the performance?

[scarf005]

haven't measured it but should be faster since there's no vector copying.

[scarf005]

wrote first ever test file, can't figure out how to make it work.

TEST_CASE( "auto_consume_priority", "[auto_consume][food][zone]" )
{
    clear_all_state();

    map &here = get_map();

    constexpr auto auto_eat_zone_origin = tripoint{ 60, 60, 0 };
    tripoint auto_eat_zone_origin_abs = here.getabs( auto_eat_zone_origin );
    constexpr auto three_steps_rd = tripoint{ 3, 3, 0 };
    auto &zmgr = zone_manager::get_manager();
    zmgr.add( "Auto eat zone", zone_type_id( "AUTO_EAT" ),
              faction_id( "your_followers" ), false, true,
              auto_eat_zone_origin_abs - three_steps_rd,
              auto_eat_zone_origin_abs + three_steps_rd );

    avatar &you = get_avatar();
    you.setpos( auto_eat_zone_origin );
    you.set_stored_kcal( 1000 );

    // Place food in the auto-eat zone
    item hardtack( "hardtack", calendar::turn, 20 );
    item cooked_meat( "meat_cooked", calendar::turn, 20 );
    item pine_nuts( "pine_nuts", calendar::turn, 20 );

    tripoint hardtack_pos = auto_eat_zone_origin + tripoint( 0, 0, 0 );
    tripoint cooked_meat_pos = auto_eat_zone_origin + tripoint( 1, 0, 0 );
    tripoint pine_nuts_pos = auto_eat_zone_origin + tripoint( 2, 0, 0 );

    here.add_item_or_charges( hardtack_pos, hardtack );
    here.add_item_or_charges( cooked_meat_pos, cooked_meat );
    here.add_item_or_charges( pine_nuts_pos, pine_nuts );


    for( const item &it : here.i_at( hardtack_pos ) ) {
        INFO( it.tname() );
        CHECK( it.typeId() == itype_id( "hardtack" ) );
    }

    // Player should auto-consume food
    for( int i = 0; i < 10; i++ ) {
        find_auto_consume( you, consume_type::FOOD );
    }

    // check that the player has consumed the food
    CHECK( you.get_stored_kcal() > 1000 );

    CHECK( here.i_at( hardtack_pos ).size() == 20 );
    CHECK( here.i_at( cooked_meat_pos ).size() == 10 );
    CHECK( here.i_at( pine_nuts_pos ).size() == 10 );
}

/home/scarf/repo/cata/Cataclysm/tests/auto_eat_zone_test.cpp:56: FAILED:
  CHECK( you.get_stored_kcal() > 1000 )
with expansion:
  1,000 (0x3e8) > 1,000 (0x3e8)

/home/scarf/repo/cata/Cataclysm/tests/auto_eat_zone_test.cpp:63: FAILED:
  CHECK( here.i_at( hardtack_pos ).size() == 20 )
with expansion:
  1 == 20

/home/scarf/repo/cata/Cataclysm/tests/auto_eat_zone_test.cpp:64: FAILED:
  CHECK( here.i_at( cooked_meat_pos ).size() == 10 )
with expansion:
  1 == 10

/home/scarf/repo/cata/Cataclysm/tests/auto_eat_zone_test.cpp:65: FAILED:
  CHECK( here.i_at( pine_nuts_pos ).size() == 10 )
with expansion:
  1 == 10

===============================================================================
test cases: 1 | 1 failed
assertions: 5 | 1 passed | 4 failed

[Coolthulhu]

/home/scarf/repo/cata/Cataclysm/tests/auto_eat_zone_test.cpp:63: FAILED:
  CHECK( here.i_at( hardtack_pos ).size() == 20 )
with expansion:
  1 == 20

i_at().size() refers to item count, but the hardtacks are probably merged into one item with 20 charges.

As for the eating failed, it would be best if you split it into "find the item" and "actually eat the item", so that you can test them separately.
You can test them as one, but you'll probably need to debug it to find why does it fail to consume.

[Lamandus]

What is the state of the PR right now?

[scarf005]

it's waiting for review.

[chaosvolt]

So this should be ready given Scarf's improvements on this.

[scarf005]

test for drink is wonky to say at least, but this PR's been sitting for uncomfortably long. as for now it works without visible issues, i'll merge then fix if it breaks afterwards.

[Unrepentant-Atheist]

Das bueno! Das very bueno! Next step: #2573