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

fix(MMAP/core/PathGenerator): Try to fix more water creatures #13705

Merged

Conversation

MikaMauger
Copy link
Contributor

@MikaMauger MikaMauger commented Nov 5, 2022

This reverts commit 66e6d33.

Changes Proposed:

  • Fix a lot of water creatures by reverting aforementioned PR, recast doesn't handle very well underwater terrain, above water terrain and water itself as connected, resulting in creatures using weird path or not be able to compute a path at all
  • Check TrinityCore PR for more information
  • Water creatures should follow you in a more straightforward way.
  • Water creatures should continue to attack you even if you have waterwalk and/or are jumping in water (which was partially fixed by fix(Core/Movement): Water-bound creatures should not evade if their t… #12112, thanks to @UltraNix )

Issues Addressed:

Tests Performed:

  • Before: path is wrong
    WoWScrnShot_110522_112328
  • After: path is ok
    WoWScrnShot_110522_112601
  • Before: can't even find a path
    WoWScrnShot_110522_114143
  • Before: When attacking from range, he gets stuck then evades/resets
    WoWScrnShot_110522_113825
  • After: path is ok
    WoWScrnShot_110522_112713

If you want to see what happens in RecastDemo:
Capture_decran_2022-11-01_233355
Capture_decran_2022-11-01_233408

How to Test the Changes:

  1. .go xyz -9243.936523 -2453.144531 56.821400 and do range attacks on murlocs from beach when they are in water
  2. .go xyz -9250.759766 -1182.086792 67.439095 and do range attacks on murlocs from beach when they are in water
  3. A lot more locations where there is a river or a beach/coast

Known Issues and TODO List:

  • We need to find a way to handle underwater floor correctly, not very useful for 3.3.5, but definitely needed for cataclysm zones like Vashj'ir, which is not the target for AzerothCore anyway
  • This PR doesn't fix the abrupt transitions between movement/attacks of water NPCs / hiccups.

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).

For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.

@Yehonal Yehonal added CORE Related to the core file-cpp Used to trigger the matrix build labels Nov 5, 2022
@MikaMauger
Copy link
Contributor Author

You'll need to regenerate mmaps in order to get the fix.

@acidmanifesto acidmanifesto added Mmaps/Vmaps Issues with maps/vmaps/los Waiting to be Tested Ready to be Reviewed SENSITIVE WIP SENSITIVE Work in Progress, Do not Update, Do not Merge until author approves. labels Nov 5, 2022
@PkllonG
Copy link
Contributor

PkllonG commented Nov 5, 2022

image
It's really much better

@PkllonG
Copy link
Contributor

PkllonG commented Nov 5, 2022

image
They will also follow the players from the "water surface" to the "water“

@acidmanifesto
Copy link
Contributor

i'm going to mention this so there is no rush with people thinking this is some golden unicorn end all.
There is a issue with this.
As current where it stands is water mobs that can walk on land can not be pulled from the water to land.
They end up evading instead.
Maybe we will have less issues then current. or maybe we will have more issues reported over time.
Weigh the Pro's and Con's first and see if this can be adjusted to fix the issue, if not then a internal discussion will be needed in either correcting the pr, taking it as is, or closing it.

@acidmanifesto
Copy link
Contributor

Also in reference to a screen shot earlier. NPC is swimming above the water surface.
image

@Gultask
Copy link
Contributor

Gultask commented Nov 9, 2022

Also in reference to a screen shot earlier. NPC is swimming above the water surface.

I think that's the normal running animation

@Gultask
Copy link
Contributor

Gultask commented Nov 9, 2022

If anyone can. Please try:

  1. Checking Gazh'ranka
    .additem 19975 20
    .additem 19974 4
    .go xyz -11695 -1770 13 309 Teleports to Pagle's Pointe
    Use item, try to pull from the land in one instance, see the whole path taken in the other.

  2. This issue: [Rather be Fishin'] Eels cannot swim upward to attack you #13551

.go xyz -4051.062988 2796.326172 -2 1
.npc add temp 5462
move to land, do ranged attacks against it, see if it evades after a while

.go xyz -94.3 -1116.1 35.1 0
Attack snapjaw.
See no evade

@MikaMauger
Copy link
Contributor Author

MikaMauger commented Nov 9, 2022

  1. Gazh'ranka
  • before:
    1.-.Gazhranka.Before.mp4
  • after:
    1.-.Gazhranka.After.mp4
  1. Eels
  • before:
    2.-.Eels.Before.mp4
  • after:
    2.-.Eels.After.mp4
  1. Water elem
  • before:
    3.-.Water.Elem.Before.mp4
  • after:
    3.-.Water.Elem.After.mp4
  1. Snapjaw
  • before:
    4.-.Snapjaw.Before.mp4
  • after:
    4.-.Snapjaw.After.mp4

@MikaMauger
Copy link
Contributor Author

Also in reference to a screen shot earlier. NPC is swimming above the water surface.

I think that's the normal running animation

Indeed, he's just running here. The water effect on his feet always appear until NPC is fully on land.

@MikaMauger MikaMauger changed the title Revert "fix(MMAP): Build ADT floor just like WMO floor below liquid (… fix(MMAP/core/PathGenerator): Try to fix more water creatures Nov 11, 2022
@PkllonG
Copy link
Contributor

PkllonG commented Nov 11, 2022

Feels better than before

@Gultask
Copy link
Contributor

Gultask commented Nov 12, 2022

I've been testing for quite a while now and everything I can think of is much better than what it was. No regressions that I can see.
None of the problems reported in TC, either.

@Gultask Gultask added Tested This PR has been tested and is working. and removed Waiting to be Tested labels Nov 12, 2022
@Nefertumm
Copy link
Member

Removed blank space causing issues on CI

@smellbee
Copy link

when this will be merged?

@StraysFromPath
Copy link

I have tested this edit and it is a huge improvement fixing murlocs across darkshore and westfall.

@heyitsbench
Copy link
Contributor

What is the status on this?

@Nyeriah
Copy link
Member

Nyeriah commented Dec 25, 2022

What is the status on this?

It's going to be merged after the holidays.

@Gultask Gultask removed the SENSITIVE WIP SENSITIVE Work in Progress, Do not Update, Do not Merge until author approves. label Dec 29, 2022
@Annamaria-CC
Copy link
Member

@MikaMauger

I updated the MMAP version so the core will complain when old files are used

@Annamaria-CC Annamaria-CC merged commit 24fa3ba into azerothcore:master Jan 2, 2023
@Gultask
Copy link
Contributor

Gultask commented Jan 2, 2023

Thank you very much for the amazing fix, @MikaMauger !

@Annamaria-CC
Copy link
Member

Yes its looking good so far :)

@MikaMauger MikaMauger deleted the revert-9990-fix/oceanMMAPs branch January 3, 2023 12:32
Tennesseej pushed a commit to Tennesseej/azerothcore-wotlk that referenced this pull request Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE Related to the core file-cpp Used to trigger the matrix build Mmaps/Vmaps Issues with maps/vmaps/los Tested This PR has been tested and is working. To Be Merged
Projects
None yet