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

Add the option to display ascii art in item description #37598

Merged
merged 11 commits into from
Feb 13, 2020

Conversation

Fris0uman
Copy link
Contributor

@Fris0uman Fris0uman commented Jan 31, 2020

Summary

SUMMARY: Interface "Add the option to display ascii art in item description"

Purpose of change

Add the option to display ascii art in item description. The option is on by default, can be turned off in settings>options>graphics

Describe the solution

Add one optional string entry "ascii_picture" to items
Display the ascii art at the bottom of the item description
If a line is longer than 42 char throw an error and truncate it

Describe alternatives you've considered

Testing

Using the balloon ascii that is too big to fit:

"ascii_picture": [
      "        ,,,,,,,,,,,,,",
      "    .;;;;;;;;;;;;;;;;;;;,.",
      " .;;;;;;;;;;;;;;;;;;;;;;;;,",
      ".;;;;;;;;;;;;;;;;;;;;;;;;;;;;.",
      "<color_red>;;;;;@;;;;;;;;;;;;;;;;;;;;;;;;</color>' .............",
      ";;;;@@;;;;;;;;;;;;;;;;;;;;;;;;'.................",
      ";;;;@@;;;;;;;;;;;;;;;;;;;;;;;;'...................`",
      ";;;;@;;;;;;;;;;;;;;;@;;;;;;;'.....................",
      " `;;;;;;;;;;;;;;;;;;;@@;;;;;'..................;....",
      "   `;;;;;;;;;;;;;;;;@@;;;;'....................;;...",
      "     `;;;;;;;;;;;;;@;;;;'...;.................;;....",
      "        `;;;;;;;;;;;;'   ...;;...............;.....",
      "           `;;;;;;'        ...;;..................",
      "              ;;              ..;...............",
      "              `                  ............",
      "             `                      ......",
      "             `                         ..",
      "           `                           '",
      "          `                           '",
      "         `                           '",
      "        `                           `",
      "        `                           `,",
      "        `",
      "         `",
      "           `."
    ],

image

From the inventory:
image

Additional context

This would only add the option to put ascii art in the item not the art itself.

@kevingranade
Copy link
Member

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/ascii-art-in-item-description/19462/31

@esotericist
Copy link
Contributor

So in addition to the feedback I already gave you (this should be an array of strings instead of a string with line endings in it)
I would recommend adding something in the doc folder, presumably in one of the json-related files, about how this works, what it's intended for, and useful information on typical line lengths

@Fris0uman Fris0uman marked this pull request as ready for review February 1, 2020 11:57
@Qrox
Copy link
Contributor

Qrox commented Feb 1, 2020

I think you can add a flag to item_info to trim the line in the display code when it's too long. (And perhaps also a new hotkey to show a popup to display the full ascii art).

@ifreund ifreund added <Enhancement / Feature> New features, or enhancements on existing Info / User Interface Game - player communication, menus, etc. Items / Item Actions / Item Qualities Items and how they work and interact labels Feb 1, 2020
@esotericist
Copy link
Contributor

Truncating the lines based on the width of the display is a good idea. Adding a keypress to bring up the full thing is a little more awkward, because you'd have to make sure you get it in all of the relevant contexts (and there's several).

I think get the truncation in, and key bindings can happen in a future PR (if they happen at all).

" `",
" `",
" `."
],
Copy link
Contributor

Choose a reason for hiding this comment

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

While an example is good, I definitely think we want to mention what the typical line widths are in the common interfaces so that artists don't have to guess at whether or not their design will fit.

src/item_factory.cpp Outdated Show resolved Hide resolved
@codemime codemime added the [C++] Changes (can be) made in C++. Previously named `Code` label Feb 2, 2020
src/item.cpp Outdated Show resolved Hide resolved
if( !obj.ascii_picture.empty() ) {
std::vector<std::string> tmp_ascii_pic;
for( std::string line : obj.ascii_picture ) {
if( line.length() > 42 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't really use neither length nor substr here. These functions consider string length in terms of bytes, but the string is actually in UTF-8 (multi-byte encoding) and can also contain tags. Use utf8_width and remove_color_tags to get the actual "printable" length of the string.

42 is a magic number (quite famous, but still magic). Use a named constant instead.

src/item_factory.cpp Outdated Show resolved Hide resolved
src/item_factory.cpp Outdated Show resolved Hide resolved
@ZhilkinSerg ZhilkinSerg merged commit b44cf7b into CleverRaven:master Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` <Enhancement / Feature> New features, or enhancements on existing Info / User Interface Game - player communication, menus, etc. Items / Item Actions / Item Qualities Items and how they work and interact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants