Fix #452 - implement skill bonus caps. Fix for conversation crash with CloseItemConversation. Attempts to address spawns being called during deletion like RemovePet

EQ2/source/WorldServer/Combat.cpp

@@ -707,6 +707,11 @@ int8 Entity::DetermineHit(Spawn* victim, int8 type, int8 damage_type, float ToHi
 				skillAddedByWeapon += item_stat_weapon_skill;
 			}
 			MStats.unlock();
+			
+			float max_bonus_skill = GetRuleSkillMaxBonus();
+			if(skillAddedByWeapon > max_bonus_skill) {
+				skillAddedByWeapon = max_bonus_skill;
+			}
 		}
 	}
 	
@@ -1250,13 +1255,10 @@ bool Entity::CheckInterruptSpell(Entity* attacker) {
 	int8 percent = rule_manager.GetGlobalRule(R_Spells, NoInterruptBaseChance)->GetInt32();
 	Skill* skill = GetSkillByName("Focus", true);
 
-	float focusSkillPts = 0.0f;
-	MStats.lock();
-	focusSkillPts = stats[ITEM_STAT_FOCUS];
-	MStats.unlock();
+	float focus_skill_with_bonus = CalculateSkillWithBonus("Focus", ITEM_STAT_FOCUS, true);
 
-	if(skill)
-		percent += ((skill->current_val + 1 + focusSkillPts)/6);
+	percent += ((1 + focus_skill_with_bonus)/6);
+	
 	if(MakeRandomInt(1, 100) > percent) {
 		LogWrite(COMBAT__DEBUG, 0, "Combat", "'%s' interrupted spell for '%s': %i%%", attacker->GetName(), GetName(), percent);
 		GetZone()->Interrupted(this, attacker, SPELL_ERROR_INTERRUPTED);

EQ2/source/WorldServer/Entity.cpp

@@ -1209,12 +1209,16 @@ void Entity::SetMaxSpeed(float val){
 float Entity::CalculateSkillStatChance(char* skillName, int16 item_stat, float max_cap, float modifier, bool add_to_skill)
 {
 	float skillAndItemsChance = 0.0f;
-
+	float maxBonusCap = (float)GetLevel()*rule_manager.GetGlobalRule(R_Combat, MaxSkillBonusByLevel)->GetFloat();
 	Skill* skill = GetSkillByName(skillName, false);
 	if(skill){
 		MStats.lock();
 		float item_chance_or_skill = stats[item_stat];
 		MStats.unlock();
+		if(item_chance_or_skill > maxBonusCap) {
+			item_chance_or_skill = maxBonusCap;
+		}
+		
 		if(add_to_skill)
 		{
 			skillAndItemsChance = (((float)skill->current_val+item_chance_or_skill)/10.0f); // do we know 25 is accurate?  10 gives more 'skill' space, most cap at 70% with items
@@ -1223,6 +1227,9 @@ float Entity::CalculateSkillStatChance(char* skillName, int16 item_stat, float m
 		{
 			skillAndItemsChance = ((float)skill->current_val/10.0f); // do we know 25 is accurate?  10 gives more 'skill' space, most cap at 70% with items
 
+			if(modifier > maxBonusCap) {
+				modifier = maxBonusCap;
+			}
 			// take chance percentage and add the item stats % (+1 = 1% or .01f)
 			skillAndItemsChance += (skillAndItemsChance*((item_chance_or_skill + modifier)/100.0f));
 		}
@@ -1234,6 +1241,32 @@ float Entity::CalculateSkillStatChance(char* skillName, int16 item_stat, float m
 	return skillAndItemsChance;
 }
 
+float Entity::CalculateSkillWithBonus(char* skillName, int16 item_stat, bool chance_skill_increase)
+{
+	float skillAndItemsChance = 0.0f;
+	float maxBonusCap = GetRuleSkillMaxBonus();
+	
+	Skill* skill = GetSkillByName(skillName, chance_skill_increase);
+	if(skill){
+		float item_chance_or_skill = 0.0f;
+		if(item_stat != 0xFFFF) {
+			MStats.lock();
+			item_chance_or_skill = stats[item_stat];
+			MStats.unlock();
+		}
+		if(item_chance_or_skill > maxBonusCap)  { // would we be using their effective mentored level or actual level?
+			item_chance_or_skill = maxBonusCap;
+		}
+		skillAndItemsChance = skill->current_val+item_chance_or_skill;
+	}
+
+	return skillAndItemsChance;
+}
+
+float Entity::GetRuleSkillMaxBonus() {
+	return (float)GetLevel()*rule_manager.GetGlobalRule(R_Combat, MaxSkillBonusByLevel)->GetFloat();
+}
+
 void Entity::CalculateBonuses(){
 	if(lua_interface->IsLuaSystemReloading())
 		return;

EQ2/source/WorldServer/Entity.h

@@ -1273,6 +1273,8 @@ public:
 
 	bool IsEntity(){ return true; }
 	float CalculateSkillStatChance(char* skill, int16 item_stat, float max_cap = 0.0f, float modifier = 0.0f, bool add_to_skill = false);
+	float CalculateSkillWithBonus(char* skillName, int16 item_stat, bool chance_skill_increase);
+	float GetRuleSkillMaxBonus();
 	void CalculateBonuses();
 	void SetRegenValues(int16 effective_level);
 	float CalculateBonusMod();

EQ2/source/WorldServer/PlayerGroups.h

@@ -25,12 +25,23 @@ along with EQ2Emulator.  If not, see <http://www.gnu.org/licenses/>.
 #include <map>
 #include <mutex>
 #include <shared_mutex>
+#include "Spells.h"
 
 #include "../common/types.h"
 #include "Entity.h"
 
 using namespace std;
 
+// GroupOptions isn't used yet
+struct GroupOptions{
+	int8	loot_method;
+	int8	loot_items_rarity;
+	int8	auto_split;
+	int8	default_yell;
+	int8	group_autolock;
+	int8	solo_autolock;
+};
+
 /// <summary>All the generic info for the group window, plus a client pointer for players</summary>
 struct GroupMemberInfo {
 	int32	group_id;

EQ2/source/WorldServer/Rules/Rules.cpp

@@ -246,6 +246,7 @@ void RuleManager::Init()
 	RULE_INIT(R_Combat, MaxMitigationAllowedPVP, ".75"); // percentage max mitigation allowed, eg. 75% of damage can be mitigated max in PVP
 	RULE_INIT(R_Combat, StrengthNPC, "10"); // divider for strength NPC only str/x = additional dmg to low/high dmg
 	RULE_INIT(R_Combat, StrengthOther, "25"); // divider for strength other than NPC str/x = additional dmg to low/high dmg
+	RULE_INIT(R_Combat, MaxSkillBonusByLevel, "1.5"); // Level * 1.5 = max bonus skill allowed
 
 	/* SPAWN */
 	RULE_INIT(R_Spawn, SpeedMultiplier, "300"); // note: this value was 1280 until 6/1/2009, then was 600 til Sep 2009, when it became 300...?

EQ2/source/WorldServer/Rules/Rules.h

@@ -105,6 +105,7 @@ enum RuleType {
 	MaxMitigationAllowedPVP,
 	StrengthNPC,
 	StrengthOther,
+	MaxSkillBonusByLevel,
 
 	/* SPAWN */
 	SpeedMultiplier,

EQ2/source/WorldServer/Skills.cpp

@@ -487,7 +487,8 @@ void PlayerSkillList::AddSkillBonus(int32 spell_id, int32 skill_id, float value)
 		}
 		else
 			sb = skill_bonus_list.Get(spell_id);
-		if (sb->skills[skill_id] == 0) {
+		
+		if (sb->skills[skill_id] == nullptr) {
 			SkillBonusValue* sbv = new SkillBonusValue;
 			sbv->skill_id = skill_id;
 			sbv->value = value;

EQ2/source/WorldServer/World.h

@@ -154,38 +154,6 @@ struct LocationTransportDestination{
 	int32	faction_value;
 };
 
-//ideally we wouldn't need to store this information as we could get it from the Client object, 
-//however since the client object disconnects from the server when zoning we can't count on it being available
-
-/*struct PlayerGroup;
-struct GroupOptions{
-	int8	loot_method;
-	int8	loot_items_rarity;
-	int8	auto_split;
-	int8	default_yell;
-	int8	group_autolock;
-	int8	solo_autolock;
-};
-struct GroupMemberInfo{
-	string	name;
-	string	zone;
-	sint32	hp_current;
-	sint32	hp_max;
-	sint32	power_current;
-	sint32	power_max;
-	int16	level_current;
-	int16	level_max;
-	int8	race_id;
-	int8	class_id;
-	Client*	client;
-	PlayerGroup* group;
-};
-
-struct PlayerGroup{
-	deque<GroupMemberInfo*> members;
-	GroupOptions options;
-};*/
-
 struct LottoPlayer {
 	int32 end_time;
 	int8 num_matches;

EQ2/source/WorldServer/client.cpp

@@ -1599,12 +1599,9 @@ bool Client::HandlePacket(EQApplicationPacket* app) {
 			break;
 			}*/
 			float safe_height = 13.0f;
-			Skill* skill = GetPlayer()->GetSkillByName("Safe Fall", true);
-			GetPlayer()->MStats.lock();
-			float deflectionStat = GetPlayer()->stats[ITEM_STAT_SAFE_FALL];
-			GetPlayer()->MStats.unlock();
-			if (skill)
-				safe_height += (skill->current_val + 1 + deflectionStat) / 5;
+			float safe_skill_with_bonus = GetPlayer()->CalculateSkillWithBonus("Safe Fall", ITEM_STAT_SAFE_FALL, true);
+			if (safe_skill_with_bonus > 0.0f)
+				safe_height += (1 + safe_skill_with_bonus) / 5;
 
 			if (height > safe_height) {
 				int16 damage = (int16)ceil((height - safe_height) * 125);
@@ -11674,8 +11671,14 @@ void Client::HandleDialogSelectMsg(int32 conversation_id, int32 response_index)
 		}
 		
 		if (conversation_map.count(conversation_id) > 0 && conversation_map[conversation_id].count(response_index) > 0) {
-			if (spawn)
-				GetCurrentZone()->CallSpawnScript(spawn, SPAWN_SCRIPT_CONVERSATION, player, conversation.c_str());
+			if (spawn) {
+				if(conversation == "CloseItemConversation") {
+					LogWrite(LUA__ERROR, 0, "LUA", "CloseItemConversation is an invalid function call for this conversation with spawn id %u", spawn_id);
+				}
+				else {
+					GetCurrentZone()->CallSpawnScript(spawn, SPAWN_SCRIPT_CONVERSATION, player, conversation.c_str());
+				}
+			}
 			else if (item && lua_interface && item->GetItemScript())
 				lua_interface->RunItemScript(item->GetItemScript(), conversation.c_str(), item, player);
 			else

EQ2/source/WorldServer/zoneserver.cpp

@@ -1294,6 +1294,8 @@ void ZoneServer::DeleteSpawns(bool delete_all) {
 				spawn_delete_list.erase(erase_itr);
 				
 				MSpawnList.writelock(__FUNCTION__, __LINE__);
+				lua_interface->SetLuaUserDataStale(spawn);
+				
 				std::map<int32, Spawn*>::iterator sitr = spawn_list.find(spawn->GetID());
 				if(sitr != spawn_list.end()) {
 					spawn_list.erase(sitr);
@@ -1313,10 +1315,7 @@ void ZoneServer::DeleteSpawns(bool delete_all) {
 					}
 					housing_spawn_map.erase(spawn->GetID());
 				}
-				
 				MSpawnList.releasewritelock(__FUNCTION__, __LINE__);
-			
-				lua_interface->SetLuaUserDataStale(spawn);
 				
 				safe_delete(spawn);
 			}