azerothcore / mod-autobalance

Module for AzerothCore(MaNGOS -> TrinityCore -> SunwellCore)
http://www.azerothcore.org
103 stars 109 forks source link

Bug: AutoBalance + NPCBots: Bots joining BG cause segmentation fault #148

Closed podmepodme closed 1 year ago

podmepodme commented 1 year ago

Current Behaviour

With this config settings for NPC Bots

NpcBot.WanderingBots.Continents.Count = 250 (so there are plenty bots to join to bg) NpcBot.WanderingBots.BG.Enable = 1

After clicking join battle in battleground selection menu, server crashes.

Expected Behaviour

Should not crash

Steps to reproduce the problem

Server

  1. Patch core with NPCBots
  2. Install autobalance module
  3. cmake and rebuild
  4. Edit worldserver.conf lines with description values
  5. Start ./worldserver

Game

  1. On character with lvl 10+ press H
  2. Select Battlegrounds
  3. Select Warsong Gulch and Join Battle

Extra Notes

I compiled the core with debug flags, and after the crash, these are the lines of the crashdump from which I have concluded that the problem is in the AutoBalance module.

#4 0x0000555555ab5f94 in DataMap::GetDefault<AutoBalanceMapInfo, 0> (this=0x98, k="AutoBalanceMapInfo") at /home/podmepodme/WoW/Ragnaros/azerothcore/src/common/Utilities/DataMap.h:53 #5 0x0000555555aa8fda in AddCreatureToMapData (creature=0x7fff8be43000, addToCreatureList=true, playerToExcludeFromChecks=0x0, forceRecalculation=false) at /home/podmepodme/WoW/Ragnaros/azerothcore/modules/mod-autobalance/src/AutoBalance.cpp:649 #6 0x0000555555ad77b6 in AutoBalance_AllCreatureScript::Creature_SelectLevel (this=0x7ffff49b6440, creature=0x7fff8be43000) at /home/podmepodme/WoW/Ragnaros/azerothcore/modules/mod-autobalance/src/AutoBalance.cpp:1897 #7 0x00005555570c38cb in ScriptMgr::Creature_SelectLevel(CreatureTemplate const*, Creature*)::$_3::operator()(AllCreatureScript*) const (this=0x7fffffff5878, script=0x7ffff49b6440) at /home/podmepodme/WoW/Ragnaros/azerothcore/src/server/game/Scripting/ScriptDefines/AllCreatureScript.cpp:55

It seems that I fixed it with the code below. I tested the changes in one BG and one dung.

Added 1896 line.

image

I have no idea if this "fix" is enough, so please give me feedback and if i should create PR.

AC rev. hash/commit

AzerothCore rev. e5fe783fb956+ 2023-08-02 18:01:04 +0000 (master branch) (Unix, Debug, Static)

Operating system

Ubuntu 22.04 jammy

Custom changes or Modules

podmepodme commented 1 year ago

included code wont fix issue, after death, bots wont respawn and their lvl is setted to 0

kjack9 commented 1 year ago

Are you using the NPCBots-specific version of mod-autobalance? My understanding is that you HAVE to use Trickerer's version to get them to play well together.

The PR changes you proposed don't actually do what you'd think - the IsRaid flag is inconsistent at best, and I've pretty much given up on trying to use it for anything, choosing instead to count the number of max players.

podmepodme commented 1 year ago

No, I have not been using this version of mod-autobalance. And I did not find anything to help me when I searched about this problem. However, I think that compatibility could be built in here, if it is not, please let me know why. I actually fixed it the next day by adding this code to methods of class AutoBalance_AllCreatureScript.

int creatureTemplateEntry = creature->GetCreatureTemplate()->Entry;
if (creatureTemplateEntry >= 70000 && creatureTemplateEntry <= 90000) {
    return;
} 

My goal was to disable NPCbot balancing. I found out that NPCbots creature template ids start at 70000. Closing the range at 90000 is generally not correct. This is because every time the world server is restarted, the npcbots ids increase. https://github.com/trickerer/Trinity-Bots/issues/607

I understand that this "solution" is a hackfix and should not be used. However, this solution is sufficient for my personal server (and maybe for someone else too).

I have little, almost no experience with Azerothcore, but if there is interest in adding this compatibility, I can try to find a better solution.

podmepodme commented 1 year ago

aahh sorry, you was responding to PR author.

kjack9 commented 1 year ago

aahh sorry, you was responding to PR author.

Hah! I didn't realize that you weren't both. I'll make comments on the PR itself then.

ben-of-codecraft commented 1 year ago

Are you using the NPCBots-specific version of mod-autobalance?

No, I am using NPCbots patch and base mod-rebalance.

It has worked fine for many weeks now on all 3 battlegrounds, all classic dungeons, and classic raids and we have run with varying party sizes.

My understanding of this mod is it modifies npcs in an instance scaling their stats and replacing them with new ones. Since NPC bots are creatures they are attempting to be scaled which breaks logic in there related to bracketing spawned npcs from botmgr.

I will close the PR and move to a mod-autobalance-npcbot instead of applying to base mod. One of the staff asked me to submit because it came up in support requests.

kjack9 commented 1 year ago

I think the changes I'm making next might fix this issue for you as a side effect. I'm removing the IsBattleground() checks entirely since AutoBalance shouldn't be doing anything (I can think of) in a BG anyways. It should ignore them.

If you're impatient to test, you can try out the new code (not everything is done yet, still WIP) here: https://github.com/kjack9/mod-autobalance/tree/9100569ccbc2a86cc118da256da41bb02599c5aa

ben-of-codecraft commented 1 year ago

I patched up the mod already, so it has been working fine for me for quite a while, so I am not waiting on anything.

The place where I could see PvP scaling of npc's based on player count is in Alterec Valley. The bosses and enemies in there are too tough even for a small number of 3-5 and the bots are too stupid to support you. So it becomes a difficult one to win through taking towers. Either I or my buddy will patch that up in our fork version specifically meant to work with NPC bots, so no worries.