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.

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

Inventory view update - Show color for food in containers, show worst rot status for containers with multiple items #46329

Closed
wants to merge 14 commits into from
Closed
Changes from 8 commits
Commits
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
51 changes: 46 additions & 5 deletions src/item.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4588,6 +4588,8 @@ std::string item::tname( unsigned int quantity, bool with_prefix, unsigned int t

std::string maintext;
std::string contents_suffix_text;
std::string colorprefix;
std::string colorsuffix;

if( is_corpse() || typeId() == itype_blood || item_vars.find( "name" ) != item_vars.end() ) {
maintext = type_name( quantity );
Expand Down Expand Up @@ -4621,16 +4623,55 @@ std::string item::tname( unsigned int quantity, bool with_prefix, unsigned int t
contents_item.charges > 1 )
? contents_item.charges
: 1;

// Color second half of container contents depending on perishability (highest perishability of all contents)
if( contents_item.is_food() ) {
// For comestibles
const item_pocket* const parent_pocket = contained_where(contents_item);
colorprefix = "<" + string_from_color( contents_item.color_in_inventory() ) + ">";
colorprefix.replace( 1, 1, "color" ); // changes <c_cyan> to <color_cyan>
if( contents.get_sealed_summary() == item_contents::sealed_summary::all_sealed && parent_pocket->spoil_multiplier() == 0 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if( contents.get_sealed_summary() == item_contents::sealed_summary::all_sealed && parent_pocket->spoil_multiplier() == 0 ) {
if( parent_pocket->sealed() && parent_pocket->spoil_multiplier() == 0 ) {

sealed_summary checks all of the container's pockets, but in this case only the pocket containing the content item matters.

Copy link
Contributor

Choose a reason for hiding this comment

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

And it seems this only checks for sealed container but not rot progress, whereas the code below in the multiple-item case only checks for rot progress but not sealed container?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The top checks for rot progress by calling contents_item.color_in_inventory(), although sealed will supercede it.
The bottom only currently checks rot progress without checking sealed since multiple items in sealed containers isn't a thing right now(?) (see lower comment, if not I can put it in!)
And yes, I think checking sealed and the parent pocket are redundant, the parent pocket by itself is probably (?) fine, will fix!

colorprefix = "<color_light_blue>";
}
colorsuffix = "</color>";
}

contents_suffix_text = string_format( pgettext( "item name",
//~ [container item name] " > [inner item name]"
" > %1$s" ),
" > %1$s%2$s" ),
/* with_contents=false for nested items to prevent excessively long names */
colorprefix, contents_item.tname( contents_count, true, 0, false )
/* with_contents=false for nested items to prevent excessively long names */
contents_item.tname( contents_count, true, 0, false ) );
) + string_format( "%s", colorsuffix );
Copy link
Contributor

Choose a reason for hiding this comment

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

Please store the color instead of the color prefix/suffix strings and use colorize( text, color ) (from color.h) to add color to the text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! I was not aware of that function, but will use it now!


} else if( !contents.empty() ) {
contents_suffix_text = string_format( npgettext( "item name",

int worstrot = 0;
// If container has multiple food items, take the worst rot state and use that as the color for the "# items" text.
// 0 = No food; 1 = Non-perishable (cyan); 2 = Perishable (light cyan); 3 = Old (yellow); 4 = Rotten (brown)
for( const item *it : contents.all_items_top( item_pocket::pocket_type::CONTAINER ) ) {
if( worstrot < 4 && it->rotten() ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove the check for sealed container here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it currently possible to seal a container with multiple items in such a way that it is no longer rotting, and for the container to have multiple "pockets" in which one is sealed and no longer rotting while other pockets are not sealed and actively rotting?
I removed it due to complexity of handling the above case (and that sealing a container with multiple items in it is not something I think is supported right now (?))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually would be happy to write sealed (non-rotting) support for multiple items if you have a quick way to generate a sealed + spoil modifier = 0 container with multiple items that are perishable inside so I can test it!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can write a unit test that creates one such item and check the name of that item, if you want.

worstrot = 4;
colorprefix = "<color_brown>";
} else if( worstrot < 3 && it->is_going_bad() ) {
worstrot = 3;
colorprefix = "<color_yellow>";
} else if( worstrot < 2 && it->goes_bad() ) {
worstrot = 2;
colorprefix = "<color_light_cyan>";
} else if( worstrot < 1 && it->is_food() ) {
worstrot = 1;
colorprefix = "<color_cyan>";
}
}

colorsuffix = ( worstrot > 0 ) ? "</color>" : "";
contents_suffix_text = string_format( pgettext( "item name",
//~ [container item name] " > [count] item"
" > %1$zd item", " > %1$zd items",
contents.num_item_stacks() ), contents.num_item_stacks() );
" > %1$s" ), colorprefix ) +
string_format( ngettext( "%1$zd item", "%1$zd items", contents.num_item_stacks() ),
contents.num_item_stacks() ) +
string_format( pgettext( "color suffix", "%1$s" ), colorsuffix );
Copy link
Contributor

@Qrox Qrox Dec 27, 2020

Choose a reason for hiding this comment

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

Please store the color instead of the color prefix/suffix strings and use colorize( text, color ) (from color.h) to add color to the text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Oh huh, that's odd, didn't realize this comment was duplicated; addressed it in the latest commit!)

}
}

Expand Down