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 (Core\Script) Map Calculation Rewrite #8775

Closed
wants to merge 32 commits into from
Closed

Fix (Core\Script) Map Calculation Rewrite #8775

wants to merge 32 commits into from

Conversation

acidmanifesto
Copy link
Contributor

@acidmanifesto acidmanifesto commented Oct 27, 2021

CURRENT STATUS

Only one issue left.
Blink and any front Leap spell seems to clip thru floors and bridges. Terrain ground and gameobjects is fine. Just need to figure out the going thru the floor and bridge issue now.

Purpose of this PR:
This is to address Blink, Warrior's Charge, and other vmap calculated functions that may be problematic in certain cases.

Changes Proposed:

This is to address various issues of Blink, Warrior's charge, and other vmap related calculated funcitons.

Issues Addressed:

Errornous vmap calculation handling.

Tests Performed:

Builds and compiles with no issues. Other scripts adjusted in core with this pr.

How to Test the Changes:

  1. .cheat power on
  2. as a priest perform the move blink spell 1953 in any and all terran variations
  3. as a warrior perform the move Charge spell 100 in any and all terran variations

Known Issues and TODO List:

Test any variations where vmap calculations may be needed.
Test whatever quest\scenario\instance that the spell script spell_svalna_revive_champion is performed. located in northerned map.
Test Exsisting transports to ensure u can still ride from point a to point b (Horde blimps), Flight manager.

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.

This is to address Blink, Warrior's Charge, and other vmap calculated functions that may be problematic in certain cases.
@github-actions github-actions bot added CORE Related to the core Script labels Oct 27, 2021
@acidmanifesto
Copy link
Contributor Author

I request the Tester's needed. We will need alot of testing before any possiblity of merging is to be considered.

@acidmanifesto acidmanifesto changed the title Map Calculation Rewrite Fix (Core\Script) Map Calculation Rewrite Oct 27, 2021
Copy link
Member

@Nefertumm Nefertumm left a comment

Choose a reason for hiding this comment

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

Seems fine now

@Nefertumm
Copy link
Member

Build is failing

/home/runner/work/azerothcore-wotlk/azerothcore-wotlk/src/server/scripts/Northrend/IcecrownCitadel/icecrown_citadel.cpp:2187:131: fatal error: implicit conversion turns floating-point number into bool: 'float' to 'bool' [-Wimplicit-conversion-floating-point-to-bool]
            pos.m_positionZ = caster->GetMap()->GetHeight(caster->GetPhaseMask(), pos.GetPositionX(), pos.GetPositionY(), caster->GetPositionZ(), true);

@acidmanifesto
Copy link
Contributor Author

Build is failing

/home/runner/work/azerothcore-wotlk/azerothcore-wotlk/src/server/scripts/Northrend/IcecrownCitadel/icecrown_citadel.cpp:2187:131: fatal error: implicit conversion turns floating-point number into bool: 'float' to 'bool' [-Wimplicit-conversion-floating-point-to-bool]
            pos.m_positionZ = caster->GetMap()->GetHeight(caster->GetPhaseMask(), pos.GetPositionX(), pos.GetPositionY(), caster->GetPositionZ(), true);

its gonna be one of those days with the compiling testing. im on it.

@acidmanifesto
Copy link
Contributor Author

At this point i fix one thing with float to bool i end up getting another one. This is starting to extend my reach of what i want to do, and seems to be leaning towards a massive core rewrite which i just dont have the time or the patience for. I will have ask for a help wanted tag for the time being. This pr does compile and can be tested in game. However that is no excuse for failing compile testing.

@mpfans
Copy link
Contributor

mpfans commented Oct 28, 2021

I will test this PR in two days

@mpfans
Copy link
Contributor

mpfans commented Oct 29, 2021

Because it conflicts with one of my PR, I can't test it

@acidmanifesto
Copy link
Contributor Author

acidmanifesto commented Oct 30, 2021

ok. so extensive testing on my end. there appears to be select issues with blink allowing the priest character to mesh under GAMEOBJECTS. I suspect this is another issues dealing with the version of vmaps\mmaps we have currently on azerothcore. This PR will need to sit stagnant until we ever upgrade maps to a higher revision.

@acidmanifesto
Copy link
Contributor Author

#6232

@acidmanifesto
Copy link
Contributor Author

#7072

@acidmanifesto
Copy link
Contributor Author

Current this is how it looks with this PR.

Show casing Warrior's charge and Blink below:

Warriors Charge:
https://youtu.be/znHdW1R_9zM

Blink with the vmap issues at the end:
https://youtu.be/lOamL5nAveU

So far only blink suffers the issues as we need to have the vmaps be in line with version 4.8

@ghost ghost mentioned this pull request Nov 15, 2021
@ghost
Copy link

ghost commented Nov 25, 2021

Does this need to be tested yet or is this WIP?

@acidmanifesto acidmanifesto added the SENSITIVE WIP SENSITIVE Work in Progress, Do not Update, Do not Merge until author approves. label Apr 2, 2022
@acidmanifesto
Copy link
Contributor Author

acidmanifesto commented Apr 2, 2022

Current Status:
Charge does not undermesh
Blink still clips thru gameobjects and structures.

Results:
Need map improvements, which there are other PR's in progress on from other people. will test then when new maps gets deployed.

@acidmanifesto
Copy link
Contributor Author

acidmanifesto commented Apr 25, 2022

i need to retest to see if blink is still clipping thru game objects like bridges and stones, houses, whatever.
Warrior Charge is good with no issues. But if u do charge while in .gm on you will hit some weird assert error, but only when in .gm on
the thing is we only need my map.cpp and map.h adjustments.
but then we needed the collision to be linked to it via dynamic tree and because of that we ended up with errors in the creature cpp, object cpp and icecrown_citadel.cpp
so one adjust ment literally became a small cluster mess. so i have to look what i need to change in the dynamictree to fix the collison being ignored with blink
then worry about the other files crying error later such as the compile testers.

@IntelligentQuantum
Copy link
Member

I ported this from TC Maybe it will help for charge idk
IntelligentQuantum@e571b09

@acidmanifesto
Copy link
Contributor Author

I ported this from TC Maybe it will help for charge idk IntelligentQuantum@e571b09

i will look into that later. but there was other reasons for this pr besides charge.

@Yehonal Yehonal removed the Script label Apr 25, 2022
@acidmanifesto
Copy link
Contributor Author

CURRENT STATUS

Only one issue left.
Blink and any front Leap spell seems to clip thru floors and bridges. Terrain ground and gameobjects is fine. Just need to figure out the going thru the floor and bridge issue now.

acidmanifesto and others added 2 commits April 25, 2022 19:29
Co-authored-by: Kargatum <dowlandtop@yandex.com>
Co-authored-by: Kargatum <dowlandtop@yandex.com>
{
G3D::Vector3 v(x, y, z);
G3D::Ray r(v, G3D::Vector3(0, 0, -1));
DynamicTreeIntersectionCallback callback(phasemask, VMAP::ModelIgnoreFlags::Nothing);
impl->intersectZAllignedRay(r, callback, maxSearchDist);

if (callback.didHit())
if (dCallback)
Copy link
Contributor

Choose a reason for hiding this comment

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

dCallback is always null, so this is completely useless.

@UltraNix
Copy link
Contributor

UltraNix commented May 7, 2022

Sorry, but to me all the changes done here are nonsenses

@acidmanifesto
Copy link
Contributor Author

Sorry, but to me all the changes done here are nonsenses

it stops the undermshing and been stopping all undermeshing for a while. i encourage u to try the pr and see first hand.

@UltraNix
Copy link
Contributor

UltraNix commented May 7, 2022

I don't need to test it - I just see the code. Map::GetHeight always returns VMAP_INVALID_HEIGHT_VALUE so broke many things.
E.g.

  • .gps always returns invalid ground and floor z
  • non-flyable mobs can fly to you (just .gm fly, ascend and .cometome to any mob
  • any of the gameobjects with collision (elevators, ships, etc) get wrong ground position
  • many spells with destination are also broken, e.g. blink, fishing
  • flying mobs on death gets wrong fall position
  • confused movement generator (e.g. blind) completely broken
  • and many more...

@acidmanifesto
Copy link
Contributor Author

I don't need to test it - I just see the code. Map::GetHeight always returns VMAP_INVALID_HEIGHT_VALUE so broke many things. E.g.

  • .gps always returns invalid ground and floor z
  • non-flyable mobs can fly to you (just .gm fly, ascend and .cometome to any mob
  • any of the gameobjects with collision (elevators, ships, etc) get wrong ground position
  • many spells with destination are also broken, e.g. blink, fishing
  • flying mobs on death gets wrong fall position
  • confused movement generator (e.g. blind) completely broken
  • and many more...

if you feel like all this is happening with out testing then you are welcome to close the PR.

@UltraNix
Copy link
Contributor

UltraNix commented May 7, 2022

Yes - I'm pretty sure.

@UltraNix UltraNix closed this May 7, 2022
@acidmanifesto acidmanifesto deleted the MapCalculations branch May 7, 2022 16:58
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 Help Wanted SENSITIVE WIP SENSITIVE Work in Progress, Do not Update, Do not Merge until author approves. WIP Work in Progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants