From e0fd2e7c476b995ccac973d34c0c7d63b5e52353 Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Sun, 2 Feb 2020 20:04:30 -0700 Subject: [PATCH 1/9] Add item description when lacking crafting knowledge To encourage player to memorize more recipes, and/or make it clearer that "You could craft" would appear in this place if more recipes were known. Resolves #37643 Splits the existing `if` condition into two parts, first to check if known recipes are empty, and display this message at the bottom of the item description panel: You don't know anything you could craft with it. Note, unless recipes are listed, this message will be shown for all items, whether or not recipes exist for the item. --- src/item.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/item.cpp b/src/item.cpp index 5ed41661ba9fa..acba0d127fa7e 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -3302,7 +3302,11 @@ void item::final_info( std::vector &info, const iteminfo_query *parts, tid = contents.front().typeId(); } const std::set &known_recipes = g->u.get_learned_recipes().of_component( tid ); - if( !known_recipes.empty() && parts->test( iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES ) ) { + if( known_recipes.empty() ) { + insert_separation_line( info ); + info.push_back( iteminfo( "DESCRIPTION", + _( "You don't know anything you could craft with it." ) ) ); + } else if( parts->test( iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES ) ) { const inventory &inv = g->u.crafting_inventory(); if( known_recipes.size() > 24 ) { From f45a1e7e15b26c69365a4f22e73b6eeb9e83d293 Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Mon, 3 Feb 2020 16:26:53 -0700 Subject: [PATCH 2/9] Refactor item 'you could craft' section Some failed test cases in `tests/iteminfo_test.cpp` made me realize my previous commit should have kept the `DESCRIPTION_APPLICABLE_RECIPES` check, since that is a flag indicating whether the section is applicable to that kind of item, and should be shown at all. Doing that check first made it clear that the preceding block, initializing `tid` and `known_recipes`, could move inside the `if` and avoid being needlessly calculated in some cases. Due to indentation, this looks like a larger diff than it really is, but it's all the same logic as before - if no recipes are known, the section will display "You don't know anything you could craft with it." Verified that all tests in `make check` are passing now, and confirmed in-game that messages are shown as expected. --- src/item.cpp | 65 +++++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/item.cpp b/src/item.cpp index acba0d127fa7e..e43e35efd295c 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -3295,43 +3295,46 @@ void item::final_info( std::vector &info, const iteminfo_query *parts, } // list recipes you could use it in - itype_id tid; - if( contents.empty() ) { // use this item - tid = typeId(); - } else { // use the contained item - tid = contents.front().typeId(); - } - const std::set &known_recipes = g->u.get_learned_recipes().of_component( tid ); - if( known_recipes.empty() ) { - insert_separation_line( info ); - info.push_back( iteminfo( "DESCRIPTION", - _( "You don't know anything you could craft with it." ) ) ); - } else if( parts->test( iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES ) ) { - const inventory &inv = g->u.crafting_inventory(); + if( parts->test( iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES ) ) { + itype_id tid; + if( contents.empty() ) { // use this item + tid = typeId(); + } else { // use the contained item + tid = contents.front().typeId(); + } + const std::set &known_recipes = g->u.get_learned_recipes().of_component( tid ); - if( known_recipes.size() > 24 ) { - insert_separation_line( info ); - info.push_back( iteminfo( "DESCRIPTION", - _( "You know dozens of things you could craft with it." ) ) ); - } else if( known_recipes.size() > 12 ) { + if( known_recipes.empty() ) { insert_separation_line( info ); info.push_back( iteminfo( "DESCRIPTION", - _( "You could use it to craft various other things." ) ) ); + _( "You don't know anything you could craft with it." ) ) ); } else { - const std::string recipes = enumerate_as_string( known_recipes.begin(), known_recipes.end(), - [ &inv ]( const recipe * r ) { - if( r->deduped_requirements().can_make_with_inventory( - inv, r->get_component_filter() ) ) { - return r->result_name(); - } else { - return string_format( "%s", r->result_name() ); - } - } ); - if( !recipes.empty() ) { + const inventory &inv = g->u.crafting_inventory(); + + if( known_recipes.size() > 24 ) { insert_separation_line( info ); info.push_back( iteminfo( "DESCRIPTION", - string_format( _( "You could use it to craft: %s" ), - recipes ) ) ); + _( "You know dozens of things you could craft with it." ) ) ); + } else if( known_recipes.size() > 12 ) { + insert_separation_line( info ); + info.push_back( iteminfo( "DESCRIPTION", + _( "You could use it to craft various other things." ) ) ); + } else { + const std::string recipes = enumerate_as_string( known_recipes.begin(), known_recipes.end(), + [ &inv ]( const recipe * r ) { + if( r->deduped_requirements().can_make_with_inventory( + inv, r->get_component_filter() ) ) { + return r->result_name(); + } else { + return string_format( "%s", r->result_name() ); + } + } ); + if( !recipes.empty() ) { + insert_separation_line( info ); + info.push_back( iteminfo( "DESCRIPTION", + string_format( _( "You could use it to craft: %s" ), + recipes ) ) ); + } } } } From 3228bad2554c1647a0ae78d6bc0826cac6cf15de Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Thu, 6 Feb 2020 17:14:32 -0700 Subject: [PATCH 3/9] WIP Attempt to use get_available_recipes In discussion with @codemime, determined that `player::get_available_recipes` is suitable for use here in place of `get_learned_recipes`, but my C++fu is weak and I've somehow caused a game crash that doesn't even include this file in the backtrace. Committing to ask for help on what I'm doing wrong. --- src/item.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/item.cpp b/src/item.cpp index e43e35efd295c..0cdf3a74847c7 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -3302,28 +3302,30 @@ void item::final_info( std::vector &info, const iteminfo_query *parts, } else { // use the contained item tid = contents.front().typeId(); } - const std::set &known_recipes = g->u.get_learned_recipes().of_component( tid ); - if( known_recipes.empty() ) { + // FIXME: Somehow the following lines are causing a game crash whenever the crafting menu is opened + const inventory &crafting_inv = g->u.crafting_inventory(); + const std::set &item_recipes = g->u.get_available_recipes( crafting_inv, + nullptr ).of_component( tid ); + + if( item_recipes.empty() ) { insert_separation_line( info ); info.push_back( iteminfo( "DESCRIPTION", _( "You don't know anything you could craft with it." ) ) ); } else { - const inventory &inv = g->u.crafting_inventory(); - - if( known_recipes.size() > 24 ) { + if( item_recipes.size() > 24 ) { insert_separation_line( info ); info.push_back( iteminfo( "DESCRIPTION", _( "You know dozens of things you could craft with it." ) ) ); - } else if( known_recipes.size() > 12 ) { + } else if( item_recipes.size() > 12 ) { insert_separation_line( info ); info.push_back( iteminfo( "DESCRIPTION", _( "You could use it to craft various other things." ) ) ); } else { - const std::string recipes = enumerate_as_string( known_recipes.begin(), known_recipes.end(), - [ &inv ]( const recipe * r ) { + const std::string recipes = enumerate_as_string( item_recipes.begin(), item_recipes.end(), + [ &crafting_inv ]( const recipe * r ) { if( r->deduped_requirements().can_make_with_inventory( - inv, r->get_component_filter() ) ) { + crafting_inv, r->get_component_filter() ) ) { return r->result_name(); } else { return string_format( "%s", r->result_name() ); From 5617811a8c79e3b270b7479d466d987814cbfe04 Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Thu, 6 Feb 2020 18:19:00 -0700 Subject: [PATCH 4/9] Fix crash, tweak message Avoid ending up with a null pointer refererence from `of_component`. Shorten the "no recipes" message slightly to make it fit without wrapping more often. --- src/item.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/item.cpp b/src/item.cpp index 0cdf3a74847c7..c8ec6efe45d10 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -3303,15 +3303,14 @@ void item::final_info( std::vector &info, const iteminfo_query *parts, tid = contents.front().typeId(); } - // FIXME: Somehow the following lines are causing a game crash whenever the crafting menu is opened const inventory &crafting_inv = g->u.crafting_inventory(); - const std::set &item_recipes = g->u.get_available_recipes( crafting_inv, - nullptr ).of_component( tid ); + const recipe_subset available_recipe_subset = g->u.get_available_recipes( crafting_inv ); + const std::set &item_recipes = available_recipe_subset.of_component( tid ); if( item_recipes.empty() ) { insert_separation_line( info ); info.push_back( iteminfo( "DESCRIPTION", - _( "You don't know anything you could craft with it." ) ) ); + _( "You know of nothing you could craft with it." ) ) ); } else { if( item_recipes.size() > 24 ) { insert_separation_line( info ); From 793e3b59860acd178af56af80a2352c93e5ad84d Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Fri, 7 Feb 2020 16:21:52 -0700 Subject: [PATCH 5/9] WIP Add test for item recipes in description --- tests/iteminfo_test.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/iteminfo_test.cpp b/tests/iteminfo_test.cpp index 46a1e4ea495e1..3d9adb4f45aec 100644 --- a/tests/iteminfo_test.cpp +++ b/tests/iteminfo_test.cpp @@ -99,3 +99,43 @@ TEST_CASE( "nutrient_ranges_for_recipe_exemplars", "[item][iteminfo]" ) "Vitamins (RDA): Calcium (7-28%), Iron (0-83%), " "Vitamin A (3-11%), Vitamin B12 (2-6%), and Vitamin C (1-85%)\n" ); } + +TEST_CASE( "show_available_recipes_using_item", "[item][iteminfo]" ) +{ + iteminfo_query q( { iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES } ); + + GIVEN( "a character with no cooking skill" ) { + avatar character; + const recipe *pur = &recipe_id( "pur_tablets" ).obj(); + + REQUIRE( character.get_skill_level( pur->skill_used ) == 0 ); + REQUIRE_FALSE( character.knows_recipe( pur ) ); + + WHEN( "they have potassium iodide tablets in their inventory" ) { + item &iodine = character.i_add( item( "iodine" ) ); + + THEN( "nothing is craftable from it" ) { + std::vector info_v; + std::string info = iodine.info( info_v, &q, 1 ); + CHECK( info == "--\nYou know of nothing you could craft with it.\n" ); + } + + AND_WHEN( "they have the water purification tablet recipe memorized" ) { + item &textbook = character.i_add( item( "textbook_chemistry" ) ); + character.set_skill_level( pur->skill_used, pur->difficulty + 1 ); + character.learn_recipe( pur ); + REQUIRE( character.knows_recipe( pur ) ); + + THEN( "they can use potassium iodide tablets to craft it" ) { + std::vector info_v; + std::string info = iodine.info( info_v, &q, 1 ); + CHECK( info == "--\n" + "You could use it to craft: " + "water purification tablet\n" ); + } + } + } + + } +} + From 3ffc4f0ec4dc7a485dc322e6d2f16333a82b7fa0 Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Sat, 8 Feb 2020 14:27:06 -0700 Subject: [PATCH 6/9] Use ternary expression instead of 6-liner --- src/item.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/item.cpp b/src/item.cpp index 50af046e87ad1..3db57d5ffc59c 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -3296,13 +3296,7 @@ void item::final_info( std::vector &info, const iteminfo_query *parts, // list recipes you could use it in if( parts->test( iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES ) ) { - itype_id tid; - if( contents.empty() ) { // use this item - tid = typeId(); - } else { // use the contained item - tid = contents.front().typeId(); - } - + itype_id tid = contents.empty() ? typeId() : contents.front().typeId(); const inventory &crafting_inv = g->u.crafting_inventory(); const recipe_subset available_recipe_subset = g->u.get_available_recipes( crafting_inv ); const std::set &item_recipes = available_recipe_subset.of_component( tid ); From 3561ef31b2119db9f60c6281f3619f707dbaa4c5 Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Sat, 8 Feb 2020 17:57:34 -0700 Subject: [PATCH 7/9] WIP Improve item recipe tests (still failing) Test coverage for modified item info behavior --- tests/iteminfo_test.cpp | 87 ++++++++++++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 22 deletions(-) diff --git a/tests/iteminfo_test.cpp b/tests/iteminfo_test.cpp index 3d9adb4f45aec..d6afa98c423a8 100644 --- a/tests/iteminfo_test.cpp +++ b/tests/iteminfo_test.cpp @@ -7,6 +7,7 @@ #include "game.h" #include "item.h" #include "iteminfo_query.h" +#include "recipe_dictionary.h" static void iteminfo_test( const item &i, const iteminfo_query &q, const std::string &reference ) { @@ -100,42 +101,84 @@ TEST_CASE( "nutrient_ranges_for_recipe_exemplars", "[item][iteminfo]" ) "Vitamin A (3-11%), Vitamin B12 (2-6%), and Vitamin C (1-85%)\n" ); } -TEST_CASE( "show_available_recipes_using_item", "[item][iteminfo]" ) +TEST_CASE( "show available recipes with item as an ingredient", "[item][iteminfo][recipes]" ) { iteminfo_query q( { iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES } ); + const recipe *crowbar = &recipe_id( "makeshift_crowbar" ).obj(); + const recipe *purtab = &recipe_id( "pur_tablets" ).obj(); + g->u.empty_skills(); - GIVEN( "a character with no cooking skill" ) { - avatar character; - const recipe *pur = &recipe_id( "pur_tablets" ).obj(); + GIVEN( "character has a pipe and no skill" ) { + item &pipe = g->u.i_add( item( "pipe" ) ); - REQUIRE( character.get_skill_level( pur->skill_used ) == 0 ); - REQUIRE_FALSE( character.knows_recipe( pur ) ); + REQUIRE( g->u.get_skill_level( crowbar->skill_used ) == 0 ); - WHEN( "they have potassium iodide tablets in their inventory" ) { - item &iodine = character.i_add( item( "iodine" ) ); + THEN( "they can craft a makeshift crowbar from it" ) { + iteminfo_test( + pipe, q, + "--\nYou could use it to craft: makeshift crowbar\n" ); + } + } + + GIVEN( "character has potassium iodide tablets" ) { + item &iodine = g->u.i_add( item( "iodine" ) ); + + WHEN( "they don't have skills or recipes" ) { + g->u.set_skill_level( purtab->skill_used, 0 ); + + REQUIRE( g->u.get_skill_level( purtab->skill_used ) == 0 ); + REQUIRE_FALSE( g->u.get_available_recipes( g->u.crafting_inventory() ).contains( purtab ) ); THEN( "nothing is craftable from it" ) { - std::vector info_v; - std::string info = iodine.info( info_v, &q, 1 ); - CHECK( info == "--\nYou know of nothing you could craft with it.\n" ); + iteminfo_test( + iodine, q, + "--\nYou know of nothing you could craft with it.\n" ); + } + } + + AND_WHEN( "they acquire the needed skill" ) { + g->u.set_skill_level( purtab->skill_used, purtab->difficulty ); + + REQUIRE( g->u.get_skill_level( purtab->skill_used ) == purtab->difficulty ); + REQUIRE_FALSE( g->u.get_available_recipes( g->u.crafting_inventory() ).contains( purtab ) ); + + THEN( "still nothing is craftable from it" ) { + iteminfo_test( + iodine, q, + "--\nYou know of nothing you could craft with it.\n" ); } - AND_WHEN( "they have the water purification tablet recipe memorized" ) { - item &textbook = character.i_add( item( "textbook_chemistry" ) ); - character.set_skill_level( pur->skill_used, pur->difficulty + 1 ); - character.learn_recipe( pur ); - REQUIRE( character.knows_recipe( pur ) ); + WHEN( "they have no book, but have the recipe memorized" ) { + g->u.learn_recipe( purtab ); + + REQUIRE( g->u.knows_recipe( purtab ) ); + REQUIRE( g->u.get_available_recipes( g->u.crafting_inventory() ).contains( purtab ) ); THEN( "they can use potassium iodide tablets to craft it" ) { - std::vector info_v; - std::string info = iodine.info( info_v, &q, 1 ); - CHECK( info == "--\n" - "You could use it to craft: " - "water purification tablet\n" ); + iteminfo_test( + iodine, q, + "--\n" + "You could use it to craft: " + "water purification tablet\n" ); } } - } + WHEN( "they have the recipe in a book, but not memorized" ) { + item &book = g->u.i_add( item( "textbook_chemistry" ) ); + g->u.do_read( book ); + + REQUIRE_FALSE( g->u.knows_recipe( purtab ) ); + REQUIRE( g->u.get_available_recipes( g->u.crafting_inventory() ).contains( purtab ) ); + + THEN( "they can use potassium iodide tablets to craft it" ) { + iteminfo_test( + iodine, q, + "--\n" + "You could use it to craft: " + "water purification tablet\n" ); + } + } + } } } From 6fb8d13f6b2cbe994a006e9367bd69999c695075 Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Sat, 8 Feb 2020 18:22:09 -0700 Subject: [PATCH 8/9] Fix item recipe info test cases Reduced to essential assertions; all passing now --- tests/iteminfo_test.cpp | 44 +++++++++-------------------------------- 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/tests/iteminfo_test.cpp b/tests/iteminfo_test.cpp index d6afa98c423a8..8b58d90a51928 100644 --- a/tests/iteminfo_test.cpp +++ b/tests/iteminfo_test.cpp @@ -106,41 +106,21 @@ TEST_CASE( "show available recipes with item as an ingredient", "[item][iteminfo iteminfo_query q( { iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES } ); const recipe *crowbar = &recipe_id( "makeshift_crowbar" ).obj(); const recipe *purtab = &recipe_id( "pur_tablets" ).obj(); - g->u.empty_skills(); - - GIVEN( "character has a pipe and no skill" ) { - item &pipe = g->u.i_add( item( "pipe" ) ); - - REQUIRE( g->u.get_skill_level( crowbar->skill_used ) == 0 ); - - THEN( "they can craft a makeshift crowbar from it" ) { - iteminfo_test( - pipe, q, - "--\nYou could use it to craft: makeshift crowbar\n" ); - } - } + g->u.empty_traits(); - GIVEN( "character has potassium iodide tablets" ) { + GIVEN( "character has a potassium iodide tablet and no skill" ) { item &iodine = g->u.i_add( item( "iodine" ) ); + g->u.empty_skills(); - WHEN( "they don't have skills or recipes" ) { - g->u.set_skill_level( purtab->skill_used, 0 ); - - REQUIRE( g->u.get_skill_level( purtab->skill_used ) == 0 ); - REQUIRE_FALSE( g->u.get_available_recipes( g->u.crafting_inventory() ).contains( purtab ) ); - - THEN( "nothing is craftable from it" ) { - iteminfo_test( - iodine, q, - "--\nYou know of nothing you could craft with it.\n" ); - } + THEN( "nothing is craftable from it" ) { + iteminfo_test( + iodine, q, + "--\nYou know of nothing you could craft with it.\n" ); } - AND_WHEN( "they acquire the needed skill" ) { + WHEN( "they acquire the needed skills" ) { g->u.set_skill_level( purtab->skill_used, purtab->difficulty ); - REQUIRE( g->u.get_skill_level( purtab->skill_used ) == purtab->difficulty ); - REQUIRE_FALSE( g->u.get_available_recipes( g->u.crafting_inventory() ).contains( purtab ) ); THEN( "still nothing is craftable from it" ) { iteminfo_test( @@ -150,9 +130,7 @@ TEST_CASE( "show available recipes with item as an ingredient", "[item][iteminfo WHEN( "they have no book, but have the recipe memorized" ) { g->u.learn_recipe( purtab ); - REQUIRE( g->u.knows_recipe( purtab ) ); - REQUIRE( g->u.get_available_recipes( g->u.crafting_inventory() ).contains( purtab ) ); THEN( "they can use potassium iodide tablets to craft it" ) { iteminfo_test( @@ -164,11 +142,7 @@ TEST_CASE( "show available recipes with item as an ingredient", "[item][iteminfo } WHEN( "they have the recipe in a book, but not memorized" ) { - item &book = g->u.i_add( item( "textbook_chemistry" ) ); - g->u.do_read( book ); - - REQUIRE_FALSE( g->u.knows_recipe( purtab ) ); - REQUIRE( g->u.get_available_recipes( g->u.crafting_inventory() ).contains( purtab ) ); + g->u.i_add( item( "textbook_chemistry" ) ); THEN( "they can use potassium iodide tablets to craft it" ) { iteminfo_test( From 39d32cf08e87b683b7dc054031112dcb5ca23470 Mon Sep 17 00:00:00 2001 From: Eric Pierce Date: Sat, 8 Feb 2020 20:39:39 -0700 Subject: [PATCH 9/9] Remove unused crowbar variable Caught by Travis CI --- tests/iteminfo_test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/iteminfo_test.cpp b/tests/iteminfo_test.cpp index 8b58d90a51928..7c7fdd7c80cc1 100644 --- a/tests/iteminfo_test.cpp +++ b/tests/iteminfo_test.cpp @@ -104,7 +104,6 @@ TEST_CASE( "nutrient_ranges_for_recipe_exemplars", "[item][iteminfo]" ) TEST_CASE( "show available recipes with item as an ingredient", "[item][iteminfo][recipes]" ) { iteminfo_query q( { iteminfo_parts::DESCRIPTION_APPLICABLE_RECIPES } ); - const recipe *crowbar = &recipe_id( "makeshift_crowbar" ).obj(); const recipe *purtab = &recipe_id( "pur_tablets" ).obj(); g->u.empty_traits();