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

Improve NPC size > 1 pathing #658

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ipkpjersi
Copy link
Contributor

@ipkpjersi ipkpjersi commented Sep 21, 2024

Wow, this was a ton of effort to clean up this PR but I think I did it properly. I tried many things, but ultimately I had to interactive rebase and just drop the commits I didn't want that were from other branches. My old PR I closed was 7 files changed and this new PR is 7 files changed, so it's extremely likely I did clean up this branch properly.

Anyway, onto this feature:

This is probably fine to merge. It doesn't fix all pathing bugs that currently exist (it doesn't fix player clipping when moving/attacking/clicking on objects etc), that would likely be a ton of work, but this does address not handling NPC size > 1 pathing, and it doesn't seem to introduce any new bugs.

With that said, I'll go ahead and mark this as draft for now so we can make sure it gets tested more first since I don't really consider it completely fully tested. Marked as ready, we can review it/merge it whenever.

@ipkpjersi ipkpjersi changed the title Npc size pathing Npc size > 1 pathing Sep 21, 2024
@ipkpjersi ipkpjersi changed the title Npc size > 1 pathing Improve NPC size > 1 pathing Sep 21, 2024
@ipkpjersi
Copy link
Contributor Author

ipkpjersi commented Sep 21, 2024

Tested this after cleaning the branch, it seems to test well, it looks like I did clean up the branch properly and it is still showing 7 files changed.

This could probably use further testing but I also think it's still technically an improvement over the pathfinding we have on live, and it does actually fix instances of not being able to attack NPCs - I haven't run into that issue once on this branch (torag, cows on certain tiles, etc). Still, further testing is likely a good idea.

@ipkpjersi
Copy link
Contributor Author

Note: I should probably comment out the print messages (but leave them in the codebase for helpful debugging)

@ipkpjersi
Copy link
Contributor Author

This is ready for review/testing now.

@ipkpjersi
Copy link
Contributor Author

Resolved conflicts with my refactoring PR, hopefully no more conflicts in the future with this one lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant