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.

Sign up for GitHub

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 two travel-related overmap keybinds for a better UX #64174

Merged
merged 2 commits into from
Mar 17, 2023
Merged

Add two travel-related overmap keybinds for a better UX #64174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5518c4
Add two travel-related overmap keybinds for a better UX
strategineer Mar 12, 2023
2180185
Use the keyboard_any input_method instead of keyboard_char and keyboa…
strategineer Mar 12, 2023
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
12 changes: 12 additions & 0 deletions data/raw/keybindings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,18 @@
"name": "Choose destination",
"bindings": [ { "input_method": "keyboard_char", "key": "W" }, { "input_method": "keyboard_code", "key": "w", "mod": [ "shift" ] } ]
},
{
"type": "keybinding",
"id": "GO_TO_DESTINATION",
"name": "Go to destination",
"bindings": [ { "input_method": "keyboard_any", "key": "g" } ]
},
{
"type": "keybinding",
"id": "CENTER_ON_DESTINATION",
"name": "Center on destination",
"bindings": [ { "input_method": "keyboard_any", "key": "c" } ]
},
{
"type": "keybinding",
"id": "FIRE",
Expand Down
76 changes: 57 additions & 19 deletions src/overmap_ui.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,8 @@ static void draw_om_sidebar(
print_hint( "LEVEL_UP" );
print_hint( "LEVEL_DOWN" );
print_hint( "CENTER" );
print_hint( "CENTER_ON_DESTINATION" );
print_hint( "GO_TO_DESTINATION" );
print_hint( "SEARCH" );
print_hint( "CREATE_NOTE" );
print_hint( "DELETE_NOTE" );
Expand Down Expand Up @@ -1695,6 +1697,44 @@ static std::vector<tripoint_abs_omt> get_overmap_path_to( const tripoint_abs_omt

static int overmap_zoom_level = DEFAULT_TILESET_ZOOM;

static std::string try_travel_to_destination( avatar &player_character, const tripoint_abs_omt curs,
const bool driving )
{
std::vector<tripoint_abs_omt> path = get_overmap_path_to( curs, driving );
bool same_path_selected = false;
if( path == player_character.omt_path ) {
same_path_selected = true;
}
std::string confirm_msg;
if( !driving && player_character.weight_carried() > player_character.weight_capacity() ) {
confirm_msg = _( "You are overburdened, are you sure you want to travel (it may be painful)?" );
} else if( !driving && player_character.in_vehicle ) {
confirm_msg = _( "You are in a vehicle but not driving. Are you sure you want to walk?" );
} else if( driving ) {
if( same_path_selected ) {
confirm_msg = _( "Drive to this point?" );
} else {
confirm_msg = _( "Drive to your destination?" );
}
} else {
if( same_path_selected ) {
confirm_msg = _( "Travel to this point?" );
} else {
confirm_msg = _( "Travel to your destination?" );
}
}
if( query_yn( confirm_msg ) ) {
if( driving ) {
player_character.assign_activity( player_activity( autodrive_activity_actor() ) );
} else {
player_character.reset_move_mode();
player_character.assign_activity( ACT_TRAVELLING );
}
return "QUIT";
}
return nullptr;
Copy link
Member

Choose a reason for hiding this comment

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

Why return a nullptr here?

Copy link
Contributor Author

@strategineer strategineer Mar 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No particular reason (unfortunately). Looking back at the code I think it would be cleaner to return true/false in this function and set the action variable to "QUIT" conditionally from the calling code. Although we've still got a function that returns a value and has a side-effect which isn't ideal... Let me type up a quick PR before I forget to refactor this.

EDIT: #64353

Copy link
Member

@BrettDong BrettDong Mar 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I meant constructing a std::string from nullptr is going to crash the program here.

Edit: #64355

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh interesting, it wasn't crashing on my end when i tested it. But I'm not surprised that it could. Thanks for keeping an eye out!

Copy link
Member

Choose a reason for hiding this comment

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

If you press N to the prompt you'll reach here and access nullptr memory.

}

static tripoint_abs_omt display( const tripoint_abs_omt &orig,
const draw_data_t &data = draw_data_t() )
{
Expand Down Expand Up @@ -1748,6 +1788,9 @@ static tripoint_abs_omt display( const tripoint_abs_omt &orig,
ictxt.register_action( "MOUSE_MOVE" );
ictxt.register_action( "SELECT" );
ictxt.register_action( "CHOOSE_DESTINATION" );
ictxt.register_action( "CENTER_ON_DESTINATION" );
ictxt.register_action( "GO_TO_DESTINATION" );


// Actions whose keys we want to display.
ictxt.register_action( "CENTER" );
Expand Down Expand Up @@ -1840,6 +1883,19 @@ static tripoint_abs_omt display( const tripoint_abs_omt &orig,
curs.x() = p.x();
curs.y() = p.y();
}
} else if( action == "GO_TO_DESTINATION" ) {
avatar &player_character = get_avatar();
if( !player_character.omt_path.empty() ) {
const bool driving = player_character.in_vehicle && player_character.controlling_vehicle;
action = try_travel_to_destination( player_character, curs, driving );
}
} else if( action == "CENTER_ON_DESTINATION" ) {
avatar &player_character = get_avatar();
if( !player_character.omt_path.empty() ) {
tripoint_abs_omt p = player_character.omt_path[0];
curs.x() = p.x();
curs.y() = p.y();
}
} else if( action == "CHOOSE_DESTINATION" ) {
avatar &player_character = get_avatar();
const bool driving = player_character.in_vehicle && player_character.controlling_vehicle;
Expand All @@ -1851,25 +1907,7 @@ static tripoint_abs_omt display( const tripoint_abs_omt &orig,
player_character.omt_path.swap( path );
}
if( same_path_selected && !player_character.omt_path.empty() ) {
std::string confirm_msg;
if( !driving && player_character.weight_carried() > player_character.weight_capacity() ) {
confirm_msg = _( "You are overburdened, are you sure you want to travel (it may be painful)?" );
} else if( !driving && player_character.in_vehicle ) {
confirm_msg = _( "You are in a vehicle but not driving. Are you sure you want to walk?" );
} else if( driving ) {
confirm_msg = _( "Drive to this point?" );
} else {
confirm_msg = _( "Travel to this point?" );
}
if( query_yn( confirm_msg ) ) {
if( driving ) {
player_character.assign_activity( player_activity( autodrive_activity_actor() ) );
} else {
player_character.reset_move_mode();
player_character.assign_activity( ACT_TRAVELLING );
}
action = "QUIT";
}
action = try_travel_to_destination( player_character, curs, driving );
}
} else if( action == "TOGGLE_BLINKING" ) {
uistate.overmap_blinking = !uistate.overmap_blinking;
Expand Down