New Server, Couple of Small Issues

[Uleat]

Please read both pages of this thread and re-ask your question after doing so:
http://www.eqemulator.org/forums/sho...hlight=spellid

[haggzor]

Alright, well I figured out one thing. This is the line of code that's causing Harm Touch to fail (client_packet.cpp line 4623, Handle_OP_CastSpell method):

		
if(castspell->slot < MAX_PP_MEMSPELL)
		{
			LogFile->write(EQEMuLog::Debug, "Slot [%d] is less than MAX_PP_MEMSPELL [%d]", castspell->slot, MAX_PP_MEMSPELL);

			spell_to_cast = m_pp.mem_spells[castspell->slot];
			if(spell_to_cast != castspell->spell_id)
			{
				LogFile->write(EQEMuLog::Debug, "Code thinks I'm a cheater because [%d] does not equal [%d]", spell_to_cast, castspell->spell_id);

				InterruptSpell(castspell->spell_id); //CHEATER!!!
				return;
			}
		}
		else {
			LogFile->write(EQEMuLog::Debug, "Slot [%d] is greater than or equal to MAX_PP_MEMSPELL [%d]", castspell->slot, MAX_PP_MEMSPELL);

			InterruptSpell();
			return;
		}

In eq_packet_structs, you can see that MAX_PP_MEMSPELL is set to 9.

static const uint32 MAX_PP_MEMSPELL		= 9;

In the comments for Harm Touch and LoH it mentions that it's assuming that the spell slot is going to be 8, which is not correct in the Titanium client. LoH and Harm Touch are spell slot 9 (ABILITY_SLOT).

Does anyone know what the MAX_PP_MEMSPELL represents? I haven't been able to figure that one out yet. My assumption is that it represents a separation between memorized spells and ability/discipline/item spells (which are all handled individually).

Basically, as long as that value is set to 9 and Harm Touch is coming in on spell slot 9, it automatically goes to interruptSpell, which is why it's not working. My guess is that this block needs an additional elseif specifically for Abilities that are being processed as spells, such as LoH and Harm Touch.

Any input would be wonderful!

[haggzor]

Alrighty, that fixed it. Basically I added a flag in the existing special SK and Paladin portion of the code to bypass the spell slot evaluation and memorized spell check that was breaking Harm Touch and Lay on Hands.

client_packet.cpp(line 4581)

		uint16 spell_to_cast = 0;

		// flag to bypass memorized spell check
		bool isKnightAbility = false;

		//current client seems to send LH in slot 8 now...
		if(castspell->slot == ABILITY_SPELL_SLOT &&
			castspell->spell_id == SPELL_LAY_ON_HANDS && GetClass() == PALADIN) {
			if(!p_timers.Expired(&database, pTimerLayHands)) {
				Message(13,"Ability recovery time not yet met.");
				InterruptSpell(castspell->spell_id);
				return;
			}
			spell_to_cast = SPELL_LAY_ON_HANDS;

			isKnightAbility = true;
			
			p_timers.Start(pTimerLayHands, LayOnHandsReuseTime);
		} else if(castspell->slot == ABILITY_SPELL_SLOT &&
			(castspell->spell_id == SPELL_HARM_TOUCH
				|| castspell->spell_id == SPELL_HARM_TOUCH2
			) && GetClass() == SHADOWKNIGHT) {

			if(!p_timers.Expired(&database, pTimerHarmTouch)) {
				Message(13,"Ability recovery time not yet met.");
				InterruptSpell(castspell->spell_id);
				return;
			}

			if(GetLevel() < 40)
				spell_to_cast = SPELL_HARM_TOUCH;
			else
				spell_to_cast = SPELL_HARM_TOUCH2;
				
			LogFile->write(EQEMuLog::Debug, "Harm Touch Spell to Cast: %d (based on lvl [%d])... starting recast timer.", spell_to_cast, GetLevel());
			
			isKnightAbility = true;

			p_timers.Start(pTimerHarmTouch, HarmTouchReuseTime);
		}

		//handle disciplines, OLD, they keep changing this
		if(castspell->slot == DISCIPLINE_SPELL_SLOT) {
			if(!UseDiscipline(castspell->spell_id, castspell->target_id)) {
				printf("Unknown ability being used by %s, spell being cast is: %i\n",GetName(),castspell->spell_id);
				InterruptSpell(castspell->spell_id);
			}
			return;
		}

		if(castspell->slot < MAX_PP_MEMSPELL)
		{
			LogFile->write(EQEMuLog::Debug, "Slot [%d] is less than MAX_PP_MEMSPELL [%d]", castspell->slot, MAX_PP_MEMSPELL);

			spell_to_cast = m_pp.mem_spells[castspell->slot];
			if(spell_to_cast != castspell->spell_id)
			{
				LogFile->write(EQEMuLog::Debug, "Code thinks I'm a cheater because [%d] does not equal [%d]", spell_to_cast, castspell->spell_id);

				InterruptSpell(castspell->spell_id); //CHEATER!!!
				return;
			}
		}
		else if (castspell->slot >= MAX_PP_MEMSPELL && !isKnightAbility) {
			LogFile->write(EQEMuLog::Debug, "Slot [%d] is greater than or equal to MAX_PP_MEMSPELL [%d]", castspell->slot, MAX_PP_MEMSPELL);

			InterruptSpell();
			return;
		}
		
		LogFile->write(EQEMuLog::Debug, "Casting Spell: [%i] on target [%d]", spell_to_cast, castspell->target_id);

		CastSpell(spell_to_cast, castspell->target_id, castspell->slot);

[provocating]

So will this go into the GIT source as a fix soon? Please !

I just compiled this fix and everything seems to work.

[demonstar55]

Well, you can do it without an extra flag and its not in a format I feel like dealing with. (Either pull requests on github or a unified diff posted on the code submission forums, note pull requests are preferred because its easier, unless you don't know how to use git that is ...)

[haggzor]

Yeah, this isn't the prettiest fix in the world. =)

I just posted it in this format because I wanted to show where the problem was located. I had sort of assumed that the development team didn't want random people doing code submissions and that they would fix it in whatever way they see as best. If you would like, I'd be happy to pretty it up some, remove some of the extraneous logging that I added and submit it. I apologize, I'm pretty new to this community. I set up this account a few years ago but never actually got as far as installing anything until this week. Heh.

[demonstar55]

Its an open source project, any submissions are welcome, but not guaranteed to be accepted of course.

I would recommend submitting pull requests on GitHub since it's much easier on people with access to pull them.

[haggzor]

Admittedly, I'm new to Git. I've always used SVN. I assume that since Git doesn't have a single master repo that I can access, I'm safe to just commit my updated changes? How do I then submit the commit for review (I'm reading about Git as I'm writing this)?

[demonstar55]

Basically, fork the main repo, commit your changes to the fork, make a pull request to your fork. Its recommended that you push your changes to a branch on your fork.

This thread might help, I'm sure github has some better docs in their support http://eqemulator.org/forums/showthread.php?t=36515

[haggzor]

Perfect, thanks!

[demonstar55]

http://bpaste.net/show/162354/ if someone wants to test this patch (I can't right now), I think it should be better. (Plus I think it was the original intent of the code ...)

[haggzor]

Code committed in a branch and pull request completed:

https://github.com/EQEmu/Server/pull/102

I'm going to put a copy of the fix on here so that others might have a stopgap measure until if / when it's approved.

Updates to zone/client_packet.cpp (line 4476)

void Client::Handle_OP_CastSpell(const EQApplicationPacket *app)
{
	if (app->size != sizeof(CastSpell_Struct)) {
		std::cout << "Wrong size: OP_CastSpell, size=" << app->size << ", expected " << sizeof(CastSpell_Struct) << std::endl;
		return;
	}
	if (IsAIControlled()) {
		this->Message_StringID(13, NOT_IN_CONTROL);
		//Message(13, "You cant cast right now, you arent in control of yourself!");
		return;
	}

	CastSpell_Struct* castspell = (CastSpell_Struct*)app->pBuffer;

#ifdef _EQDEBUG
	LogFile->write(EQEMuLog::Debug, "cs_unknown2: %u %i", (uint8)castspell->cs_unknown[0], castspell->cs_unknown[0]);
	LogFile->write(EQEMuLog::Debug, "cs_unknown2: %u %i", (uint8)castspell->cs_unknown[1], castspell->cs_unknown[1]);
	LogFile->write(EQEMuLog::Debug, "cs_unknown2: %u %i", (uint8)castspell->cs_unknown[2], castspell->cs_unknown[2]);
	LogFile->write(EQEMuLog::Debug, "cs_unknown2: %u %i", (uint8)castspell->cs_unknown[3], castspell->cs_unknown[3]);
	LogFile->write(EQEMuLog::Debug, "cs_unknown2: 32 %p %u", &castspell->cs_unknown, *(uint32*) castspell->cs_unknown );
	LogFile->write(EQEMuLog::Debug, "cs_unknown2: 32 %p %i", &castspell->cs_unknown, *(uint32*) castspell->cs_unknown );
	LogFile->write(EQEMuLog::Debug, "cs_unknown2: 16 %p %u %u", &castspell->cs_unknown, *(uint16*) castspell->cs_unknown, *(uint16*) castspell->cs_unknown+sizeof(uint16) );
	LogFile->write(EQEMuLog::Debug, "cs_unknown2: 16 %p %i %i", &castspell->cs_unknown, *(uint16*) castspell->cs_unknown, *(uint16*) castspell->cs_unknown+sizeof(uint16) );
#endif
	LogFile->write(EQEMuLog::Debug, "OP CastSpell: slot=%d, spell=%d, target=%d, inv=%lx", castspell->slot, castspell->spell_id, castspell->target_id, (unsigned long)castspell->inventoryslot);

	if ((castspell->slot == USE_ITEM_SPELL_SLOT) || (castspell->slot == POTION_BELT_SPELL_SLOT))	// ITEM or POTION cast
	{
		//discipline, using the item spell slot
		if (castspell->inventoryslot == 0xFFFFFFFF) {
			if (!UseDiscipline(castspell->spell_id, castspell->target_id)) {
				LogFile->write(EQEMuLog::Debug, "Unknown ability being used by %s, spell being cast is: %i\n", GetName(), castspell->spell_id);
				InterruptSpell(castspell->spell_id);
			}
			return;
		}
		else if ((castspell->inventoryslot < 30) || (castspell->slot == POTION_BELT_SPELL_SLOT))	// sanity check
		{
			const ItemInst* inst = m_inv[castspell->inventoryslot]; //slot values are int16, need to check packet on this field
			//bool cancast = true;
			if (inst && inst->IsType(ItemClassCommon))
			{
				const Item_Struct* item = inst->GetItem();
				if (item->Click.Effect != (uint32)castspell->spell_id)
				{
					database.SetMQDetectionFlag(account_name, name, "OP_CastSpell with item, tried to cast a different spell.", zone->GetShortName());
					InterruptSpell(castspell->spell_id);	//CHEATER!!
					return;
				}

				if ((item->Click.Type == ET_ClickEffect) || (item->Click.Type == ET_Expendable) || (item->Click.Type == ET_EquipClick) || (item->Click.Type == ET_ClickEffect2))
				{
					if (item->Click.Level2 > 0)
					{
						if (GetLevel() >= item->Click.Level2)
						{
							ItemInst* p_inst = (ItemInst*)inst;
							int i = parse->EventItem(EVENT_ITEM_CLICK_CAST, this, p_inst, nullptr, "", castspell->inventoryslot);

							if (i == 0) {
								CastSpell(item->Click.Effect, castspell->target_id, castspell->slot, item->CastTime, 0, 0, castspell->inventoryslot);
							}
							else {
								InterruptSpell(castspell->spell_id);
								return;
							}
						}
						else
						{
							database.SetMQDetectionFlag(account_name, name, "OP_CastSpell with item, did not meet req level.", zone->GetShortName());
							Message(0, "Error: level not high enough.", castspell->inventoryslot);
							InterruptSpell(castspell->spell_id);
						}
					}
					else
					{
						ItemInst* p_inst = (ItemInst*)inst;
						int i = parse->EventItem(EVENT_ITEM_CLICK_CAST, this, p_inst, nullptr, "", castspell->inventoryslot);

						if (i == 0) {
							CastSpell(item->Click.Effect, castspell->target_id, castspell->slot, item->CastTime, 0, 0, castspell->inventoryslot);
						}
						else {
							InterruptSpell(castspell->spell_id);
							return;
						}
					}
				}
				else
				{
					Message(0, "Error: unknown item->Click.Type (0x%02x)", item->Click.Type);
				}
			}
			else
			{
				Message(0, "Error: item not found in inventory slot #%i", castspell->inventoryslot);
				InterruptSpell(castspell->spell_id);
			}
		}
		else
		{
			Message(0, "Error: castspell->inventoryslot >= 30 (0x%04x)", castspell->inventoryslot);
			InterruptSpell(castspell->spell_id);
		}
	}
	else if (castspell->slot == DISCIPLINE_SPELL_SLOT) {	// DISCIPLINE cast
		if (!UseDiscipline(castspell->spell_id, castspell->target_id)) {
			printf("Unknown ability being used by %s, spell being cast is: %i\n", GetName(), castspell->spell_id);
			InterruptSpell(castspell->spell_id);
			return;
		}
	}
	else if (castspell->slot == ABILITY_SPELL_SLOT) {	// ABILITY cast (LoH and Harm Touch)
		uint16 spell_to_cast = 0;

		if (castspell->spell_id == SPELL_LAY_ON_HANDS && GetClass() == PALADIN) {
			if (!p_timers.Expired(&database, pTimerLayHands)) {
				Message(13, "Ability recovery time not yet met.");
				InterruptSpell(castspell->spell_id);
				return;
			}
			spell_to_cast = SPELL_LAY_ON_HANDS;
			p_timers.Start(pTimerLayHands, LayOnHandsReuseTime);
		}
		else if ((castspell->spell_id == SPELL_HARM_TOUCH
			|| castspell->spell_id == SPELL_HARM_TOUCH2) && GetClass() == SHADOWKNIGHT) {
			if (!p_timers.Expired(&database, pTimerHarmTouch)) {
				Message(13, "Ability recovery time not yet met.");
				InterruptSpell(castspell->spell_id);
				return;
			}

			// determine which version of HT we are casting based on level
			if (GetLevel() < 40)
				spell_to_cast = SPELL_HARM_TOUCH;
			else
				spell_to_cast = SPELL_HARM_TOUCH2;

			p_timers.Start(pTimerHarmTouch, HarmTouchReuseTime);
		}
		
		if (spell_to_cast > 0)	// if we've matched LoH or HT, cast now
			CastSpell(spell_to_cast, castspell->target_id, castspell->slot);
	}
	else	// MEMORIZED SPELL (first confirm that it's a valid memmed spell slot, then validate that the spell is currently memorized)
	{
		uint16 spell_to_cast = 0;

		if(castspell->slot < MAX_PP_MEMSPELL)
		{
			spell_to_cast = m_pp.mem_spells[castspell->slot];
			if(spell_to_cast != castspell->spell_id)
			{
				InterruptSpell(castspell->spell_id); //CHEATER!!!
				return;
			}
		}
		else if (castspell->slot >= MAX_PP_MEMSPELL) {
			InterruptSpell();
			return;
		}

		CastSpell(spell_to_cast, castspell->target_id, castspell->slot);
	}
	return;
}

[haggzor]

BTW, thanks to demonstar55 for the great Git tutorial. Helped a lot.

[haggzor]

@Provocating

The bug fix has been approved and merged. It should be available soon in Git.

[provocating]

And it worked perfectly, thanks much!