Implementing: Implied Targets

[thefoof]

Update on this, I re-wrote my code multiple times in different places and now it uses the implied target BEFORE the error checks so none of those have to be altered, simplifying things a lot. Again this is only for the spell process and it may need a few more checks but it functions. Planning on commiting it soon then will look at getting regular combat to work through implied targets as well.

I've tested it with both friendly and non-friendly spells and seems to work great =)

video

Some of the other devs should check out the code to make sure it doesn't break anything (not sure how it would, all of the code I wrote can be found within the functions "SpellProcess::ProcessSpell" and "SpellProcess::GetSpellTargets")

[Jabantiz]

I looked over the code you submitted to SVN and it all looks good. The only thing I noticed was when you check for a pet you seem to assume it is a player pet, it is possible for it to belong to a NPC so you need to check who "owns" the pet, you can do that with GetOwner()

Code: Select all

if (secondary_target->IsPet() && secondary_target->GetOwner()->IsNPC())

You need to make sure the spawn is a pet first before you call GetOwner() as it will return a null pointer and crash the server if it is called on a NPC that is not a pet.

[thefoof]

I looked over the code you submitted to SVN and it all looks good. The only thing I noticed was when you check for a pet you seem to assume it is a player pet, it is possible for it to belong to a NPC so you need to check who "owns" the pet, you can do that with GetOwner()

Code: Select all

if (secondary_target->IsPet() && secondary_target->GetOwner()->IsNPC())

You need to make sure the spawn is a pet first before you call GetOwner() as it will return a null pointer and crash the server if it is called on a NPC that is not a pet.

Thanks for looking it over. I'll make that change later, I also discovered a ProcessSpell bug where if you call the function without a target the server crashes. So I added a check in the useability command to default target to caster, if caster has no target.

[thefoof]

One flaw I've found now, if you use a friendly spell with a single target type, on an npc without a target, the spell gets cast on the npc.

I tried to just add another check in GetSpellTargets() to set initial_target and target as the caster, but that causes a crash and I can't figure out why.

EDIT: I added an error check for now to stop friendly spells from being cast on non-player pet npcs until I or someone else comes back to this.

[Jabantiz]

I tried to just add another check in GetSpellTargets() to set initial_target and target as the caster, but that causes a crash and I can't figure out why.

My guess would be a null pointer, I would have to see the code that causes the crash though to be sure.

[thefoof]

Looked at this a little bit today, getting crashes on startup so I think there's just something fundamental about pointers in c++ I don't understand. Here's the code I came up with, for implied auto attack

Code: Select all

void Player::ProcessCombat() {
	Spawn* secondary_target = 0;
	if (target->HasTarget())
	{
		secondary_target = target->GetTarget();
		if (!secondary_target->IsPlayer())
		{
			if (secondary_target->appearance.attackable)
			{
				if (secondary_target->IsNPC())
				{
					if (!secondary_target->IsPet() || (secondary_target->IsPet() && ((NPC*)secondary_target)->GetOwner()->IsNPC()))
					{
						target = secondary_target;
					}
				}
			}
		}
	}
	// If no target OR target is not an entity OR not in combat OR casting a spell return out
	if (!target || !target->IsEntity() || !EngagedInCombat() || IsCasting())
		return;

Not 100% sure this would work in practice as the server crashes on startup, but the check should work, if someone else with higher knowledge want's to take a look at it. I'm not going to mess with this anymore as this is the only place I could find to set the attack target.

[thefoof]

Lol it just dawned on me randomly I was checking target before checking if there was a target so that was probably the problem *facepalm*

Code: Select all

void Player::ProcessCombat() {
	//If no target delete combat_target and return out
	if (!target) {
		combat_target = 0;
		return;
	}
	// If is not an entity OR not in combat OR casting a spell return out
	if (!target->IsEntity() || !EngagedInCombat() || IsCasting())
		return;

	if (target->HasTarget()) {
		if (target->IsPlayer() || (target->IsNPC() && target->IsPet() && !((NPC*)target)->GetOwner()->IsNPC())){
			Spawn* secondary_target = target->GetTarget();
			if (secondary_target->IsEntity()) {
				if (!secondary_target->IsPlayer()) {
					if (secondary_target->IsNPC()) {
						if (secondary_target->appearance.attackable) {
							if (!secondary_target->IsPet() || (secondary_target->IsPet() && ((NPC*)secondary_target)->GetOwner()->IsNPC())) {
								combat_target = secondary_target;
							}
						}
					}
				}
			}
		}
	}
	else
		combat_target = target;

Code committed, I made a few design changes. Changing target in this code would affect spellcasting targets in certain situations, so I instead added a new pointer in Player.h. When ProcessCombat() is called and the player does not have a target, combat_target gets set to 0.

If someone knows what value gets sent to tell the client to face a certain target in combat, let me know, or just set it to send combat_target.

I also changed the check for the autoattack command so it can be used without a target/friendly target. I assume it the check was there before because there wasn't a check in old combat code, but combat handles this now.

[John Adams]

Excellent work, Foof. I hope to get time soon to check it out. Implied Targets were one of my favorite features going from EQ to EQ2 years ago

[thefoof]

I tried to just add another check in GetSpellTargets() to set initial_target and target as the caster, but that causes a crash and I can't figure out why.

I got this figured out, it was because of how the implied target logwrite was getting the name of the implied target, committed my code so this situation should work right now.

[thefoof]

Just posting to say this system is pretty much ready for testing - can't think of anything else that needs to be added that can be known without testing it out thoroughly. (other than the facing the right target thing but that might be client side)

[John Adams]

Awesome Foof. I'll get the code on EQ2TC for our numerous players to begin hammering on right away