Test and refactor item info display functions

[wapcaplet]

Summary

SUMMARY: Infrastructure "Test and refactor item info display functions"

Purpose of change

To make the item::info function and its brethren cleaner, more modular, and more well-tested.

Describe the solution

Refactors existing iteminfo test cases and adds many new ones. Refactors a few item::xxx_info functions.

Removes some unnecessary/empty info output, including the following:

The "Compatible magazines:" section, previously displayed for all tools, even those with no magazines:

This "Ammunition" section of combat stats, previously displayed for many kinds of charged battery:

And this empty "Type:" section, formerly displayed for various guns when they did not have a magazine inserted (all glocks, AR pistol, all rivtech guns and many others):

Grants pet armor the possibility to have a get_base_env_resist, meaning their fire / acid / env protection is (supposedly) more correctly indicated in the item info panel now, at least based on my test cases - I could not reproduce the change in-game with existing pet armors.

Adds iteminfo_parts enum values to filter information:

• BASE_OWNER - Item owner (ex. Your Followers)
• BASE_DPS - Typical damage per second
• FOOD_ALLERGEN - This food will cause an allergic reaction
• BOOK_INCLUDED_RECIPES - List of recipes included in a book
• DESCRIPTION_ALLERGEN - This clothing will cause an allergic reaction
• DESCRIPTION_DIE - This item can be used as a die, and has N sides

Adds many new tests, including:

• combat info
  • weapon attack ratings, moves, damage per second
• armor fit and sizing, power armor flag
• armor protection
  • bash/cut/bullet based on material and thickness
  • acid/fire/env require nonzero environmental protection
• volume and weight
  • metric and imperial
• requirements
  • strength, perception, intelligence
  • skills and levels needed
• food info
  • fun, portions, consume time
  • fruit allergy, hidden poison, hidden hallucinogen
  • human flesh and cannibalism
• medicine info
  • quench, fun, stim, portions, consume time, addiction
• gun info
  • range, damage, armor piercing, critical multiplier
  • dispersion, signt dispersion
  • ammo capacity, default ammo
  • recoil, reload time, firing modes, dispersion
• gunmod info
  • flags - reach attack, disable sights, consumable
  • sight dispersion, aim speed, damage modifier
  • compatible guns, mod location
• bionic info
  • power storage, capacity
  • encumbrance, environmental protection
• book info
  • for beginners, morale/fun, time per chapter
  • skill min, skill max, intelligence requirement
• tool info, debug info, and final info

Finally, adds new test items, including suppressor, matches, apple, brewable tennis ball wine, radioactive carafe, super-juicy blueberry gum, poisonous bitter almonds, hallucinogenic nutmeg, some real-life computer literature, power armor, and meower armor.

Describe alternatives you've considered

Everyone knows the item info panel is far less than ideal. Using tabs or some other hierarchical display/navigation would be great, but even just restructuring the existing info is tricky due to the rigid way the functions are grouped and called. I hope to do some amount of refactoring of these into smaller, more modular pieces - but I consider a total iteminfo display overhaul like #38030 to be well beyond the scope of this PR.

I considered a variety of other cosmetic and phrasing changes, optimizing line-wrapping, making bullet points more consistently presented, and various other presentational improvements - but again, felt those things to be diversions, and outside scope.

Except for the removal of useless "Compatible magazines:" and "Ammunition" sections as noted above, and the relocation of a few pieces of gunmod and UPS info into more appropriate sections, there are no intended changes to the in-game info display.

Testing

tests/cata_test [iteminfo]

In-game testing with various items, to ensure their info displays as expected.

Additional context

This test suite and the item info panel itself have been my pet project since #37823, continuing with #40314 and #40686

Test results with [iteminfo] tag:

• Before 111 assertions in 22 test cases
• After: 399 assertions in 40 test cases

[wapcaplet]

There is a bad test case; I was afraid this one might fail, since the item order seems nondeterminstic:

iteminfo_test.cpp:1261: FAILED:
  CHECK( item_info_str( supp, usedon ) == "--\n" "Used on: <color_c_cyan>rifle</color> and <color_c_cyan>pistol</color>\n" )
with expansion:
  "--
  Used on: <color_c_cyan>pistol</color> and <color_c_cyan>rifle</color>
  "
  ==
  "--
  Used on: <color_c_cyan>rifle</color> and <color_c_cyan>pistol</color>
  "

Setting back to WIP to figure it out.

[jbytheway]

It would not be a bad idea to add a new argument to enumerate_as_string to ask it to sort the strings for you. Would be useful in this case, and I bet a bunch of other places that call enumerate_as_string would use it too.

(Remember to use localized comparison)

[wapcaplet]

It would not be a bad idea to add a new argument to enumerate_as_string to ask it to sort the strings for you. Would be useful in this case, and I bet a bunch of other places that call enumerate_as_string would use it too.

(Remember to use localized comparison)

I spent a while studying that function and some of its invocations, but had some difficulty understanding how it works. I'm also hesitant to change that function, since it's so widely used, and has no test cases. Making it do sorting would be nice, but looks like a minor side-project that I think would be out of scope for this PR.

For now I have simply commented out the order-dependent test that was failing.

[wapcaplet]

Travis CI make+curses build is failing on this test case:

npc_test.cpp:255: FAILED:
  REQUIRE( guy != nullptr )
with expansion:
  nullptr != nullptr
with messages:
  Should not crash from infinite recursion
  NPC on acid should not acquire unstable footing status

I don't see any way my changes could be the cause of this.

[kevingranade]

Yea that's been occasionally failing for a while now. unrelated.

…

On Sat, May 30, 2020, 1:18 PM Eric Pierce ***@***.***> wrote: Travis CI make+curses build is failing on this test case: npc_test.cpp:255: FAILED: REQUIRE( guy != nullptr ) with expansion: nullptr != nullptr with messages: Should not crash from infinite recursion NPC on acid should not acquire unstable footing status I don't see any way my changes could be the cause of this. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#40883 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGSA5A35ZO5GG5FJ7BG663RUFSZRANCNFSM4NLR37UA> .

[wapcaplet]

My new tests have a handful of problems with trailing whitespace in clang-tidy, which brought a couple more display bugs to my attention, and there's a conflict in src/item.cpp I need to resolve before this can be merged.