-
Notifications
You must be signed in to change notification settings - Fork 4
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
Chain Spells Range (Thorim lightning et al) with Suggested Fix. #211
Comments
I'm bored so I may try to fix it and open a PR on TC for it. |
Well, this fix seems to work TrinityCore/TrinityCore#19412 however it has yet to be scrutinized by TC and depending on how old Feenix' fork is you'll meet some changes that differ from the commits here. They may not be immediately mergeable. However, the concept is the same regardless of when you forked. |
TC was kinda different from our core but the pull request is in ^^ |
@Fapswhenoom Interesting, let me know how it goes! There may be some side effects and I've asked for some guidance on that PR but haven't received much of a reply. All I can say is it will chain exactly the distance it is suppose to from the center but I'm unsure if this is a grounded center (2D) or a middle of the object. This may cause problems with large bosses who are being chain lighting'd, as noted in a WoW forums post I linked. I'm unsure how the expected behavior and tbh unsure of how it behaves against large bosses since the original code and my code both rely on a mixture of both 2D and 3D distance calculations. |
Yeah, I tried to apply some common sense when it came to mergin your PR into our code but it compiles and doesn't immediately causes a crash. My PSU died so I haven't been able to test it properly but that's what PTR is for ^^ |
Issue:
Chain spells are chaining at a range of n + 1.5y for most players where n is the chain distance they are suppose to have.
Affects:
Resto shamans, your reign will be no longer =).
So, this is something that affects most servers. I believe what I'm about to describe it NOT expected behavior but I have no proof off-hand. Most servers just hack fix it at best.
Chain targets are currently using the API designed for object searching. They use the predicate WorldObjectSpellAreaTargetCheck. This is all well and good BUT chain targets should NOT be using this predicate imo. I think it was designed with the concept of AOE spells. This can be seen by analyzing WorldObjectSpellAreaTargetCheck::operator()
It looks fine and reasonable until you actually dig into the implementation of the distance calculations.
This is how they compute whether an object is within distance. They use the dist additively with GetObjectSize(). What is GetObjectSize()? It's defined by a Units UNIT_COMBAT_REACH field which defaults to 1.5y and can be modified with things like Noggen Fogger. UNIT_COMBAT_REACH is currently defined by unit object scale * default reach. So, that's another issue. Only hack fixing it will still make noggenfogger or baby spice abusable.
Solution:
Create a separate predicate to be used in target check for the world object searcher that either uses a different calculation method for distance/range. Maybe more flags like the horrific LoS fix I did for TC? That was unfun to deal with though. So I'd recommend just creating another method like IsWithinCenterDist3d or something. Would be pretty easy to fix if C++ wasn't a 1970s language that supposed predicates easier. But I digress.
Yea, soooo that should work? Who knows.
The text was updated successfully, but these errors were encountered: