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

refactor map::route to extract (some) cost functions #74373

Merged
merged 10 commits into from
Jun 18, 2024

Conversation

nornagon
Copy link
Contributor

@nornagon nornagon commented Jun 7, 2024

Summary

None

Purpose of change

map::route does a lot, and a lot of it is shared logic that is inconsistent
with other places (e.g. monster::know_danger_at). This is a first step toward
refactoring that shared logic into a common function.

Describe the solution

I extracted two functions from map::route: cost_to_pass and cost_to_avoid.
cost_to_pass is responsible for calculating the difficulty of navigating the
terrain, e.g. by climbing, bashing, or opening doors, or deciding that it's not
navigable. cost_to_avoid is responsible for calculating the danger of
navigating the terrain, e.g. if there's a trap present. The reason there are
two functions instead of one is that they are additive: some terrain might be
difficult to pass and have a trap present. Splitting them simplifies the
logic in each.

In future I'd like to add logic for avoiding dangerous fields into
cost_to_avoid, but I've kept the logic unchanged for this refactor to make
things easier to review.

Describe alternatives you've considered

Testing

Tested player and NPC pathing around caltrops and ledges. Both seem to work
correctly, avoiding when possible but walking through them when necessary.

That's definitely not an exhaustive test, but it's indicative that things
aren't totally broken!

Additional context

@github-actions github-actions bot added Vehicles Vehicles, parts, mechanics & interactions [C++] Changes (can be) made in C++. Previously named `Code` labels Jun 7, 2024
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Jun 7, 2024
@NetSysFire NetSysFire added the Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style label Jun 7, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 8, 2024
@RenechCDDA
Copy link
Member

Tested player and NPC pathing around caltrops and ledges.

Can I bring this to your attention? #74200 (comment)

As in, "some pathfinding may currently be broken". You can see that in the example video the omniscient player pathing still manages to run into a trap, somehow. (Also the beartrap does not stop them, which is in itself a problem).

I am not saying this is in scope for your changes. I am just trying to provide some information that may help your decision making.

@nornagon
Copy link
Contributor Author

nornagon commented Jun 8, 2024

Hm, I wasn't able to reproduce that in my testing. Would you be able to provide a save so I can make sure I'm testing exactly the same thing?

@RenechCDDA
Copy link
Member

Hm, I wasn't able to reproduce that in my testing. Would you be able to provide a save so I can make sure I'm testing exactly the same thing?

Aphid Murder-trimmed.tar.gz

Due to the various testing shenanigans I was getting up to, the trap will error on load unless you also load the trap definition from #74200's original post. Note that the trap function was replaced with beartrap. Other than that it is identical. I am still able to reproduce it by loading the save and doing the same menu move to that item.

@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 12, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 15, 2024
@Maleclypse Maleclypse merged commit 46913cd into CleverRaven:master Jun 18, 2024
26 checks passed
@CLIDragon CLIDragon mentioned this pull request Aug 25, 2024
10 tasks
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` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style json-styled JSON lint passed, label assigned by github actions Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants