trickerer / Trinity-Bots

NPCBots for TrinityCore and AzerothCore 3.3.5
https://github.com/trickerer/TrinityCore-3.3.5-with-NPCBots/
472 stars 156 forks source link

[TC] [Crash] [Core] Transmogrification compatibility, using a previous fix / change causes crash #303

Closed Logandros closed 1 year ago

Logandros commented 1 year ago

This is similar to a previously reported issue after applying Rochet2's Transmog code. I experienced the exact same issue as listed here: https://github.com/trickerer/Trinity-Bots/issues/101

However after making the changes to CharacterDatabase.cpp

Changing: PrepareStatement(CHAR_SEL_NPCBOT_EQUIP_BY_ITEM_INSTANCE, "SELECT creatorGuid, giftCreatorGuid, count, duration, charges, flags, enchantments, randomPropertyId, durability, playedTime, text, guid, itemEntry, owner_guid " "FROM item_instance WHERE guid IN (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", CONNECTION_SYNCH); To: PrepareStatement(CHAR_SEL_NPCBOT_EQUIP_BY_ITEM_INSTANCE, "SELECT creatorGuid, giftCreatorGuid, count, duration, charges, flags, enchantments, randomPropertyId, durability, playedTime, text, guid, itemEntry, owner_guid, 0, transmog " "FROM item_instance WHERE guid IN (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", CONNECTION_SYNCH);

and modifying botdatamgr.cpp to include stmt->setUInt32(++index, botitem->transmog); after stmt->setString(++index, botitem->GetText());

I now get the following error when I build"

3>botdatamgr.cpp 3>D:\BUILD STUFF\repos\TrinityWotlk\src\server\game\AI\NpcBots\botdatamgr.cpp(453,38): error C2248: 'Item::transmog': cannot access private member declared in class 'Item' 3>D:\BUILD STUFF\repos\TrinityWotlk\src\server\game\Entities\Item\Item.h(228,10): message : see declaration of 'Item::transmog' 3>D:\BUILD STUFF\repos\TrinityWotlk\src\server\game\Entities\Item\Item.h(61,19): message : see declaration of 'Item'

Any help would be appreciated, thanks in advance.

trickerer commented 1 year ago

You'll have to modify Item.h file. You can either play dirty and move 'transmog' field up to public section or write a public accessor:

    uint32 GetPaidMoney()...
    uint32 GetPaidExtendedCost()...
+    uint32 GetTransmog() const { return transmog; }
...

In this case you'll need to replace in botdatamgr.cpp botitem->transmog -> botitem->GetTransmog()

And fix the title please.

Logandros commented 1 year ago

Sorry about the title, I got lost in formatting and linking to be as descriptive as possible that I forgot an important part.

I tried not playing dirty and I modified Item.h

uint32 GetPaidMoney() const { return m_paidMoney; }
uint32 GetPaidExtendedCost() const { return m_paidExtendedCost; }
uint32 GetTransmog() const { return transmog; }

and changed botdatamgr.cpp to

stmt->setString(++index, botitem->GetText());
stmt->setUInt32(++index, botitem->GetTransmog());
stmt->setUInt32(++index, botitem->GetGUID().GetCounter());

But that caused more errors

D:\BUILD STUFF\repos\TrinityWotlk\src\server\game\Entities\Item\Item.h(206,16): error C2535: 'uint32 Item::GetTransmog(void) const': member function already defined or declared (compiling source file D:\BUILD STUFF\repos\TrinityWotlk\src\server\game\Entities\Item\Item.cpp)

So I played dirty and moved transmog to the public sections and reverted botdatamgr.cpp back to

stmt->setUInt32(++index, botitem->transmog);

and it built just fine without error. I'm just going to assume I misunderstand your public accessor method, as I'm quite the beginner.

trickerer commented 1 year ago

No, you didn't make a mistake. It's just that seems like the accessor was already there and you could just use it and only make changes to botdatamgr.cpp But no matter, glad you could figure this one out after all.

Logandros commented 1 year ago

So I'm not sure what I missed trickerer. I followed the previous post to the letter modifying both files which then allowed my server to build. However I'm back having the issue that the original author had. When I give an item to an npcbot, it creates an entry in item_instance with a '0' GUID which I assume causes the crash on the next restart. I had to remove the entry and I was able to start the server again. All very similar symptoms to the original poster. I dumped the sql of the problematic entry:

INSERT INTOitem_instance(guid,itemEntry,owner_guid,creatorGuid,giftCreatorGuid,count,duration,charges,flags,enchantments,randomPropertyId,durability,playedTime,text,transmog,enchant) VALUES (0, 13127, 3, 0, 4, 1, 0, '0 0 0 0 0 ', 0, '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 ', 0, 70, 0, '', 0, 396380);

As you can see GUID is 0. I'm guessing that crashes the server because if I restart without clearing that entry I get a crash at this point:

`Starting NpcBot system...

Bot appearance data loaded Bot race data loaded Bots transmog data is not loaded. Table characters_npcbot_transmog is empty! Loaded 36 bot data entries`

I'm not sure what I missed.

Logandros commented 1 year ago

Just to clarify, I've tried it both ways in botdatmangr.cpp

stmt->setUInt32(++index, botitem->GetTransmog()); stmt->setUInt32(++index, botitem->transmog);

and both builds will crash on reboot with the 0 GUID

Edit: Now that I'm looking at this a little longer. I'm not sure my issue is the same. It looks like the GUID (I assume) you are trying to pull in my case is not in transmog but in enchant. So I'm not sure if I have an error somewhere else that's causing my transmog to be stored in the wrong field or if I need to update npcbot to pull it from enchant?? I'm so very new at this so a lot of guessing on my part with 0 knowledge and just tying to read what I think it's doing.

trickerer commented 1 year ago

Please dump here your

.../server/database/Database/Implementation/CharacterDatabase.cpp:L149

(CHAR_REP_ITEM_INSTANCE)

Logandros commented 1 year ago

Mine is actually L155, probably due to the conflicts I had to resolve when adding the transmogrifier code, but here is what I got:

PrepareStatement(CHAR_REP_ITEM_INSTANCE, "REPLACE INTO item_instance (itemEntry, owner_guid, creatorGuid, giftCreatorGuid, count, duration, charges, flags, enchantments, randomPropertyId, durability, playedTime, text, transmog, enchant, guid) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", CONNECTION_ASYNC);

trickerer commented 1 year ago

So, after stmt->setUInt32(++index, botitem->transmog); (or GetTransmog()) you need to add another line:

stmt->setUInt32(++index, botitem->enchant);

It could also be GetEnchant() or something, look it up in your Item.h. Oh and make sure its type is uint32

Logandros commented 1 year ago

here is the code from Item.h

uint32 transmog = 0; uint32 enchant = 0;

So should I just add

stmt->setUInt32(++index, botitem->enchant);

Edit: That was the code I played dirty with earlier in the ticket and moved to the public area

trickerer commented 1 year ago

I recommend you look for existing accessor first (ecnhant could be a private field just like transmog). Yes you can do just that then.

BTW do you have that random echant mod thing? Is it a separate patch or integrated into transmog?

Logandros commented 1 year ago

I do have that random enchant thing, I added via the Eluna LUA, I did not merge it in to the core. Server built fine but crashes at the same spot, after 'Loaded 36 bot data entries' if I clear the 1 entry in the DB the next line should be 'Spawned 36 npcbot(s) within 1 grid(s) in 26 ms' but still fails with the new code.

Logandros commented 1 year ago

I can gladly remove that LUA script to see if that is what is causing this, I certainly can go without that, if you think that is the issue.

For the heck of it, I wiped out the enchant data in the DB but that didn't seem to help or matter. It seems only deleting the 0 GUID entry allows the server to start.

trickerer commented 1 year ago

See here: https://github.com/Rochet2/TrinityCore/blob/transmog_3.3.5/src/server/database/Database/Implementation/CharacterDatabase.cpp#L149 line 149

The only difference in prepared statement from yours is that enchant column, so you have something else in your core besides transmog

https://github.com/Rochet2/TrinityCore/blob/transmog_3.3.5/src/server/game/Entities/Item/Item.cpp#L364 lines 364-366 Compare these lines to yours, generally you need to make botdatamgr.cpp arguments uniform with your version of those lines

essentially last arguments must be: text, transmog, enchant, guid. Text and guid are already there.

Logandros commented 1 year ago

The only difference I see in my item.cpp versus botdatamgr.cp is that item.cpp has stmt->setUInt32(++index, transmog); stmt->setUInt32(++index, enchant); stmt->setUInt32(++index, guid);

botdatamgr.cpp has stmt->setString(++index, botitem->GetText()); stmt->setUInt32(++index, botitem->transmog); stmt->setUInt32(++index, botitem->GetGUID().GetCounter());

trickerer commented 1 year ago

That's what I was talking about: that stmt->setUInt32(++index, botitem->enchant); must be there

stmt->setString(++index, botitem->GetText());
stmt->setUInt32(++index, botitem->transmog);
+stmt->setUInt32(++index, botitem->enchant);
stmt->setUInt32(++index, botitem->GetGUID().GetCounter());
Logandros commented 1 year ago

Yeah, I added that earlier. Built without issue but still crashed after loading the bots and I guess while attempting to spawn them. Having the enchant stmt in or out didn't resolve the issue.

trickerer commented 1 year ago

It won't help with existing item_instance records having guid = 0, you have to erase them. But new items should have guid set correctly

Logandros commented 1 year ago

OK I will wipe it out and try to see if I can duplicate the problem now that it should be resolved going forward and report back and close the ticket.... wish I could send you a beer for all your free time and work on the project. We love the npc bots in my household!

trickerer commented 1 year ago

You can also swap enchant and guid for those entries

Logandros commented 1 year ago

Amazing support as always trickerer, thank you for helping me clean this up so that it all works together!

Logandros commented 1 year ago

Seems I jumped the gun. Although it was now correctly updating GUID, and oddly I did shutdown and reboot without issues. It seems hours later when I needed to restart, it crashed again loading the bots and trying to spawn them. What I noticed this time is that the bot that had the item I had given them had a value in characters_npcbot / equipShoulders (389432) which were just some random shoulders I had given him this morning to test. In order to get the server to start I had to 0 that entry out. So something still doesn't like me giving my bot an item...

trickerer commented 1 year ago

You need to build in Debug mode to get a crashlog which will show where the problem is.

Logandros commented 1 year ago

Is it ok for me to link the entire log here? I can use pastebin, or is there private stuff in there I need to remove?

Edit: or do you only need to see a specific section of the log?

trickerer commented 1 year ago

Use can pastebin or gist.github.com. There is no private stuff in the log - this is a standard for reporting bugs to include a clashlog. It is best when it's a full log.

Logandros commented 1 year ago

https://gist.github.com/Logandros/8a31a6559a131353dd441d51665bac35

If I remove the (shoulders) item. The crash is resolved.

UPDATEcharacters.characters_npcbotSETowner= 3,roles= 37,spec= 3,faction= 35,equipMhEx= 0,equipOhEx= 0,equipRhEx= 0,equipHead= 0,equipShoulders= 389432,equipChest= 0,equipWaist= 0,equipLegs= 0,equipFeet= 0,equipWrist= 0,equipHands= 0,equipBack= 0,equipBody= 0,equipFinger1= 0,equipFinger2= 0,equipTrinket1= 0,equipTrinket2= 0,equipNeck= 0,spells_disabled= '' WHEREentry= 70011;

trickerer commented 1 year ago

Show your CHAR_SEL_CHARACTER_INVENTORY prepared statement

.../server/database/Database/Implementation/CharacterDatabase.cpp:L97-98

and last 20 lines of your Item::LoadFromDB(), where transmog and enchant are being set

Logandros commented 1 year ago

PrepareStatement(CHAR_SEL_CHARACTER_INVENTORY, "SELECT creatorGuid, giftCreatorGuid, count, duration, charges, flags, enchantments, randomPropertyId, durability, playedTime, text, bag, slot, " "item, itemEntry, 0, transmog, enchant FROM character_inventory ci JOIN item_instance ii ON ci.item = ii.guid WHERE ci.guid = ? ORDER BY bag, slot", CONNECTION_ASYNC);

and I'm not sure where Item::LoadFromDB() is from is that in item.h / .cpp ? I will search in the meantime

trickerer commented 1 year ago

In Item.cpp of course where it's implemented, Item.h only contains its declaration. Search for transmog = fields[

Logandros commented 1 year ago

Oddly I'm setting it much higher in the file L505-506

https://gist.github.com/Logandros/3452335637143254e0996769ead736c2

Edit: Apologies, you probably only wanted to see this section: https://gist.github.com/Logandros/af47c271fa2cffbf075fd101c4737bac

trickerer commented 1 year ago

Alright. So, to fix the crash you need to extend the first line of your CHAR_SEL_NPCBOT_EQUIP_BY_ITEM_INSTANCE prepared statement in CharacterDatabase.cpp like this:

Old:

PrepareStatement(CHAR_SEL_NPCBOT_EQUIP_BY_ITEM_INSTANCE, "SELECT creatorGuid, ... guid, itemEntry, owner_guid "

New:

PrepareStatement(CHAR_SEL_NPCBOT_EQUIP_BY_ITEM_INSTANCE, "SELECT creatorGuid, ... guid, itemEntry, owner_guid, 0, 0, transmog, enchant "

Count the arguments in the statement, there must be 18 arguments now. This should fix the crash

Logandros commented 1 year ago

That fixed the issue, reloaded and shoulders are in the bot's equipment list!