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

More vehicle part descriptions #24462

Merged
merged 10 commits into from Jul 26, 2018

Conversation

3 participants
@mlangsdorf
Copy link
Contributor

commented Jul 24, 2018

Expand some vehicle part descriptions based on feedback from reddit and discourse.

Refactor the code that prints the vehicle part description so it can be used it more places than just the install menu.

Print the descriptions for all vparts on the current tile in the general vehicle interaction display, using the window panel that was otherwise blank

mlangsdorf added some commits Jul 24, 2018

vehicle descriptions: expanded descriptions
Feedback from reddit and discourse is that the vehicle descriptions are useful,
but could use some more details on interactions. Add the default keystrokes to
use controls, the interactions between seats and seatbelts, and the commands
to use most of the crafting rigs and engineering tools to the descriptions.
vehicle part descriptions: make a format_description function
vehicle part descriptions might be useful in places other than the
install menu, so move the code that formats the description into its
own function.
vehicle part descriptions: display descriptions in overview
Display vehicle part descriptions in the overview, so players don't have
to install a part to see what it does. Instead, players can see the
descriptions of installed parts.

Incidentally, rename print_part_desc() to print_part_list() to minimize
confusion with the new print_vparts_desc().
@mlangsdorf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2018

overview_vehicle_parts
The new vehicle parts display, showing the expanded description for the controls vehicle part, among others.

@ZhilkinSerg ZhilkinSerg self-assigned this Jul 24, 2018

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2018

Minor issues (can be addressed in later PRs):

  • pressing >, <, TAB or ENTER clears window with parts descriptions;
  • there are no keybindings to scroll window with part descriptions.
@mlangsdorf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2018

I can add a commit to fix the minor issues.

I though fold_and_print would handle the scrolling automatically, but if it doesn't, I can copy the logic from print_parts_list.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2018

I though fold_and_print would handle the scrolling automatically, but if it doesn't, I can copy the logic from print_parts_list.

Maybe it does, but there is no keybinding for it and description for more than 3 parts are garbled.

image

image

@mlangsdorf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2018

Right, I agree there is a problem. I was just explaining why I didn't code it better - because I thought I had. Ah, well.
Working on a fix, and poking at it more already let me fix the x for y swap that was making the word wrapping so cramped.

@mlangsdorf mlangsdorf changed the title More vehicle part descriptions [WIP] More vehicle part descriptions Jul 24, 2018

vehicle interact: rename/rebind fuel list scroll keys
Now that vehicle part descriptions take up space in the w_msg window,
that window needs to be able to scroll. Remove the > and < keys from
the keybinds for the vehicle fuel list by creating two new keybinds.

Astyle vehicle_interact:do_main_loop() in the process.
@mlangsdorf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2018

That was fun.

The descriptions list should be truncated to the window size now, with a message indicating the scroll keys. Hitting TAB, BACKTAB, <, and >, should all do reasonably sensible things and the vehicle
parts description list won't disappear when you're scrolling through the fuel list.

vehicle part descriptions: handle overflow and scrolling
change vpart_info::format_description to count the number of lines
printed and return that value.

change vehicle:print_vparts_desc to count the number of lines it is
printing versus the window size, and to add scroll messages if the
there are more lines of description than available space. Add an offset,
so the descriptions don't always start printing at the frame part.

adjust veh_interact::do_main_loop to add the keybinds for scrolling
through the descriptions.

incidentally, in veh_interact::move_cursor, correctly pass the size of
w_msg so descriptions get line wrapped to the width of the window and not
the height.

@mlangsdorf mlangsdorf force-pushed the mlangsdorf:more_veh_descriptions branch to 96fff3d Jul 24, 2018

@mlangsdorf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2018

Okay, this commit has grown to ~950 lines. I can split off the data/json/vehicle_parts changes if people would like, but the src changes are fairly tightly coupled.

@@ -28,7 +28,7 @@
"broken_color": "light_blue",
"difficulty": 2,
"durability": 100,
"description": "An extendable cantilever crane. Can be used to jack up another vehicle to change the other vehicle's wheels, or lift a heavy component like an engine from another vehicle.",
"description": "An extendable cantilever crane. If it is in your line of sight and within two tiles of another vehicle, you will automatically use if when you want jack up the other vehicle to change the its wheels, or lift a heavy component like an engine that you are installing in or removing from the other vehicle.",

This comment has been minimized.

Copy link
@AMurkin

AMurkin Jul 24, 2018

Contributor

Too many spaces here and in two next changes.

This comment has been minimized.

Copy link
@mlangsdorf

mlangsdorf Jul 24, 2018

Author Contributor

Thanks.
Fixed with the 6th commit.

@mlangsdorf mlangsdorf changed the title [WIP] More vehicle part descriptions More vehicle part descriptions Jul 24, 2018

@@ -11,7 +11,7 @@
"broken_color": "light_blue",
"difficulty": 5,
"durability": 200,
"description": "A rigid steel boom crane. Can be used to jack up another vehicle to change the other vehicle's wheels, or lift a heavy component like an engine from another vehicle.",
"description": "A rigid steel boom crane. If it is in your line of sight and within two tiles of another vehicle, you will automatically use if when you want jack up the other vehicle to change the its wheels, or lift a heavy component like an engine that you are installing in or removing from the other vehicle.",

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Jul 25, 2018

