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

Travel to: fix travel to on ramps #74762

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

Brambor
Copy link
Contributor

@Brambor Brambor commented Jun 24, 2024

Summary

Bugfixes "Travel to no longer cancels on moving up/down on a ramp"

Purpose of change

Travel to doesn't work with ramps:

Cataclysm_.Dark.Days.Ahead.-.0.G-10289-g31fb5cbf61-dirty.2024-06-24.19-21-45.mp4

Commanding the character to Travel to.

Note the bottom right: The moment you step down it cancels.

Describe the solution

  1. Change get_movement_action_from_delta (called only in get_next_auto_move_direction):
    • Consider xy before z, if xy is valid and non-zero, ignore z.
    • If none of the arguments are valid, return ACTION_NULL instead of ACTION_MOVE_FORTH_LEFT.
  2. Change get_next_auto_move_direction:
    • Allow the character to be 1 tile off of the expected direction.
    • The off by 1 in z coordinate is needed for moving over ramps.
    • The off by 1 in x and y is likely not going to be used, but if the character wants to move east and is, for some reason, pushed to the south, they can move northeast and be back the path.
    • Every cycle removes one tile from the route, so if the character wants to move, but fails, it now takes two cycles instead of one to detect this. However, it still terminates.
    • This change would allow for the character to be on the wrong floor if something screws up. I am open to suggestions. (See testing)
  • Rename var as second commit, because I don't want to make a separate PR.
  • Rationale: I am renaming ramp_offset to ramp_z_offset as I didn't know what it was until I got to the last line. It also might have been teleporting in x or y direction, not the z direction.

Describe alternatives you've considered

Change the path generation to know about ramps and produce the path accordingly. I prefer this as this is simple and also fixes ramp movement in my upcoming PR:

Testing

Travel to across ramp now works:

Cataclysm_.Dark.Days.Ahead.-.0.G-10289-g9fcb743a1e-dirty.2024-06-24.19-10-11.mp4

Indeed, the character can walk on the wrong floor, if they automove onto a ramp they didn't expect:

Before (better)

Cataclysm_.Dark.Days.Ahead.-.0.G-10289-g31fb5cbf61-dirty.2024-06-24.19-26-26.mp4

After (worse)

Cataclysm_.Dark.Days.Ahead.-.0.G-10289-g31fb5cbf61-dirty.2024-06-24.19-35-53.mp4

However, I believe this PR is a net benefit.

Overmap travel to still says "You cannot reach the destination" when on such a bridge, but that is likely a different problem.

Additional context

Originally found here: #74644 (comment)

@Brambor Brambor mentioned this pull request Jun 24, 2024
21 tasks
@github-actions github-actions bot added the [C++] Changes (can be) made in C++. Previously named `Code` label Jun 24, 2024
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jun 24, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 24, 2024
@Maleclypse Maleclypse merged commit 303c00f into CleverRaven:master Jun 26, 2024
21 of 28 checks passed
@Brambor Brambor deleted the travel-to-ramps branch July 22, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants