Basic Attack Behavior Live Accuracy Improvements (#926)

* Overhaul BasicAttack Behavior so it matches the live 1.10.64 client

dCommon/dEnums/eBasicAttackSuccessTypes.h

@@ -0,0 +1,12 @@
+#ifndef __EBASICATTACKSUCCESSTYPES__H__
+#define __EBASICATTACKSUCCESSTYPES__H__
+
+#include <cstdint>
+
+enum class eBasicAttackSuccessTypes : uint8_t {
+	SUCCESS = 1,
+	FAILARMOR,
+	FAILIMMUNE
+};
+
+#endif  //!__EBASICATTACKSUCCESSTYPES__H__

dGame/dBehaviors/BasicAttackBehavior.cpp

@@ -5,7 +5,7 @@
 #include "EntityManager.h"
 #include "DestroyableComponent.h"
 #include "BehaviorContext.h"
-
+#include "eBasicAttackSuccessTypes.h"
 
 void BasicAttackBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
 	if (context->unmanaged) {
@@ -31,130 +31,120 @@ void BasicAttackBehavior::Handle(BehaviorContext* context, RakNet::BitStream* bi
 	}
 	Game::logger->LogDebug("BasicAttackBehavior", "Number of allocated bits %i", allocatedBits);
 	const auto baseAddress = bitStream->GetReadOffset();
+
+	DoHandleBehavior(context, bitStream, branch);
+
+	bitStream->SetReadOffset(baseAddress + allocatedBits);
+}
+
+void BasicAttackBehavior::DoHandleBehavior(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
+	auto* targetEntity = EntityManager::Instance()->GetEntity(branch.target);
+	if (!targetEntity) {
+		Game::logger->Log("BasicAttackBehavior", "Target targetEntity %i not found.", branch.target);
+		return;
+	}
+
+	auto* destroyableComponent = targetEntity->GetComponent<DestroyableComponent>();
+	if (!destroyableComponent) {
+		Game::logger->Log("BasicAttackBehavior", "No destroyable found on the obj/lot %llu/%i", branch.target, targetEntity->GetLOT());
+		return;
+	}
+
 	bool isBlocked{};
 	bool isImmune{};
 	bool isSuccess{};
 
 	if (!bitStream->Read(isBlocked)) {
-		Game::logger->LogDebug("BasicAttackBehavior", "Unable to read isBlocked");
+		Game::logger->Log("BasicAttackBehavior", "Unable to read isBlocked");
 		return;
 	}
 
-	if (isBlocked) return;
+	if (isBlocked) {
+		destroyableComponent->SetAttacksToBlock(std::min(destroyableComponent->GetAttacksToBlock() - 1, 0U));
+		EntityManager::Instance()->SerializeEntity(targetEntity);
+		this->m_OnFailBlocked->Handle(context, bitStream, branch);
+		return;
+	}
 
 	if (!bitStream->Read(isImmune)) {
-		Game::logger->LogDebug("BasicAttackBehavior", "Unable to read isImmune");
+		Game::logger->Log("BasicAttackBehavior", "Unable to read isImmune");
 		return;
 	}
 
-	if (isImmune) return;
-
-	if (bitStream->Read(isSuccess) && isSuccess) { // Success
-		uint32_t unknown{};
-		if (!bitStream->Read(unknown)) {
-			Game::logger->LogDebug("BasicAttackBehavior", "Unable to read unknown");
+	if (isImmune) {
+		this->m_OnFailImmune->Handle(context, bitStream, branch);
 		return;
 	}
 
-		uint32_t damageDealt{};
-		if (!bitStream->Read(damageDealt)) {
-			Game::logger->LogDebug("BasicAttackBehavior", "Unable to read damageDealt");
+	if (!bitStream->Read(isSuccess)) {
+		Game::logger->Log("BasicAttackBehavior", "failed to read success from bitstream");
 		return;
 	}
 
-		// A value that's too large may be a cheating attempt, so we set it to MIN too
-		if (damageDealt > this->m_MaxDamage || damageDealt < this->m_MinDamage) {
-			damageDealt = this->m_MinDamage;
+	if (isSuccess) {
+		uint32_t armorDamageDealt{};
+		if (!bitStream->Read(armorDamageDealt)) {
+			Game::logger->Log("BasicAttackBehavior", "Unable to read armorDamageDealt");
+			return;
+		}
+
+		uint32_t healthDamageDealt{};
+		if (!bitStream->Read(healthDamageDealt)) {
+			Game::logger->Log("BasicAttackBehavior", "Unable to read healthDamageDealt");
+			return;
+		}
+
+		uint32_t totalDamageDealt = armorDamageDealt + healthDamageDealt;
+
+		// A value that's too large may be a cheating attempt, so we set it to MIN
+		if (totalDamageDealt > this->m_MaxDamage) {
+			totalDamageDealt = this->m_MinDamage;
 		}
 
-		auto* entity = EntityManager::Instance()->GetEntity(branch.target);
 		bool died{};
 		if (!bitStream->Read(died)) {
-			Game::logger->LogDebug("BasicAttackBehavior", "Unable to read died");
+			Game::logger->Log("BasicAttackBehavior", "Unable to read died");
 			return;
 		}
-
-		if (entity != nullptr) {
-			auto* destroyableComponent = entity->GetComponent<DestroyableComponent>();
-			if (destroyableComponent != nullptr) {
-				PlayFx(u"onhit", entity->GetObjectID());
-				destroyableComponent->Damage(damageDealt, context->originator, context->skillID);
-			}
-		}
+		auto previousArmor = destroyableComponent->GetArmor();
+		auto previousHealth = destroyableComponent->GetHealth();
+		PlayFx(u"onhit", targetEntity->GetObjectID());
+		destroyableComponent->Damage(totalDamageDealt, context->originator, context->skillID);
 	}
 
 	uint8_t successState{};
 	if (!bitStream->Read(successState)) {
-		Game::logger->LogDebug("BasicAttackBehavior", "Unable to read success state");
+		Game::logger->Log("BasicAttackBehavior", "Unable to read success state");
 		return;
 	}
 
-	switch (successState) {
-	case 1:
+	switch (static_cast<eBasicAttackSuccessTypes>(successState)) {
+	case eBasicAttackSuccessTypes::SUCCESS:
 		this->m_OnSuccess->Handle(context, bitStream, branch);
 		break;
+	case eBasicAttackSuccessTypes::FAILARMOR:
+		this->m_OnFailArmor->Handle(context, bitStream, branch);
+		break;
 	default:
-		Game::logger->LogDebug("BasicAttackBehavior", "Unknown success state (%i)!", successState);
+		if (static_cast<eBasicAttackSuccessTypes>(successState) != eBasicAttackSuccessTypes::FAILIMMUNE) {
+			Game::logger->Log("BasicAttackBehavior", "Unknown success state (%i)!", successState);
+			return;
+		}
+		this->m_OnFailImmune->Handle(context, bitStream, branch);
 		break;
 	}
-
-	bitStream->SetReadOffset(baseAddress + allocatedBits);
 }
 
 void BasicAttackBehavior::Calculate(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
-	auto* self = EntityManager::Instance()->GetEntity(context->originator);
-	if (self == nullptr) {
-		Game::logger->LogDebug("BasicAttackBehavior", "Invalid self entity (%llu)!", context->originator);
-		return;
-	}
-
 	bitStream->AlignWriteToByteBoundary();
 
 	const auto allocatedAddress = bitStream->GetWriteOffset();
 
-	bitStream->Write(uint16_t(0));
+	bitStream->Write<uint16_t>(0);
 
 	const auto startAddress = bitStream->GetWriteOffset();
 
-	bitStream->Write0(); // Blocked
-	bitStream->Write0(); // Immune
-	bitStream->Write1(); // Success
-
-	if (true) {
-		uint32_t unknown3 = 0;
-		bitStream->Write(unknown3);
-
-		auto damage = this->m_MinDamage;
-		auto* entity = EntityManager::Instance()->GetEntity(branch.target);
-
-		if (entity == nullptr) {
-			damage = 0;
-			bitStream->Write(damage);
-			bitStream->Write(false);
-		} else {
-			bitStream->Write(damage);
-			bitStream->Write(true);
-
-			auto* destroyableComponent = entity->GetComponent<DestroyableComponent>();
-			if (damage != 0 && destroyableComponent != nullptr) {
-				PlayFx(u"onhit", entity->GetObjectID(), 1);
-				destroyableComponent->Damage(damage, context->originator, context->skillID, false);
-				context->ScheduleUpdate(branch.target);
-			}
-		}
-	}
-
-	uint8_t successState = 1;
-	bitStream->Write(successState);
-
-	switch (successState) {
-	case 1:
-		this->m_OnSuccess->Calculate(context, bitStream, branch);
-		break;
-	default:
-		Game::logger->LogDebug("BasicAttackBehavior", "Unknown success state (%i)!", successState);
-		break;
-	}
+	DoBehaviorCalculation(context, bitStream, branch);
 
 	const auto endAddress = bitStream->GetWriteOffset();
 	const uint16_t allocate = endAddress - startAddress + 1;
@@ -164,6 +154,87 @@ void BasicAttackBehavior::Calculate(BehaviorContext* context, RakNet::BitStream*
 	bitStream->SetWriteOffset(startAddress + allocate);
 }
 
+void BasicAttackBehavior::DoBehaviorCalculation(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) {
+	auto* targetEntity = EntityManager::Instance()->GetEntity(branch.target);
+	if (!targetEntity) {
+		Game::logger->Log("BasicAttackBehavior", "Target entity %llu is null!", branch.target);
+		return;
+	}
+
+	auto* destroyableComponent = targetEntity->GetComponent<DestroyableComponent>();
+	if (!destroyableComponent || !destroyableComponent->GetParent()) {
+		Game::logger->Log("BasicAttackBehavior", "No destroyable component on %llu", branch.target);
+		return;
+	}
+
+	const bool isBlocking = destroyableComponent->GetAttacksToBlock() > 0;
+
+	bitStream->Write(isBlocking);
+
+	if (isBlocking) {
+		destroyableComponent->SetAttacksToBlock(destroyableComponent->GetAttacksToBlock() - 1);
+		EntityManager::Instance()->SerializeEntity(targetEntity);
+		this->m_OnFailBlocked->Calculate(context, bitStream, branch);
+		return;
+	}
+
+	const bool isImmune = destroyableComponent->IsImmune();
+
+	bitStream->Write(isImmune);
+
+	if (isImmune) {
+		this->m_OnFailImmune->Calculate(context, bitStream, branch);
+		return;
+	}
+
+	bool isSuccess = false;
+	const uint32_t previousHealth = destroyableComponent->GetHealth();
+	const uint32_t previousArmor = destroyableComponent->GetArmor();
+
+	const auto damage = this->m_MinDamage;
+
+	PlayFx(u"onhit", targetEntity->GetObjectID(), 1);
+	destroyableComponent->Damage(damage, context->originator, context->skillID, false);
+	context->ScheduleUpdate(branch.target);
+
+	const uint32_t armorDamageDealt = previousArmor - destroyableComponent->GetArmor();
+	const uint32_t healthDamageDealt = previousHealth - destroyableComponent->GetHealth();
+	isSuccess = armorDamageDealt > 0 || healthDamageDealt > 0 || (armorDamageDealt + healthDamageDealt) > 0;
+
+	bitStream->Write(isSuccess);
+
+	eBasicAttackSuccessTypes successState = eBasicAttackSuccessTypes::FAILIMMUNE;
+	if (isSuccess) {
+		if (healthDamageDealt >= 1) {
+			successState = eBasicAttackSuccessTypes::SUCCESS;
+		} else if (armorDamageDealt >= 1) {
+			successState = this->m_OnFailArmor->m_templateId == BehaviorTemplates::BEHAVIOR_EMPTY ? eBasicAttackSuccessTypes::FAILIMMUNE : eBasicAttackSuccessTypes::FAILARMOR;
+		}
+
+		bitStream->Write(armorDamageDealt);
+		bitStream->Write(healthDamageDealt);
+		bitStream->Write(targetEntity->GetIsDead());
+	}
+
+	bitStream->Write(successState);
+
+	switch (static_cast<eBasicAttackSuccessTypes>(successState)) {
+	case eBasicAttackSuccessTypes::SUCCESS:
+		this->m_OnSuccess->Calculate(context, bitStream, branch);
+		break;
+	case eBasicAttackSuccessTypes::FAILARMOR:
+		this->m_OnFailArmor->Calculate(context, bitStream, branch);
+		break;
+	default:
+		if (static_cast<eBasicAttackSuccessTypes>(successState) != eBasicAttackSuccessTypes::FAILIMMUNE) {
+			Game::logger->Log("BasicAttackBehavior", "Unknown success state (%i)!", successState);
+			break;
+		}
+		this->m_OnFailImmune->Calculate(context, bitStream, branch);
+		break;
+	}
+}
+
 void BasicAttackBehavior::Load() {
 	this->m_MinDamage = GetInt("min damage");
 	if (this->m_MinDamage == 0) this->m_MinDamage = 1;
@@ -171,7 +242,14 @@ void BasicAttackBehavior::Load() {
 	this->m_MaxDamage = GetInt("max damage");
 	if (this->m_MaxDamage == 0) this->m_MaxDamage = 1;
 
+	// The client sets the minimum damage to maximum, so we'll do the same.  These are usually the same value anyways.
+	if (this->m_MinDamage < this->m_MaxDamage) this->m_MinDamage = this->m_MaxDamage;
+
 	this->m_OnSuccess = GetAction("on_success");
 
 	this->m_OnFailArmor = GetAction("on_fail_armor");
+
+	this->m_OnFailImmune = GetAction("on_fail_immune");
+
+	this->m_OnFailBlocked = GetAction("on_fail_blocked");
 }

dGame/dBehaviors/BasicAttackBehavior.h

@@ -7,10 +7,45 @@ public:
 	explicit BasicAttackBehavior(const uint32_t behaviorId) : Behavior(behaviorId) {
 	}
 
+	/**
+	 * @brief Reads a 16bit short from the bitStream and when the actual behavior handling finishes with all of its branches, the bitStream
+	 * is then offset to after the allocated bits for this stream.
+	 * 
+	 */
+	void DoHandleBehavior(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch);
+
+	/**
+	 * @brief Handles a client initialized Basic Attack Behavior cast to be deserialized and verified on the server.
+	 * 
+	 * @param context The Skill's Behavior context.  All behaviors in the same tree share the same context 
+	 * @param bitStream The bitStream to deserialize.  BitStreams will always check their bounds before reading in a behavior
+	 * and will fail gracefully if an overread is detected.
+	 * @param branch The context of this specific branch of the Skill Behavior.  Changes based on which branch you are going down.
+	 */
 	void Handle(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) override;
 
+	/**
+	 * @brief Writes a 16bit short to the bitStream and when the actual behavior calculation finishes with all of its branches, the number
+	 * of bits used is then written to where the 16bit short initially was.
+	 * 
+	 */
 	void Calculate(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch) override;
 
+	/**
+	 * @brief Calculates a server initialized Basic Attack Behavior cast to be serialized to the client
+	 * 
+	 * @param context The Skill's Behavior context.  All behaviors in the same tree share the same context
+	 * @param bitStream The bitStream to serialize to.
+	 * @param branch The context of this specific branch of the Skill Behavior.  Changes based on which branch you are going down.
+	 */
+	void DoBehaviorCalculation(BehaviorContext* context, RakNet::BitStream* bitStream, BehaviorBranchContext branch);
+
+	/**
+	 * @brief Loads this Behaviors parameters from the database.  For this behavior specifically:
+	 * max and min damage will always be the same.  If min is less than max, they are both set to max.
+	 * If an action is not in the database, then no action is taken for that result.
+	 * 
+	 */
 	void Load() override;
 private:
 	uint32_t m_MinDamage;
@@ -20,4 +55,8 @@ private:
 	Behavior* m_OnSuccess;
 
 	Behavior* m_OnFailArmor;
+
+	Behavior* m_OnFailImmune;
+
+	Behavior* m_OnFailBlocked;
 };