azerothcore / azerothcore-wotlk

Complete Open Source and Modular solution for MMO
http://www.azerothcore.org
GNU Affero General Public License v3.0
6.55k stars 2.63k forks source link

path finder bug mmaps #7072

Open Krejza9 opened 3 years ago

Krejza9 commented 3 years ago

Current Behaviour

The current path generator for creatures does not handle very well small objects such as gates, fences, boxes, etc. If an creatures is following you , it always goes through these objects, it never goes around them video from AC https://youtu.be/bnAXVFIwedg

Expected Blizzlike Behaviour

Creatures always goes around the objects, see video https://youtu.be/UE4-gAZKUPI

Source

No response

Steps to reproduce the problem

  1. find small objects such as gates, fences, boxes, etc.
  2. aggro creature
  3. see result

Extra Notes

No response

AC rev. hash/commit

5368da64e08e780d2cd28b1d58e06b726411fcc4

Operating system

Ubuntu x64

Custom changes or Modules

No response

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/100042989-path-finder-bug-mmaps?utm_campaign=plugin&utm_content=tracker%2F40032087&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F40032087&utm_medium=issues&utm_source=github).
syssneck commented 3 years ago

Yes, they really don't care about fences and stuff^^ This was about a week ago on Chromie:

https://user-images.githubusercontent.com/68868567/126649326-c32b51aa-6905-41f0-8e57-d657fc8ba115.mp4

syssneck commented 3 years ago

I also noticed, that they get quite confused when I run around obstacles. It seems like their pathing is then not updating correctly to the "shortest possible/unobstructed way to the attacker". Example: Confused Forest Spider

https://user-images.githubusercontent.com/68868567/126650681-aec001d4-e0e5-46e0-80e9-3192a891f928.mp4

T1ti commented 3 years ago

The error seems to be related to the height of the object Those M2 objects, also known as world doodads seem to work correctly when they are big, because trees are also M2 objects. yet NPCs don't try to walk over trees.

There is some faulty code that make the NPCs walk over them based on the size of the object. They are supposed to walk over some of them, like a tree branch on the ground. But the ratio seems to be wrong

Krejza9 commented 3 years ago

I did some basic tests, and the mmaps are fine. I looked at them over the raycast and the fence is there.

image

btw: It's interesting to see how WoW looks from an NPC perspective

FrancescoBorzi commented 3 years ago

btw: It's interesting to see how WoW looks from an NPC perspective

@Krejza9 sidenote: it would be nice to add more guides to our wiki about this kind of things, so more users are able to debug problems by checking that

UltraNix commented 3 years ago

What about that: TrinityCore/TrinityCore#9965 and TrinityCore/TrinityCore#15656? The 1st one is about wolves NOT jumping over fences and 2nd is about wolves jumping over fences. So how it supposed to work?

@Krejza9 Can you test on retail a small unit (murlock, wolf, other small creature) and a big unit (e.g some ancient) against the same small fence?

Krejza9 commented 3 years ago

I will test it and post a video here. btw TC hasn't fixed it till today, the only working solution is on VMangos

UltraNix commented 3 years ago

TC fixed it, but then revert it because of https://github.com/TrinityCore/TrinityCore/issues/9965

Krejza9 commented 3 years ago

So after the test, path finding is used for all NPCs. Example of a spider :) https://youtu.be/jXJydoB06Tc

UltraNix commented 3 years ago

And now find something bigger than that fence and test.

Krejza9 commented 3 years ago

I can't think of anything that's a bit bigger than a fence and at the same time I can jump over it. Even the stones and logs laid on the road are as big as the fence

Krejza9 commented 3 years ago

So I managed to find an object that is a little bit bigger than the fence and I can barely jump on it. Tent: Obviously, they'll bypass him, too.

https://user-images.githubusercontent.com/5585555/128613420-68d9c7e7-54c4-4d3b-ab80-707e405fbdea.mp4

T1ti commented 3 years ago

look at how vmangos does it