Contributor

Some typos:

you will automatically use if when you want jack up the other vehicle to change the its

should probably be:

you will automatically use it when you want to jack up other vehicle to change its

@@ -28,7 +28,7 @@
"broken_color": "light_blue",
"difficulty": 2,
"durability": 100,
"description": "An extendable cantilever crane. Can be used to jack up another vehicle to change the other vehicle's wheels, or lift a heavy component like an engine from another vehicle.",
"description": "An extendable cantilever crane. If it is in your line of sight and within two tiles of another vehicle, you will automatically use if when you want jack up the other vehicle to change the its wheels, or lift a heavy component like an engine that you are installing in or removing from the other vehicle.",

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Jul 25, 2018

Contributor

Similar typos here.

@@ -46,7 +46,7 @@
"broken_color": "light_blue",
"difficulty": 2,
"durability": 100,
"description": "A small pallet lifter. Can be used to jack up another vehicle to change the other vehicle's wheels, or lift a heavy component like an engine from another vehicle.",
"description": "A small pallet lifter. If it is in your line of sight and within two tiles of another vehicle, you will automatically use if when you want jack up the other vehicle to change the its wheels, or lift a heavy component like an engine that you are installing in or removing from the other vehicle.",

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Jul 25, 2018

Contributor

Similar typos here.

@@ -65,7 +65,7 @@
"difficulty": 2,
"damage_modifier": 200,
"durability": 300,
"description": "A pair of forklift arms. Can be used to jack up another vehicle to change the other vehicle's wheels, or lift a heavy component like an engine from another vehicle.",
"description": "A pair of forklift arms. If it is in your line of sight and within two tiles of another vehicle, you will automatically use if when you want jack up the other vehicle to change the its wheels, or lift a heavy component like an engine that you are installing in or removing from the other vehicle.",

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Jul 25, 2018

Contributor

Similar typos here.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2018

The descriptions list should be truncated to the window size now, with a message indicating the scroll keys. Hitting TAB, BACKTAB, <, and >, should all do reasonably sensible things and the vehicle
parts description list won't disappear when you're scrolling through the fuel list.

Scrolling vehicle part description window with > does not have upper limit, so you can scroll through empty pages indefinitely - only < More parts here.. header will be shown:

image

Also ENTER (Confirm Choice) still clears vehicle part description window.

Okay, this commit has grown to ~950 lines. I can split off the data/json/vehicle_parts changes if people would like, but the src changes are fairly tightly coupled.

No need to split yet.

@ZhilkinSerg ZhilkinSerg added the [C++] label Jul 25, 2018

mlangsdorf added some commits Jul 25, 2018

vehicle part descriptions: more scrolling fixes, always display if po…
…ssible

limit the maximum amount of scroll based on the size of the w_msg, prevent
accumulation of scroll passed the maximum or minimum amount, and always
display the vehicle part descriptions if the w_msg isn't being used to
display something else.
@mlangsdorf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2018

Fixed the scroll-off issue. It's approximate, but seemed adequate in my testing.
Fixed the no-display on RETURN issue - now vehicle part descriptions pop up whenever the w_msg isn't being used for something more important.

Fixed the typos in the JSON - thanks for the review.

@ZhilkinSerg ZhilkinSerg self-assigned this Jul 26, 2018

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2018

Fixed the scroll-off issue. It's approximate, but seemed adequate in my testing.

The bug is still on, but only for tiles with little vehicle parts (when all descriptions go to first page).

Here is first page - it does not contain More parts here... > label:

image

But I can still press > and proceed to empty pages:

image

And if I go to tile with multiple parts (which descriptions do not fit on one page):

image

I can only proceed to some ultimate page with description of last part:

image

@ZhilkinSerg ZhilkinSerg removed their assignment Jul 26, 2018

@mlangsdorf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 26, 2018

More fun!
Okay, you can scroll down until the remaining descriptions fit in the display and no further. You can scroll up to the first part. Moving resets the scroll limits.
I think this works correctly.

vehicle part descriptions: properly handle the scroll limit
limit scrolling such that you can't scroll if all the descriptions
fit it the window, and you can't scroll down past the point that
the remaining descriptions fit in the window, and you can't scroll
up past the first description.

Method hopefully documented adequately in the function, because I
may need to reuse it in the future.

@mlangsdorf mlangsdorf force-pushed the mlangsdorf:more_veh_descriptions branch to f1abdff Jul 26, 2018

@ZhilkinSerg ZhilkinSerg self-assigned this Jul 26, 2018

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2018

Looks good now!

@ZhilkinSerg ZhilkinSerg merged commit 4f79bd6 into CleverRaven:master Jul 26, 2018

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage decreased (-0.06%) to 22.929%
Details
gorgon-ghprb Build finished.
Details

@ZhilkinSerg ZhilkinSerg removed their assignment Jul 26, 2018

@mlangsdorf mlangsdorf deleted the mlangsdorf:more_veh_descriptions branch Jul 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.