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

get_line() refactor #5452

Merged
merged 4 commits into from
Jan 28, 2024
Merged

Conversation

Doubleumc
Copy link
Contributor

@Doubleumc Doubleumc commented Jan 14, 2024

About the pull request

The codebase currently has three line-drawing algorithms:

  • get_line() - never used
  • getline() - used once
  • getline2() - used many times

They are all almost identical. There is no real use-case for one over the the other.

This PR removes getline() and getline2() and replaces those proc calls with get_line(). Furthermore, get_line() is replaced with a different method using linear interpolation, based on the examples here: https://www.redblobgames.com/grids/line-drawing/#optimization

The pros of this new method are consistent, reversible projectile paths (if you can shoot them they can shoot you), and that it is possibly more performant. Bresenham's line algorithm is famously fast -- for the 1960s. This new one looks to be faster on modern computer hardware. Floats aren't scary anymore.

Explain why it's good for the game

Less duplicated code. Consistent projectile paths.

Testing Photographs and Procedure

Screenshots & Videos

The functions of interest are:

The output of get_line_testA() is not identical to getline2(). A path starting at the center pillar and ending on a tiled floor has different results.
image

The following examples are a comparison between getline2() and get_line_testA(). All paths start at the wall and end at the ghost. Purple tiles are where both functions picked the same turf, red is unique to getline2(), blue is unique to get_line_testA(). Note that get_line_testA() results in the same path taken regardless of direction.

Example 1a: SW to NE
image
Example 1b: NE to SW
image

Example 2a: NW to SE
image
Example 2b: SE to NW
image

Changelog

🆑
refactor: projectile paths are the same in both directions, A->B and B->A
/:cl:

@github-actions github-actions bot added the Refactor Make the code harder to read label Jan 14, 2024
@Birdtalon Birdtalon added the Code Improvement Make the code longer label Jan 14, 2024
@cm13-github cm13-github added the Merge Conflict PR can't be merged because it touched too much code label Jan 15, 2024
@cm13-github
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@cm13-github cm13-github removed the Merge Conflict PR can't be merged because it touched too much code label Jan 15, 2024
@cm13-github
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@Doubleumc
Copy link
Contributor Author

CORRECTION: the path is more consistently reversible that before, but its not always reversible. From about 70% for the original getline2() to about 80% for the new method.

add small constant to each step to avoid small inaccuracies causing .99999 and rounding down
@Doubleumc
Copy link
Contributor Author

Compensated for a rounding inaccuracy, it is now consistently reversible for at least 100+ tiles.

slightly more readable comments and code
Copy link
Contributor

@Birdtalon Birdtalon left a comment

Choose a reason for hiding this comment

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

Looks good and haven't seen any big behaviour changes or problems from TM.

@Birdtalon Birdtalon added this pull request to the merge queue Jan 28, 2024
Merged via the queue into cmss13-devs:master with commit 12f5f35 Jan 28, 2024
26 checks passed
cm13-github added a commit that referenced this pull request Jan 28, 2024
@Doubleumc Doubleumc deleted the line-drawing-refactor branch January 28, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Improvement Make the code longer Refactor Make the code harder to read
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants