(fix) Add null check, reduce load on map creation, fix incorrect max …

…health on some creatures (azerothcore#171)

* Only skip critters that are flavor-only (<= level 5)

* Refactor to break out some logic into separate functions.

* Refactor more repeated operations into separate functions
- Add worldHealthMultiplier, intended to be used to scale the health of attackable game objects in the instance

* Continued refinement of non-creature-based damage and healing

* Rework damage handling, improve coverage, debug logging
- tear out and re-implement all the damage handling to ensure more cases are handled
- use the map's damage modifier when damage happens where the source isn't a creature
- better handle when the source of damage no longer exists (logged out or despawned)
- extensive (optional) logging under the `module.AutoBalance.Damage` logging prefix
- WIP implementattion of game object damage handling (destructible buildings)
- fix a long-standing error in the class name for some logging

* World damage scaling is working, WIP
- need to apply statmodifiers still
- improved logging

* - many, many logging improvements
- fix a bug with the enable check case statements (stupid case breaks)
- remove antiquated battleground checks - should never scale in battlegrounds
- better handle "special" creatures (totems, triggers, critters)
- allow combat critters to scale correctly
- allow intentionally-low-level triggers and critters to remain low level
- workaround an issue with summons showing the wrong level client-side (level change ramp up)
- further improve GM and user commands
- erase map AB info from maps when they are created, preventing issues with instance ID re-use
- better handle instances with no non-GM characters (disable scaling temporarily)

* Further work on summoned creatures, still WIP
- removed unnecessary `std::string`s

* WIP

* Rewrite multiplier calculation, reduce required processing
- majory re-wrote the multiplicer calculations to increase accuracy and clarity
- take advantage of new `OnBeforeCreatureSelectLevel` hook to set the initial level of creatures before they are added to the world
- separated level-scaled and non-level-scaled multipliers for tracking and display
- move creature relevancy check to new function `isCreatureRelevant`
- rename many variables to standardize and clarify
- add option to force a map stats update, to be used when a player enters the map
- players leaving the map are no longer included in the map's stats
- sweeping improvements in logging of the multiplier calculation process (`AutoBalance.StatGeneration`)
- update `.ab creaturestat` to show new scaled modifiers in an understandable way

* Optimize map updates to only occur when necessary. WIP.

* Create documentation for Info classes, add player lists

* Reworking config tracking, WIP

* Rewrite player tracking to remove kludges, many improvements
- replace kludgy player count detection with an internal player list (more features coming Soon)
- skip scaling on player-owned summons, totems, etc
- skip scaling on flavor critters but not on combat critters
- separate map and global config times, reducing the work to refresh if only a map config changes
- centralize detection of "non-relevant" creatures, use a `skipMe` tag to speed up processing
- correctly handle a GM leaving the instance who was previously a non-GM when they entered
- temporarily disable in-combat checking
- improve logging format to produce the ID needed for `.go creature <UUID>`
- standardize logging format

* Performance improvements, fix statmodifier selection, logging
- moved enums to top the file, added `Relevance` enum
- improve performance of `isCreatureRelevant` but saving the decision from previous runs
- fix issues with summon detection to (hopefully) cover all edge cases
- consolidate and fix StatModifier case selection, add debugs
- more logging improvements all around

* Recount players on a config reload.

* Improve player enter/leave behavior.

* Improve handling of empty instances, properly scale trigger creatures, improve `.ab mapstat`

* Check that the target for `.ab creaturestat` is in a dungeon.

* Move boss detection to separate function
- boss summons now also count as a "boss"

* Do not modify spells that hurt the player but are intended to - Dark Runes, etc
- logging improvements

* Fix world multipliers not getting reloaded. Logging improvements.

* Additional debug for boss detection

* Fix percent-based damage auras, map multipliers honor level scaling
- percent-based damage auras will now ignore level scaling (per-player scaling only)
- map multipliers (damage/healing and health) will now honor the level scaling setting
- use a custom struct to store and move around world multipliers
- properly remove LevelScalingEndGameBoost until it can be fixed (azerothcore#156)
- creatures summoned by a boss will now be scaled like bosses
- further refinement of trigger handling logic
- fix active creature not being decremented on creature removal
- fix incorrect announcing of GMs entering/exiting the instance
- improvements for in-game commands
- continued log improvements

* World multipliers now honor stat modifiers
- logging improvements

* Re-enable the processing of map stats when the instance is empty

* Combat locking: WIP

* Combat locking - better implementation. WIP, needs notifications.

* Re-implement combat locking

* Complete migration from skipMe to isCreatureRelevant

* Code and log cleanup

* Handle a combatLockMinPlayers of 0

* Update bug report template to request AB-specific information

* Update README.md with commands, loggers, and min AC version

* Add logging settings to .conf.dist file. Small logging change.

* Replace uint with uint8.

* Rename `GetCurrentTime()` and remove case ranges to make Windows happy

* Code cleanup. Replace case statements. Remove `entry`, which is unneeded.

* (Hopefully) final code cleanup, comment refinement

* Fix enemy totems not level scaling. Update logging statement.

* Resolve issue with heroic dungeons that don't have LFG levels defined

* Fix dungeon difficulty normal, add error logging if no LFG found

* Fix incorrect heroic settings.
- fix display issue with active creatures

* Fix syntax error.
- that'll teach me to commit before I build

* Handle cloned summons, improve creaturestat command
- track summoner as a part of AutoBalanceCreatureInfo
- detect when a summon is a clone and use the correct current health instead of scaling again
- add summon information to `.ab creaturestat`
- as always, logging improvements

* Handle shared damage auras, add never modify spells
- spells with SPELL_AURA_SHARE_DAMAGE_PCT effects will now correctly be unmodified
- added `spellIdsToNeverModify` and correctly skip modification of them
- add "Twin Empathy" (1177) to the `spellIdsToNeverModify` list
- convert `_IsAuraPercentDamage` to `_isAuraWithEffectType` since this probably isn't the last I'll need it
- logging changes, naturally

* Move initial scaling from first OnAllCreatureUpdate to Creature_SelectLevel

* Tell user to move logging settings to `worldserver.conf`, since they don't work well in module config

* Fix combat locking.

* Small update to spellIdsToNeverModify

* Move warning to debug.

* Add creatureIDsThatAreNotClones. Fix crash.

* Check spellIdsToNeverModify for game object damage too

* Add important logging hint

* Attempt to resolve crash for non-standard class/race combinations

* Remove disabling instance when no non-GMs in the map. Add extra SetHealth/SetMaxHealth just before adding creatures to the world.
- Fixes azerothcore#168
- Fixes azerothcore#169

* Remove hack-ish health set. Set instance min of 1. Reduce unneeded calculations.

* Restore our modified max health just after the creature is added to the world, if necessary

* Fix mis-used of health function.

conf/AutoBalance.conf.dist

@@ -4,6 +4,7 @@
 # Logging
 #
 # Add these lines to your worldserver.conf file to enable logging for AutoBalance.
+# Be sure to add them after the `Logger.module=4,Console Server` line.
 #
 # 4 = Info (Default), 5 = Debug
 #

src/AutoBalance.cpp

@@ -171,6 +171,8 @@ class AutoBalanceCreatureInfo : public DataMap::Base
     bool isBrandNew = false;                        // whether or not the creature is brand new to the map (hasn't been added to the world yet)
     bool neverLevelScale = false;                   // whether or not the creature should never be level scaled (can still be player scaled)
 
+    uint32 initialMaxHealth = 0;                    // stored max health value to be applied just before being added to the world
+
     // creature->IsSummon()                         // whether or not the creature is a summon
     Creature* summoner = nullptr;                   // the creature that summoned this creature
     std::string summonerName = "";                  // the name of the creature that summoned this creature
@@ -615,9 +617,6 @@ bool ShouldMapBeEnabled(Map* map)
 {
     if (map->IsDungeon())
     {
-        // get the map's info
-        AutoBalanceMapInfo *mapABInfo = map->CustomData.GetDefault<AutoBalanceMapInfo>("AutoBalanceMapInfo");
-
         // if globally disabled, return false
         if (!EnableGlobal)
         {
@@ -655,20 +654,6 @@ bool ShouldMapBeEnabled(Map* map)
             return false;
         }
 
-        // if the map has no players in the player list, then disabled
-        if (!mapABInfo->playerCount)
-        {
-            LOG_DEBUG("module.AutoBalance", "AutoBalance::ShouldMapBeEnabled: {} ({}{}, {}-player {}) - Not enabled because there are no non-GM players in the map.",
-                      map->GetMapName(),
-                      map->GetId(),
-                      map->GetInstanceId() ? "-" + std::to_string(map->GetInstanceId()) : "",
-                      instanceMap->GetMaxPlayers(),
-                      instanceMap->IsHeroic() ? "Heroic" : "Normal"
-            );
-
-            return false;
-        }
-
         // if the Dungeon is disabled via configuration, do not enable it
         if (isDungeonInDisabledDungeonIds(map->GetId()))
         {
@@ -2546,7 +2531,8 @@ void UpdateMapPlayerStats(Map* map)
     );
 
     // update the player count
-    mapABInfo->playerCount = mapABInfo->allMapPlayers.size();
+    // minimum of 1 to prevent scaling weirdness when only GMs are in the instnace
+    mapABInfo->playerCount = mapABInfo->allMapPlayers.size() ? mapABInfo->allMapPlayers.size() : 1;
 
     LOG_DEBUG("module.AutoBalance", "AutoBalance::UpdateMapPlayerStats: Map {} ({}{}) | playerCount = ({}).",
         instanceMap->GetMapName(),
@@ -2664,9 +2650,8 @@ void UpdateMapPlayerStats(Map* map)
         mapABInfo->highestPlayerLevel = mapABInfo->lfgTargetLevel;
         mapABInfo->lowestPlayerLevel = mapABInfo->lfgTargetLevel;
 
-        // no non-GM players on the map, disable it
-        mapABInfo->enabled = false;
-        LOG_DEBUG("module.AutoBalance", "AutoBalance::UpdateMapPlayerStats: Map {} ({}{}, {}-player {}) | has no non-GM players. Disabling (potentially temporarily).",
+        // no non-GM players on the map
+        LOG_DEBUG("module.AutoBalance", "AutoBalance::UpdateMapPlayerStats: Map {} ({}{}, {}-player {}) | has no non-GM players. Player stats derived from LFG target level.",
             map->GetMapName(),
             map->GetId(),
             map->GetInstanceId() ? "-" + std::to_string(map->GetInstanceId()) : "",
@@ -3614,6 +3599,12 @@ class AutoBalance_PlayerScript : public PlayerScript
 
         virtual void OnPlayerEnterCombat(Player* player, Unit* /*enemy*/) override
         {
+            // if the player or their map is gone, return
+            if (!player || !player->GetMap())
+            {
+                return;
+            }
+
             Map* map = player->GetMap();
 
             // If this isn't a dungeon, no work to do
@@ -3650,6 +3641,12 @@ class AutoBalance_PlayerScript : public PlayerScript
 
         virtual void OnPlayerLeaveCombat(Player* player) override
         {
+            // if the player or their map is gone, return
+            if (!player || !player->GetMap())
+            {
+                return;
+            }
+
             Map* map = player->GetMap();
 
             // If this isn't a dungeon, no work to do
@@ -4298,7 +4295,7 @@ class AutoBalance_GameObjectScript : public AllGameObjectScript
             if (_debug_damage_and_healing) _Debug_Output("OnGameObjectModifyHealth", target, source, amount, "BEFORE:", spellInfo->SpellName[0], spellInfo->Id);
 
             // modify the amount
-            amount = _Modify_GameObject_Damage_Healing(target, source, amount);
+            amount = _Modify_GameObject_Damage_Healing(target, source, amount, spellInfo);
 
             if (_debug_damage_and_healing) _Debug_Output("OnGameObjectModifyHealth", target, source, amount, "AFTER:", spellInfo->SpellName[0], spellInfo->Id);
         }
@@ -4364,7 +4361,7 @@ class AutoBalance_GameObjectScript : public AllGameObjectScript
             }
         }
 
-        int32 _Modify_GameObject_Damage_Healing(GameObject* target, Unit* source, int32 amount)
+        int32 _Modify_GameObject_Damage_Healing(GameObject* target, Unit* source, int32 amount, SpellInfo const* spellInfo)
         {
             //
             // Pre-flight Checks
@@ -4397,6 +4394,29 @@ class AutoBalance_GameObjectScript : public AllGameObjectScript
                 return amount;
             }
 
+            // if the spell ID is in our "never modify" list, return the original value
+            if
+            (
+                spellInfo &&
+                spellInfo->Id &&
+                std::find
+                (
+                    spellIdsToNeverModify.begin(),
+                    spellIdsToNeverModify.end(),
+                    spellInfo->Id
+                ) != spellIdsToNeverModify.end()
+            )
+            {
+                if (_debug_damage_and_healing)
+                    LOG_DEBUG("module.AutoBalance_DamageHealingCC", "AutoBalance_UnitScript::_Modify_Damage_Healing: Spell {}({}) is in the never modify list, returning original value of ({}).",
+                        spellInfo->SpellName[0],
+                        spellInfo->Id,
+                        amount
+                    );
+
+                return amount;
+            }
+
             // get the map's info
             AutoBalanceMapInfo *targetMapABInfo = target->GetMap()->CustomData.GetDefault<AutoBalanceMapInfo>("AutoBalanceMapInfo");
 
@@ -4560,17 +4580,38 @@ class AutoBalance_AllMapScript : public AllMapScript
             // get the map's info
             AutoBalanceMapInfo *mapABInfo=map->CustomData.GetDefault<AutoBalanceMapInfo>("AutoBalanceMapInfo");
 
+            // store the previous difficulty for comparison later
+            int prevAdjustedPlayerCount = mapABInfo->adjustedPlayerCount;
+
             // add player to this map's player list
             AddPlayerToMap(map, player);
 
             // recalculate the zone's level stats
             mapABInfo->highestCreatureLevel = 0;
             mapABInfo->lowestCreatureLevel = 0;
-            mapABInfo->avgCreatureLevel = 0;
+            //mapABInfo->avgCreatureLevel = 0;
             mapABInfo->activeCreatureCount = 0;
 
-            // Update the map's data, forced
-            UpdateMapDataIfNeeded(map, true);
+            // if the previous player count is the same as the new player count, update without force
+            if (prevAdjustedPlayerCount == mapABInfo->adjustedPlayerCount)
+            {
+                LOG_DEBUG("module.AutoBalance", "AutoBalance_AllMapScript::OnPlayerEnterAll: Player difficulty unchanged at {}. Updating map data (no force).",
+                    mapABInfo->adjustedPlayerCount
+                );
+
+                // Update the map's data
+                UpdateMapDataIfNeeded(map, false);
+            }
+            else
+            {
+                LOG_DEBUG("module.AutoBalance", "AutoBalance_AllMapScript::OnPlayerEnterAll: Player difficulty changed from ({})->({}). Updating map data (force).",
+                    prevAdjustedPlayerCount,
+                    mapABInfo->adjustedPlayerCount
+                );
+
+                // Update the map's data, forced
+                UpdateMapDataIfNeeded(map, true);
+            }
 
             // see which existing creatures are active
             for (std::vector<Creature*>::iterator creatureIterator = mapABInfo->allMapCreatures.begin(); creatureIterator != mapABInfo->allMapCreatures.end(); ++creatureIterator)
@@ -4652,6 +4693,9 @@ class AutoBalance_AllMapScript : public AllMapScript
             // get the map's info
             AutoBalanceMapInfo *mapABInfo=map->CustomData.GetDefault<AutoBalanceMapInfo>("AutoBalanceMapInfo");
 
+            // store the previous difficulty for comparison later
+            int prevAdjustedPlayerCount = mapABInfo->adjustedPlayerCount;
+
             // remove this player from this map's player list
             bool playerWasRemoved = RemovePlayerFromMap(map, player);
 
@@ -4667,7 +4711,7 @@ class AutoBalance_AllMapScript : public AllMapScript
             // recalculate the zone's level stats
             mapABInfo->highestCreatureLevel = 0;
             mapABInfo->lowestCreatureLevel = 0;
-            mapABInfo->avgCreatureLevel = 0;
+            //mapABInfo->avgCreatureLevel = 0;
             mapABInfo->activeCreatureCount = 0;
 
             // see which existing creatures are active
@@ -4676,8 +4720,26 @@ class AutoBalance_AllMapScript : public AllMapScript
                 AddCreatureToMapCreatureList(*creatureIterator, false, true);
             }
 
-            // Update the map's data, forced
-            UpdateMapDataIfNeeded(map, true);
+            // if the previous player count is the same as the new player count, update without force
+            if (prevAdjustedPlayerCount == mapABInfo->adjustedPlayerCount)
+            {
+                LOG_DEBUG("module.AutoBalance", "AutoBalance_AllMapScript::OnPlayerLeaveAll: Player difficulty unchanged at {}. Updating map data (no force).",
+                    mapABInfo->adjustedPlayerCount
+                );
+
+                // Update the map's data
+                UpdateMapDataIfNeeded(map, false);
+            }
+            else
+            {
+                LOG_DEBUG("module.AutoBalance", "AutoBalance_AllMapScript::OnPlayerLeaveAll: Player difficulty changed from ({})->({}). Updating map data (force).",
+                    prevAdjustedPlayerCount,
+                    mapABInfo->adjustedPlayerCount
+                );
+
+                // Update the map's data, forced
+                UpdateMapDataIfNeeded(map, true);
+            }
 
             // updates the player count and levels for the map
             if (map->GetEntry() && map->GetEntry()->IsDungeon())
@@ -4863,6 +4925,9 @@ class AutoBalance_AllCreatureScript : public AllCreatureScript
 
             ModifyCreatureAttributes(creature);
 
+            // store the creature's max health value for validation in `OnCreatureAddWorld`
+            creatureABInfo->initialMaxHealth = creature->GetMaxHealth();
+
             AutoBalanceCreatureInfo *creatureABInfo=creature->CustomData.GetDefault<AutoBalanceCreatureInfo>("AutoBalanceCreatureInfo");
 
             if (creature->GetLevel() != creatureABInfo->selectedLevel && isCreatureRelevant(creature))
@@ -4892,15 +4957,36 @@ class AutoBalance_AllCreatureScript : public AllCreatureScript
             InstanceMap* instanceMap = creatureMap->ToInstanceMap();
             AutoBalanceCreatureInfo *creatureABInfo=creature->CustomData.GetDefault<AutoBalanceCreatureInfo>("AutoBalanceCreatureInfo");
 
-            // final check to be sure the creature is the right level
-            if (isCreatureRelevant(creature) && creature->GetLevel() != creatureABInfo->selectedLevel && !creature->IsSummon())
+            // final checks on the creature before spawning
+            if (isCreatureRelevant(creature))
             {
-                LOG_DEBUG("module.AutoBalance", "AutoBalance_AllCreatureScript::OnCreatureAddWorld: Creature {} ({}) | is set to level ({}) just after being added to the world.",
-                            creature->GetName(),
-                            creature->GetLevel(),
-                            creatureABInfo->selectedLevel
-                );
-                creature->SetLevel(creatureABInfo->selectedLevel);
+                // level check
+                if (creature->GetLevel() != creatureABInfo->selectedLevel && !creature->IsSummon())
+                {
+                    LOG_DEBUG("module.AutoBalance", "AutoBalance_AllCreatureScript::OnCreatureAddWorld: Creature {} ({}) | is set to level ({}) just after being added to the world.",
+                                creature->GetName(),
+                                creature->GetLevel(),
+                                creatureABInfo->selectedLevel
+                    );
+                    creature->SetLevel(creatureABInfo->selectedLevel);
+                }
+
+                // max health check
+                if (creature->GetMaxHealth() != creatureABInfo->initialMaxHealth)
+                {
+
+                    float oldMaxHealth = creature->GetMaxHealth();
+                    float healthPct = creature->GetHealthPct();
+                    creature->SetMaxHealth(creatureABInfo->initialMaxHealth);
+                    creature->SetHealth(creature->GetMaxHealth() * (healthPct / 100));
+
+                    LOG_DEBUG("module.AutoBalance", "AutoBalance_AllCreatureScript::OnCreatureAddWorld: Creature {} ({}) | had its max health changed from ({})->({}) just after being added to the world.",
+                                creature->GetName(),
+                                creature->GetLevel(),
+                                oldMaxHealth,
+                                creatureABInfo->initialMaxHealth
+                    );
+                }
             }
 
             LOG_DEBUG("module.AutoBalance", "AutoBalance_AllCreatureScript::OnCreatureAddWorld: Creature {} ({}) | added to map {} ({}{}{}{})",