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

refresh look_around appearance #36324

Closed
wants to merge 28 commits into from
Closed

refresh look_around appearance #36324

wants to merge 28 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 21, 2019

Summary

SUMMARY: Interface "refactor appearance of data displayed in look_around window"

Purpose of change

implement #36306
i use look_around function a lot, and i wanted it to be a bit more pretty and readable at a quick glance. i also noticed when using look_around on a car, for example, the look_around window will display info on the car parts, like their 'health' and state.
i thought it might be nice to have the same with monsters and npc too.
so that's that. purpose of change: make look_around better.

Describe the solution

reordering data on the window, make it more pretty and more readable at quick glance.

Testing

i played a few hours without issues

Additional context

video file (10mb) : here

look_around checking a monster:
lab1
when there is something special about a tile, like blood splatter, acid pool, or foul air in mi-go outpost for example, it displays on it own lines, it immediately stand out as important.
enemy3
npc:
npc2
light tile:
item2
loaded tile:
items1

src/game.cpp Outdated Show resolved Hide resolved
@Night-Pryanik
Copy link
Contributor

As with our current situation in GUI with preference of only one language (English), I really don't like constructions like "some property plus arbitrary number of spaces after colon". Translating such constructions is really pain in the ass.

src/monster.cpp Outdated Show resolved Hide resolved
src/monster.cpp Outdated Show resolved Hide resolved
src/monster.cpp Outdated Show resolved Hide resolved
src/monster.cpp Outdated Show resolved Hide resolved
src/monster.cpp Outdated Show resolved Hide resolved
src/monster.cpp Outdated Show resolved Hide resolved
src/monster.cpp Outdated Show resolved Hide resolved
src/monster.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
captnblood and others added 10 commits December 21, 2019 18:57
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
src/game.cpp Outdated Show resolved Hide resolved
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
@molkero
Copy link
Contributor

molkero commented Dec 22, 2019

Could you colorize the item list to match the rest of the game? As in read/unread books, ammo etc.
Threat could also be shown as the color of the entity name like in the compass.

@ghost
Copy link
Author

ghost commented Dec 22, 2019

Could you colorize the item list to match the rest of the game? As in read/unread books, ammo etc.
Threat could also be shown as the color of the entity name like in the compass.

i tried to do that yesterday and i failed so far. i think too this is must have, i will continue try to do it,but, if you want to help and make code suggestion this is welcomed.

the line that is concerned is line 5966: here
in the function game::print_items_info(). i need to make use of dynamic variable for color instead of static value 'c_white'.

here is the line:
trim_and_print( w_look, point( column, ++line ), max_width, c_white, it.first );

i have looked at list_item_monster() and advanced inventory but i was not able to import that. my skills are too little yet.

@ghost
Copy link
Author

ghost commented Dec 22, 2019

ah wait, it seem i succeeded. by accident most probably. i commit.

src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/monster.cpp Outdated Show resolved Hide resolved
@ymber ymber added <Enhancement / Feature> New features, or enhancements on existing [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. Quality of Life QoL: degree to which players are comfortable, and able to enjoy CDDA labels Dec 24, 2019
@esotericist
Copy link
Contributor

esotericist commented Dec 24, 2019

While I like the approximate shape of what you're doing here, I worry a little bit about how the verbosity and vertical space usage will impact users with small terminal sizes (most especially mobile).

How usable is this when the user only has, say, 25 rows, and is looking at a tile containing a combination of vehicle or furniture, a creature, and some items on the ground?

@ghost
Copy link
Author

ghost commented Dec 24, 2019

@esotericist
it will looks as usual i suppose,
in term of screen space being taken, i haven't changed the window dimension.

the look_around function also have special case for when there is too much to look at, it says: "there is more .."
tile info is what is at the topmost area so it won't be missed,
the most important datas are already shown in priority at the top.
even on small screens the following should be visible:

place  : house
tile   : floor
decor  : bookcase
move   : impassable
light  : bright
type   : smashable, flat
cover  : 80%

entity : zombie  [ grabbing ]
health : IIIIII
senses : he is aware of your presence
stance : hostile
threat : 2

this is 14 lines.
i compare with existing case, when looking around, without this PR, in current code,
when you look_around on a monster you get sent more than 14lines to be displayed in that window.
so it shouldn't be a problem.
on those small screen, if they can display something as big as the sidebar, and this PR didn't change any window dimensions, and they are already able to display at least 14 lines in look_around window, then all should be fine.

@esotericist
Copy link
Contributor

the most important datas are already shown in priority at the top.

The issue is your most important data is not necessarily everyone's most important data. I (and presumably others have similar priorities) am often more interested in things like the items on a specific tile, and if all of the items that used to be readily visible instead get pushed off of the bottom between the new location format, the new creature format, and the already-extant vehicle information, this is a reduction in the ease-of-access of information.

Speaking for myself, I liked some of the existing information being on the same line because it used less vertical space. I understand adding additional information, but burning a lot of lines on existing information and blank space is not free. related: there's a reason we have multiple sidebar layouts, instead of everyone using "labels narrow": everyone processes information differently, and sprawling vertically is not a guaranteed improvement for everyone. it's not for me.

As a side note: it's my opinion that threat doesn't belong in the always-displayed category (since you can always look at the monster description for it), and I feel "sees you" is best represented elsewhere (with the ! in the monster list, and the graphical indicator in the overlays). i feel it doesn't deserve a line of its own either, here

@esotericist
Copy link
Contributor

To be clear, I wouldn't oppose these changes if they were behind an option of some kind so that the previous behavior (or something much like it) remained, but as this PR currently is, I feel it would be a UX regression in some noteworthy respects.

@molkero
Copy link
Contributor

molkero commented Dec 24, 2019

Apart from anything else, I really want to see that colored item list in game.

src/monster.cpp Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Apr 3, 2020

travis error doesn't seem to have anything to do with my code.
this PR is ready for the merge as far as i'm concerned.

after this gets merged, i think i'll try to have a second pass on it,
see if i can make it so the specificities of a tile gets moved down and have it along with listed objects.
it would look better, and make more sense too that way, since it's what you see on a given tile.
tile2

@ghost ghost mentioned this pull request Apr 3, 2020
@ghost ghost closed this Apr 20, 2020
@ghost ghost deleted the look_around_refresh branch April 20, 2020 22:12
This pull request was closed.
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. Quality of Life QoL: degree to which players are comfortable, and able to enjoy CDDA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants