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

Extended pickup window #36492

Merged
merged 12 commits into from
Dec 31, 2019
Merged

Extended pickup window #36492

merged 12 commits into from
Dec 31, 2019

Conversation

8street
Copy link
Contributor

@8street 8street commented Dec 27, 2019

Summary

SUMMARY: Interface "Extended pickup window"

Purpose of change

Now it’s very convenient to pick up items. Full and not cropped item names are displayed.

Describe the solution

The width of the pickup window was the fixed width. Now the pickup windows width is variable. The cycle searches for the maximum length of the item name. If item's name exceed the standard pickup window width (44 symbols) then window width equal item's name.
There is also a check for exceeding the lenght of the item's name with the width of the game window.

Describe alternatives you've considered

#36102 but this closed. The current solution really looks better.

Testing

12

@Night-Pryanik
Copy link
Contributor

Could you please show a screenshot of right-aligned pickup window with your changes?

@8street
Copy link
Contributor Author

8street commented Dec 28, 2019

Right aligned
right_aligned

Overlapping
overlapping

@8street
Copy link
Contributor Author

8street commented Dec 28, 2019

Travis Clang-tidy CMake build with Tiles and Sound error:

/home/travis/build/CleverRaven/Cataclysm-DDA/src/pickup.cpp:571:9: error: use range-based for loop instead [modernize-loop-convert,-warnings-as-errors]
        for( size_t cur_it = 0; cur_it < stacked_here.size(); cur_it++ ) {
        ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           (auto & cur_it : stacked_here)

It's my changes but I don't know what it means. Does anyone know how to fix this?

The code also nearby has a similar cycle, it works.
for( size_t index = 0; index < stacked_here.size(); index++ ) {

src/pickup.cpp Outdated
@@ -567,6 +567,15 @@ void Pickup::pick_up( const tripoint &p, int min, from_where get_items_from )
int pickupH = maxitems + pickupBorderRows;
int pickupW = 44;

//find max length of item name and resize pickup window width
for( size_t cur_it = 0; cur_it < stacked_here.size(); cur_it++ ) {
const item &this_item = *stacked_here[cur_it].front();
Copy link
Contributor

@Qrox Qrox Dec 28, 2019

Choose a reason for hiding this comment

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

cur_it in your code is only used to access the elements of stacked_here, so it can be simplified to a range-based for loop. You only need to change

        for( size_t cur_it = 0; cur_it < stacked_here.size(); cur_it++ ) {
            const item &this_item = *stacked_here[cur_it].front();

to

        for( const std::list<item_stack::iterator> &cur_list : stacked_here ) {
            const item &this_item = *cur_list.front();

@8street
Copy link
Contributor Author

8street commented Dec 28, 2019

Thanks. This solution seems to work. I mean, it works on my local pc. Although, I can’t check it there. Someone has broken the code style and the necessary Travis build does not start.

@curstwist curstwist added [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. labels Dec 28, 2019
Update the forked repo and the Travis build start to check code
@8street
Copy link
Contributor Author

8street commented Dec 30, 2019

Tests passed. Pull request can be merged.

@kevingranade kevingranade merged commit b693d93 into CleverRaven:master Dec 31, 2019
@8street 8street deleted the Extended_pickup_window branch December 31, 2019 10:18
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` Info / User Interface Game - player communication, menus, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants