fix for Wyvern Healing Breath not targeting party members

[Casey]

Changes:
Added future support for Vishap Armet increasing healing breath trigger point.
Cleaned up some code in hp threshold checking

Additions:
Added a party member HP check to heal them instead.
The master always is healed before party members.

This should close a relatively old ticket : http://bugs.dspt.info/show_bug.cgi?id=388

However, there is one thing that should be addressed before a merge:

the healing breath target type is for party members, but the CAIPetDummy::preparePetAbility function forces an ability under that target type to target the master, which is inconsistent. my patch puts an exclusion on the healing breath skills as the code exists now.

For example, the CTargetFind::getValidTarget function has a check for TARGET_PLAYER_PARTY that should function exactly like healing breath needs to.

If the preparePetAbility function is not changed, then if a party member is found at trigger% hp, then the wyvern will heal the master instead of the intended target.

[kjLotus]

the healing breath target type is for party members, but the CAIPetDummy::preparePetAbility function forces an ability under that target type to target the master, which is inconsistent.

It seems that the intended functionality of ActionAbilityStart and preparePetAbility is thus:

in ActionAbilityStart, checks for the pet being a wyvern and the wyvern type (this is where you had most of your changes). In there, you'd do all the checks for who to use breath on (like you did in your patch, so that's good so far). Then, once the target is determined (currently it only checks the master), set the wyverns PBattleSubTarget to that character. Then execute preparePetAbility(m_PBattleSubTarget) (which is already done). The thing is, preparePetAbility doesn't even use the passed in target, and does what-the-fuck-ever to assign its own target based on validTargets. It should pretty much all be removed, and have the ActionTarget just be set to PTarg. The thing is, I'm not sure if that'll break a bunch of other things (ie. I don't see any distinction between, say, Earthen Ward (self cast) and Megalith Throw (target) in ActionAbilityStart). Ideally, the validTargets checks should be in ActionAbilityStart, and verifying that the determined target does fall under the category from getValidTargets would happen just before preparePetAbility.

I'm not asking you to change anything, as I'll do it when I commit it. Since I don't have much time to work on anything until finals are over, it'll probably be a while, though. So it's up to you whether you want to do those changes or wait for me to do them :p

[Casey]

That makes sense. I am by no means an expert on PUP/SMN/BST, I really only know how DRG's wyvern is supposed to work.

That said, looking at the code, this is the SMN avatar pet code in CAIPetDummy::ActionAbilityStart()

Code: Select all

	else if(m_PPet->getPetType()==PETTYPE_AVATAR){
		for(int i=0; i<m_PPet->PetSkills.size(); i++){
			if(m_PPet->PetSkills[i]->getAnimationTime() == m_MasterCommand){
				m_PMobSkill = m_PPet->PetSkills[i];
				m_MasterCommand = MASTERCOMMAND_NONE;
				preparePetAbility(m_PPet);
				return;
			}
		}
		m_MasterCommand = MASTERCOMMAND_NONE;
	}

It seems it's relying on CAIPetDummy::preparePetAbility to actually do the targeting "work" with

Code: Select all

		// find correct targe
		if(m_PMobSkill->getValidTargets() & TARGET_SELF)
		{ //self
		    m_PBattleSubTarget = m_PPet;
		}
		else if(m_PMobSkill->getValidTargets() & TARGET_PLAYER_PARTY && (m_PMobSkill->getID() != 638 && m_PMobSkill->getID() != 639 && m_PMobSkill->getID() != 640))
		{
			m_PBattleSubTarget = m_PPet->PMaster;
		}
		else
		{
			if(m_PBattleTarget != NULL)
			{
			    m_PBattleSubTarget = m_PBattleTarget;
			}
			DSP_DEBUG_BREAK_IF(m_PBattleSubTarget == NULL);
		}

It seems like this was done to be a catch-all for bst pets and smn pets.

It appears I could remove that oddly-placed code in CAIPetDummy::preparePetAbility, clean it up and put it in the bst/smn pet code and hopefully it would break nothing.

This way, special case targeting like wyvern healing breath can co-exist with "suggested" targeting types which seems to be the case with bloodpacts/BST pets.

I'll do some research to see how earthen ward and other BPs work and report back

[Casey]

It looks like CAIPetDummy::ActionAbilityFinish() queues up the targets for AoE based upon the SQL entry for the skill, so changing the code structure a bit so that CAIPetDummy::preparePetAbility doesn't have a pointless parameter seems to work fine.

My new attached patch doesn't seem to break anything, but i'd want it checked out by other people before a merge since it is a relatively large change to the structure of pet skills.

[kjLotus]

It seems like this was done to be a catch-all for bst pets and smn pets.

It appears I could remove that oddly-placed code in CAIPetDummy::preparePetAbility, clean it up and put it in the bst/smn pet code and hopefully it would break nothing.

This way, special case targeting like wyvern healing breath can co-exist with "suggested" targeting types which seems to be the case with bloodpacts/BST pets.

Right, this would be more or less what I'd like to see. I'm pretty sure wyvern breath is the only pet ability that isn't AOE and can target party members (ignore PUP for now, I'll fix it when I actually do the PUP AI). Anyways, I don't think preparePetAbility should be in charge of targeting, only for setting up the action and pushing it on the action queue.

Looking at your posted diff seems to be what I was looking for. After I finish my homework and test a bit on my server, I'll ask whasf to throw it on the test servers for a bit to see what happens.