Krejza9 commented 3 years ago

@T1ti VMangos uses a completely redesigned mmap and path finding system compared to TC and AC, it would be very difficult to do it according to them.

UltraNix commented 3 years ago

Okay guys, bt what about that problem: https://github.com/TrinityCore/TrinityCore/issues/9965 ?

Krejza9 commented 3 years ago

The problem is that somewhere in the Path Generator there is a value that specifies what NPC can jump over it and what they can't. @Yehonal will know.

UltraNix commented 3 years ago

@Krejza9 MMAPs are static, so there is no distinction between big and small units. It's not doable in our core. https://github.com/TrinityCore/TrinityCore/issues/9965#issuecomment-19168401

Krejza9 commented 3 years ago

I don't think the problem is in the mmaps , but in how to work with them. This is an example of how navmesh generate a path with our mmaps. You can see that it works perfectly. The path is generated correctly by going around it. image

Krejza9 commented 3 years ago

but if I set Max Climb to 2.0, it behaves the same as on AC image

Krejza9 commented 3 years ago

There is a PathGenerator::GetRequiredHeightToClimb class in the code, maybe if I play with it a bit I can get the same result as navmesh. image

Krejza9 commented 3 years ago

I managed to fix the fences. Part where walkableClimb is solved is in the file /src/tools/mmaps_generator/MapBuilder.cpp (after this fix it is necessary to re-generate mmaps)

edit line config.walkableClimb = m_bigBaseUnit ? 3 : 6; to values config.walkableClimb = m_bigBaseUnit ? 2 : 4;

image

btw I admit that after this fix I have absolutely no idea how mmaps work , or what they contain. All the time I lived in the assumption that mmaps contain only mathematical representations of environment and objects and whether an NPC is able to bypass a given object is solved by calculating the path generator directly in AC. But I found out that the path generator in AC is not used for this at all and instead the path generator is used directly in mmaps tools.

UltraNix commented 3 years ago

And I will say again - this brings that problem: https://github.com/TrinityCore/TrinityCore/issues/9965

Krejza9 commented 3 years ago

in my opinion, this is correct behaviour and it is not a bug. They really have to circle the fence. It works the same way on WoW Classic and WoW BC.

Krejza9 commented 3 years ago

I believe there are some places where the fence is jumped and then it would be necessary to find such places and modify the generation just for them. For example, the configuration file for the mmap generator.

Yehonal commented 3 years ago

I've double checked the tests I made times ago on shadowlands: https://www.youtube.com/watch?v=2h3sX-yMowk

Even though it's easy to abuse of this mechanism, apparently the fences should not be jumped

Yehonal commented 3 years ago

As discussed in our DM with @Krejza9 , I think it's worth mentioning that:

  1. there was already a PR that was trying to fix this issue and it has been discontinued also because it caused some issues in some places as far as I remember.

  2. Another thing to check would be: what happens with "giant creatures"? should they be blocked by small fences as well?

Consider that if you adjust that value in mmaps generator, you're basically removing completely the possibility to take that path (unless you use a shortcut that means bypassing the mmaps which is also prone to texture falling)

However, I've started to implement a cost-based mechanism for slopes that instruct the pathfinding algorithm to avoid, as much as possible, paths with high slopes (but if the alternative is worse, then they will climb the slope anyway if the path exist) https://github.com/azerothcore/azerothcore-wotlk/blob/3c24b511f2fb2b23618e9cd0b24d28cb5afa34ad/src/common/Navigation/DetourExtended.cpp

I think that we can apply a custom flag on certain nodes during mmaps generation by informing the pathgeneration that the path includes some "fences" or other m2 objects, then letting the algorithm decide if to use that path based on the creature type/size.

UltraNix commented 3 years ago

My opinion is just to leave as it is. Bringing back the old code might cause some problems with mob's pathing and abusing it. Let's @FrancescoBorzi decide what to do with it.

UltraNix commented 3 years ago

@FrancescoBorzi

Yehonal commented 3 years ago

@UltraNix The old code should not be restored if was not a proper fix.

However, a proper PR to fix the fences is welcomed

Meltie2013 commented 3 years ago

https://github.com/azerothcore/azerothcore-wotlk/blob/bcd1a701ac0305806bc6021317b6274ef581ba77/src/tools/mmaps_generator/MapBuilder.cpp#L591-L592 These values can be changed to this if you want to fix the passing over fences.

MapBuilder.cpp

config.walkableHeight = ceilf(agentHeight / config.ch);
config.walkableClimb = floorf(agentMaxClimbModelTerrainTransition / config.ch);

MapBuilder.h

float agentHeight = 1.5f;
float const agentMaxClimbModelTerrainTransition = 1.2f;
float const agentMaxClimbTerrain = 1.8f;

This should be the end result for the fences. I've had this fixed on my project for awhile, just here to pass on a suggestion. image

UltraNix commented 3 years ago

A proper fix would be a dynamic MMaps to handle different object sizes.

FrancescoBorzi commented 3 years ago

@FrancescoBorzi

sorry for the late reply, @Yehonal has been following this kind of issues. I can't tell you more than him

sschepens commented 2 years ago

@Krejza9 how do you get to visualize the vmaps/mmaps like you posted here?

Krejza9 commented 2 years ago

It is a tool (https://github.com/recastnavigation/recastnavigation) , then you must compile it and run MoveMapGen.exe with --debug option (it create debugging files for use with RecastDemo)

Yehonal commented 2 years ago

It is a tool (https://github.com/recastnavigation/recastnavigation) , then you must compile it and run MoveMapGen.exe with --debug option (it create debugging files for use with RecastDemo)

Are you willing to create a guide on how to do it on our AC wiki? eventually, we can also fork that and implement a CI to distribute executables etc.

Yehonal commented 2 years ago

A proper fix would be a dynamic MMaps to handle different object sizes.

I worked on this: https://github.com/azerothcore/azerothcore-wotlk/blob/master/src/common/Navigation/DetourExtended.cpp#L9

I've extended the original Detour class in order to apply a custom node cost calculation for the algorithm based on the slope degree, however, unfortunately, it can't include the size of the creature compared to the size of the slope/obstacle. Hence, it doesn't really work for fences but it optimizes certain paths where creatures prefer a smoother path instead of climbing

Krejza9 commented 2 years ago

@Yehonal In my opinion, there is no point in creating any manuals and I'll tell you why. On Jul 30 I created a manual, do you think someone revised it or wrote something about it. It's been on the air since that date and nobody's paying attention.

Yehonal commented 2 years ago

@Yehonal In my opinion, there is no point in creating any manuals and I'll tell you why. On Jul 30 I created a manual, do you think someone revised it or wrote something about it. It's been on the air since that date and nobody's paying attention.

Mmm, it's not so simple to "review" wiki pages because you also need to "test" the guide somehow. And most of the people here are focused on developing. This guide that I'm requesting from you instead will be surely tested because we need it.

So my suggestion would be:

  1. Please move the VS code guide under our discussion board (we have a category for that and I also published some guidelines there): https://github.com/azerothcore/azerothcore-wotlk/discussions/categories/guides-tips Eventually, that guide can be transferred under the wiki if valid and vastly used.

  2. The recastnavigation tool is something that is really needed for the development of AC maps, so I guess it should be placed under the wiki. But if you prefer to create a topic in that board above, is fine for me as well. Then we can create a wiki page based on that.

sschepens commented 2 years ago

@Krejza9 one more question, how do you create the .obj file that is needed? Is that made with MoveMapGen.exe and then those files used in RecastDemo?

Krejza9 commented 2 years ago

Is that made with MoveMapGen.exe and then those files used in RecastDemo?

Exactly

w5860363 commented 1 year ago

https://github.com/TrinityCore/TrinityCore/commit/93a68a66c3009ea86569076ea05a39ce87dfb78d This is related to the previous TC modification, which was inherited in AZ's code