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.

Sign up for GitHub

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

Jump to bottom

[3.3.5][6.x] Core/Pets: Pet Attack Distance #16492

Merged
merged 4 commits into from Nov 28, 2016
Merged

[3.3.5][6.x] Core/Pets: Pet Attack Distance #16492

merged 4 commits into from Nov 28, 2016

Conversation

ccrs
Copy link
Member

@ccrs ccrs commented Feb 7, 2016

Search in active AutoCast spells for the correct chase distance.

@ccrs ccrs force-pushed the petcombatdistance branch 2 times, most recently from 6ba664d to 93d72f7 Compare February 7, 2016 07:41
@Rushor Rushor changed the title Core/Pets: Pet Attack Distance [3.3.5][6.x] Core/Pets: Pet Attack Distance Feb 7, 2016
@ccrs
Copy link
Member Author

ccrs commented Feb 8, 2016

btw fixes #15600 #301

@ccrs
Copy link
Member Author

ccrs commented Feb 9, 2016

  • Did you test this on a clean TC?
  • On which conditions did you test it?

@ccrs ccrs mentioned this pull request Feb 18, 2016
Closed
@Keader
Copy link
Member

Keader commented Feb 19, 2016

When I use one spell (pet spell) on a target and he's out of line of sight, the pet should pursue the target to be able to use the spell?

@Demonid
Copy link
Contributor

Demonid commented Feb 19, 2016

@Keader Indeed, the pet should go find the target to use the spell.

Something like this:

https://www.youtube.com/watch?v=IIxiusAehl0

Not my video btw.

@Goatform
Copy link
Contributor

@Keader yes, the pet should follow target if there is path available. By the way, what is has to do with this PR lol?

@ccrs
Copy link
Member Author

ccrs commented Feb 19, 2016

thats a complete unrelated issue to this PR.

@jackpoz
Copy link
Member

jackpoz commented Mar 25, 2016

could you please fix merge issues with this pr ?

@Aokromes
Copy link
Member

Any reason to don't merge this PR?

@Aokromes
Copy link
Member

Aokromes commented Jun 12, 2016 •
edited

If there is no reason to don't merge this, i will merge it on 24h.

joschiwald
joschiwald reviewed Jun 12, 2016
View reviewed changes
src/server/game/AI/CoreAI/PetAI.h
@@ -65,6 +67,8 @@ class TC_GAME_API PetAI : public CreatureAI
void DoAttack(Unit* target, bool chase);
bool CanAttack(Unit* target);
void ClearCharmInfoFlags();

TargetSpellList targetSpellStore;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this a member of PetAI now?

Copy link
Member Author

Choose a reason for hiding this comment

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

its declared right now on PetAI update, I simply moved it to the header.

Copy link
Member

Choose a reason for hiding this comment

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

if it's used only in a function body scope then it shouldn't be moved to class field

Copy link
Member Author

Choose a reason for hiding this comment

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

its used on update, so its declared on each tick :P

Copy link
Member

Choose a reason for hiding this comment

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

so ? what's wrong with that (and what's good with moving it as class member) ?

Copy link
Member

Choose a reason for hiding this comment

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

ping

Copy link
Member

Choose a reason for hiding this comment

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

still pinging

Treeston
Treeston reviewed Jul 16, 2016
View reviewed changes
src/server/game/Entities/Creature/Creature.cpp
if (spellInfo->GetRecoveryTime() == 0 && // No cooldown
spellInfo->RangeEntry->ID != 1 /*Self*/ && spellInfo->RangeEntry->ID != 2 /*Combat Range*/ && // Range bigger than melee
range < spellInfo->GetMaxRange(true)) // Bigger than previous
range = spellInfo->RangeEntry->minRangeHostile;
Copy link
Member

Choose a reason for hiding this comment

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

Why min range here?

@Treeston
Copy link
Member

Issue with this:

  1. Tell Water Elemental to attack target. It properly chases to cast range.
  2. Polymorph the target. The elemental stops casting, but chases to melee range even though bolt is still set to autocast.

@Aokromes
Copy link
Member

@ccrs #16492 (comment)

@ccrs
Copy link
Member Author

ccrs commented Nov 23, 2016

done

the other comments are not related to the PR, but to movement generators.

@Aokromes
Copy link
Member

There is anything left to do here? if no opositions i will merge this on monday.

@Aokromes Aokromes merged commit 2d1ef6b into TrinityCore:3.3.5 Nov 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackpoz jackpoz jackpoz approved these changes

Assignees
No one assigned
Labels
Branch-3.3.5a Comp-Core Feedback-PatchFix Sub-PetMinion
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet