Clean up and organize item info panel

[wapcaplet]

Summary

SUMMARY: Interface "Clean up and organize item info panel"

Purpose of change

This is a broad set of changes aimed at making the item description panel more readable and consistent, and improve player morale by grouping the information in a clear and logical manner.

Describe the solution

Changes made so far:

User interface:

• Moved base item physical characteristics (category, volume, weight, material) above description
• Grouped all combat-related attributes together (bash/cut/pierce, to-hit/moves, techniques, martial arts compatibility, average melee stats)
• Show container info (volume, watertight/sealing attributes) and contents (water or food) together
• Show repair info before disasssembly info
• Show item qualities before disassembly/repair info
• Show conductivity in the same section with other item flags
• Un-bold "Volume", "Capacity" and other out-of-place highlights
• Rephrase and unbold the "Rigid: No" message for non-rigid items
• Move "Material" into same section as Volume/Weight
• Standardize all headings so the : is not emphasized
• Highlight keywords at the start of each section to make scanning easier
• Changed "This piece of clothing" to "This clothing" for consistency
• Group together Owner / Price / Barter value, and move to a less prominent position

Infrastructure:

• Factor out new functions for modularity:
  • item::armor_protection_info
  • item::armor_fit_info
  • item::bionic_info
  • item::repair_info
  • item::contents_info
  • item::combat_info
• Added more test cases

Describe alternatives you've considered

There were some things I planned to try and tackle, but now think may be better addressed in separate PRs:

• Merge sometimes-duplicated item name / damage indicator at the top of the window (ex. for clothing) - I narrowed this down to the top few lines of input_event draw_item_info(...) in src/output.cpp - seems like someone wanted it this way, and I didn't want to break it
• Show item in same color as it would appear in inventory, instead of always grey - same reason as above
• Regroup disparate redundant sections for battery-powered items (ex., hotplate includes "Contents: medium battery (500/500 batteries), This is a medium battery cell . . . / Ammunition: batteries / Charges: 500 / Magazine: medium battery / Compatible magazines: . . .") - I couldn't get my head around the different ways items could contain or be charged or have ammo, and batteries and battery-operated devices seem to qualify for more than one category. This would need an attentive refactoring that I don't want to do.

Testing

Rerunning tests/cata_test [iteminfo] after incremental changes; adding tests as I went along; frequent playtesting

Additional context

Screenshots below compare items before (left) and after (right) the changes applied in this PR.

Halligan bar:

Compound bow:

[anothersimulacrum]

You've got a bit of a messy history here, just FYI.

[Kilvoctu]

Compiled these changes and compared with a number of different items, and what you've done seems fairly sensible so far. Subtle. Nothing glaringly out of place, though I still disagree with the item description being at the top.

[wapcaplet]

You've got a bit of a messy history here, just FYI.

@anothersimulacrum Yeah, it went all wonky after I merged from upstream master, and now shows the full commit history of my iteminfo-tests branch. If you know of some way I can clean it up, please share.

Compiled these changes and compared with a number of different items, and what you've done seems fairly sensible so far. Subtle. Nothing glaringly out of place, though I still disagree with the item description being at the top.

@Kilvoctu Thanks for testing - yes, I am doing my best not to change anything too radically, since I know players have gotten used to the placement of things. What kind of info would you put before the item description?

[anothersimulacrum]

I'll bet the problem is that you're not wiping your branch between each PR - this isn't a wrong thing to do, and doesn't necessarily lead to this problem - but because most PRs have their commits squashed, git can't just fast-forward when you try to merge with upstream (you can see it's not fast-forwarding because there are merge commits), because you've got different commits.

So, a few ways to keep this from happening:

• Keep a clean history, have good commits, so your PRs aren't squashed and git can cleanly fast-forward
• Delete and remake your branch between each PR
• Have a different branch for each PR

To clean this up:

• Rebase and drop/squash any commits that aren't making any changes
• Cherry pick the commits making changes to a new branch, and hard reset this branch to that one

If you need any further explanation or help, I'm pretty available on the discord. I can also help out here, though to a bit of a lesser extent.

[Night-Pryanik]

That's the work I always wanted to do. Good job.

Suggestion: move "Techniques when wielded" section closer to or even merge it with the "Weapon stats" section.

[Kilvoctu]

What kind of info would you put before the item description?

Descriptions are great for flavor and I'm glad they exist, but I feel that the player should be able to immediately see essential item info without expending keypresses to scroll past (sometimes very long) descriptions. Furthermore, considering as descriptions vary wildly in length, this also causes unnecessary eye movement as the player cannot be sure of when item stats begin.

On my build, I think I put it right above the disassembly information, which might be further than down than what's needed. I know it'd be a shame to put descriptions at the bottom.

Reiterating, the description should be below information that a player will reasonably want to see at a glance frequently, so to maximize consistency of the initial item info presentation, I'd think a decent place to put the description is right below the basic info: category, price, volume, etc.

Currently you have this information in this section. I think every item has a material, correct me if I'm wrong. This would leave barter value as the inconsistent factor. I would probably try to get price and barter on the same line. This way, every item should consistently start with the same four lines of information.

[8street]

@wapcaplet Can you make the item name, at the very top of window, have a color like in the inventory? You must use the function nc_color item::color_in_inventory() or something like that.

[Hirmuolio]

Brighter text for keywords at the start of the sections would maek quick scanning faster. White for Disassembling and repair.

[wapcaplet]

Thanks everyone for the feedback! These are all good suggestions - please keep them coming. I've edited the "planned" list in the description to include them. I believe I have the commit history sorted now thanks to some help from the Discord chat, and force-pushed a cleaner branch.

[wapcaplet]

Final tweaks to the top few lines:

With that, I am ready to be done with this PR, unless there are any further concerns.

[wapcaplet]

Travis CI has been giving a persistent error with tests/iteminfo_tests.cpp in the iteminfo_query constructor on MacOS:

A single-line static cast seems to get around it, but that's an ugly thing to fill the entire test with, so I added (and did my best to explain) a wrapper function to (hopefully) work around it: bbc8909

[ZhilkinSerg]

I like most of the changes, but I agree with @Night-Pryanik that item description looks more natural when displayed right after item name. I would like to keep item name and description together.

Maybe we can make ordering of various item info blocks configurable (in current or some other PR)?

I also suggest highlighting item name with white color:

[AMurkin]

What if we display at the top the first lines of description and ellipsis for long descriptions? And a full description at the bottom.
Short descriptions we show at the top under the item name.

[wapcaplet]

What if we display at the top the first lines of description and ellipsis for long descriptions? And a full description at the bottom.
Short descriptions we show at the top under the item name.

@AMurkin I don't think splitting the description into two pieces is a good idea, especially if it could happen arbitrarily (leaving, say, two words moved to page 3). A proper solution would need to be intelligent about splitting on sentence boundaries, and it could be difficult to find the other half of the description without a distinctive footnote marker or something. I'm not opposed to the idea, but I don't have any interest in doing all that stuff for this PR.

I also suggest highlighting item name with white color:

@ZhilkinSerg I did consider this, but it's complicated by the fact that clothing, favorited items, (and possibly others) have their names displayed twice at the top of this window. In these cases, the second one would get the white highlight, which could be weird.

Cataclysm-DDA/src/output.cpp

Lines 847 to 852 in 05940bf

	if( !data.get_item_name().empty() ) {
	buffer += data.get_item_name() + "\n";
	}
	if( data.get_item_name() != data.get_type_name() && !data.get_type_name().empty() ) {
	buffer += data.get_type_name() + "\n";
	}

I didn't try to tackle this because I don't know whether get_item_name or get_type_name should get precedence here.

[Kilvoctu]

I think @AMurkin meant in their proposal that the description effectively would be in two places. At the top, the description would get cut off if it exceeded one line. Then at the bottom, the description would be displayed again in full.

[ghost]

the idea in this PR is very nice.
item info UI have been in serious need of presentation reordering for so long.

if i may add an idea that is outside the scope of this PR:
idealy "item info" and other UI elements could become "panels" instances,
and like other panels: allow theme (narrow, label, classic, ..)
as some people will want labels, some won't and etc..
it would also helps toward making UI general coherency.

anyway in my opinion, it's already quite good first step in right direction.
it makes current item info ui much more readable.

[ZhilkinSerg]

Can you resolve conflicts please?

[wapcaplet]

Conflicts are resolved; Travis CI is failing on Manatouched mutation tests, but I can't reproduce it locally.

[vichvich4444]

Item description allows you to instantly understand item value. Since most of the CDDA items are from real life, player doesn't need to dive into numbers and stats. Item description should be on top after name.
I like this PR but it still doesn't solve biggest issue with item looting witch is a lot of eye moving from game screen to top-left corner. I'd suggest showing everything in the middle of screen, right above item position. Yes this means item info window will obscure other things, but in my opinion it's worth it.