[Cataclysm Classic] Equipped items are also counted to be located in a bag

Question

[Cataclysm Classic] Equipped items are also counted to be located in a bag

kanodiry opened this issue 7 months ago · 15 comments

kanodiry commented 7 months ago

If you view items as another character, the equipped items will also count as in the bag, and the character will be considered to have two items at once: equipped and in the bag.
I checked SavedVariables, and this confuses me:
As you can see there is ["bag"][-2] contains all equipped items plus 2 bags (don't know why).

BagSyncDB = {
	["Realm"] = {
		["Character"] = {
			["bag"] = {
				{
					"37903;21", -- [1]
					"57914", -- [2]
					"56449", -- [3]
					"62251;16", -- [4]
					"62251;20", -- [5]
					"58085;4", -- [6]
					"62662;11", -- [7]
					"9172;2", -- [8]
					"53051;20", -- [9]
					"33447;20", -- [10]
					"57191;8", -- [11]
					"57191;20", -- [12]
					"43015;18", -- [13]
					"62662;20", -- [14]
					"62662;20", -- [15]
					"73260;14", -- [16]
				}, -- [1]
				{
					"8529;20", -- [1]
					"8529;15", -- [2]
					"6657;18", -- [3]
					"68644;17", -- [4]
					"71096;208", -- [5]
					"64396", -- [6]
					"52843", -- [7]
					"58090;2", -- [8]
					"60853", -- [9]
					"6522;7", -- [10]
				}, -- [2]
				{
					"45439", -- [1]
					"42101", -- [2]
					"19123", -- [3]
					"792", -- [4]
					"63388;5", -- [5]
					"68646", -- [6]
					"71083;17", -- [7]
					"59477", -- [8]
					"59491", -- [9]
					"42420", -- [10]
					"52078;9", -- [11]
				}, -- [3]
				{
					"56278", -- [1]
					"56330", -- [2]
					"63458", -- [3]
					"51832", -- [4]
					"55797", -- [5]
					"51879", -- [6]
					"55260", -- [7]
					"57260", -- [8]
					"62350", -- [9]
					"56129", -- [10]
					"61472", -- [11]
					"55270", -- [12]
					"56393", -- [13]
					"56458", -- [14]
					"63787", -- [15]
					"50459", -- [16]
					"63206", -- [17]
					"50361", -- [18]
					"65905", -- [19]
					"65908", -- [20]
					"52252", -- [21]
					"43155", -- [22]
					"46874", -- [23]
					"22999", -- [24]
				}, -- [4]
				[0] = {
					"6948", -- [1]
					"40110", -- [2]
					"5507", -- [3]
					"39769", -- [4]
					"52251", -- [5]
					"49040", -- [6]
					"37863", -- [7]
					"40772", -- [8]
					"23821", -- [9]
					"67494", -- [10]
					"60223", -- [11]
					"45631", -- [12]
					"33820", -- [13]
					"44050", -- [14]
					"40769;20", -- [15]
					"49633", -- [16]
				},
				[-2] = {
					"59359", -- [1]
					"57932", -- [2]
					"56452", -- [3]
					"14617", -- [4]
					"58101", -- [5]
					"55059", -- [6]
					"58102", -- [7]
					"62432", -- [8]
					"56416", -- [9]
					"58105", -- [10]
					"58187", -- [11]
					"56398", -- [12]
					"55881", -- [13]
					"56347", -- [14]
					"62383", -- [15]
					"64377", -- [16]
					"56337", -- [17]
					"69210", -- [18]
					"51809", -- [19]
					"51809", -- [20]
				},
			},
			["equip"] = {
				"59359", -- [1]
				"57932", -- [2]
				"56452", -- [3]
				"14617", -- [4]
				"58101", -- [5]
				"55059", -- [6]
				"58102", -- [7]
				"62432", -- [8]
				"56416", -- [9]
				"58105", -- [10]
				"58187", -- [11]
				"56398", -- [12]
				"55881", -- [13]
				"56347", -- [14]
				"62383", -- [15]
				"64377", -- [16]
				"56337", -- [17]
				"69210", -- [18]
			},

Xruptor · Answer 1 · 2024-06-08T14:53:30.000Z

Unless they changed something (which I wouldn't be surprised...) then it shouldn't be doing that. Blizzard has a habit of constantly changing their code, which is why as an addon author it's getting frustrating dealing with it.

Do me a favor, try a DB reset and see it it still happens.

/bgs resetdb

kanodiry · Answer 2 · 2024-06-08T15:38:25.000Z

Sorry for not mention this, I did:

/bgs fixdb
/bgs resetdb
Delete from SavedVariables BagSync.lua (and .bak)

Xruptor · Answer 3 · 2024-06-09T23:17:48.000Z

The bag in question that is saving the equipment, should be the Keyring bag. I'm not entirely sure how the equipment is being stored there and it could possibly be a bug on the server itself. I'll review the code but here is the bag numbers in code.
https://wowpedia.fandom.com/wiki/BagID

-2 is the KEYRING_CONTAINER

Xruptor · Answer 4 · 2024-06-09T23:09:59.000Z

Sorry for not mention this, I did:

/bgs fixdb

/bgs resetdb

Delete from SavedVariables BagSync.lua (and .bak)

That means something else is going on and I'd need to investigate if it was STILL saving the equipment as part of the bags. Which it shouldn't.

kanodiry · Answer 5 · 2024-06-10T05:15:01.000Z

The bag in question that is saving the equipment, should be the Keyring bag. I'm not entirely sure how the equipment is being stored there and it could possibly be a bug on the server itself. I'll review the code but here is the bag numbers in code.
https://wowpedia.fandom.com/wiki/BagID

-2 is the KEYRING_CONTAINER

Keyring was removed when raid dungeons became live.

kanodiry · Answer 6 · 2024-06-10T08:48:21.000Z

Well, after you said that the bag with index -2 is a keyring, I decided to also look for a solution.

I made one change in
../BagSync/wireframe/events.lua

function Events:BAG_UPDATE_DELAYED(event)
	--[[	// *** //	]]

		if Scanner:IsBackpack(bagid) or Scanner:IsBackpackBag(bagid) or Scanner:IsKeyring(bagid) then
			bagname = "bag"
		elseif Scanner:IsBank(bagid) or Scanner:IsBankBag(bagid) then
			--only do this while we are at a bank
			if Unit.atBank then
				bagname = "bank"
			end
		end

	--[[	// *** //	]]
end

I removed or Scanner:IsKeyring(bagid).

And It seems work well now.
Tell me if this appropriate fix, or this could break something else.

Xruptor · Answer 7 · 2024-06-10T12:14:44.000Z

The bag in question that is saving the equipment, should be the Keyring bag. I'm not entirely sure how the equipment is being stored there and it could possibly be a bug on the server itself. I'll review the code but here is the bag numbers in code.
https://wowpedia.fandom.com/wiki/BagID
-2 is the KEYRING_CONTAINER

Keyring was removed when raid dungeons became live.

Yes it was removed but the underlying code is still there and the keyring bag itself is actually still used to store some quest keys and a few other in game tokens they use. You just can't see it. In addition remember that BagSync is backwards compatible so it has to work with the previous expansions from Classic all the way to Retail.

Xruptor · Answer 8 · 2024-06-10T12:11:40.000Z

Well, after you said that the bag with index -2 is a keyring, I decided to also look for a solution.

I made one change in ../BagSync/wireframe/events.lua
function Events:BAG_UPDATE_DELAYED(event)
	--[[	// *** //	]]

		if Scanner:IsBackpack(bagid) or Scanner:IsBackpackBag(bagid) or Scanner:IsKeyring(bagid) then
			bagname = "bag"
		elseif Scanner:IsBank(bagid) or Scanner:IsBankBag(bagid) then
			--only do this while we are at a bank
			if Unit.atBank then
				bagname = "bank"
			end
		end

	--[[	// *** //	]]
end
I removed or Scanner:IsKeyring(bagid).

And It seems work well now. Tell me if this appropriate fix, or this could break something else.

Technically yes that would fix it. However, it's not a 100% fix as all you are doing is removing the scanning of the Keyring. The question we should be asking is WHY the equipment is being scanned as part of the keyring bag? Unless something is broken on their servers or Blizzard changed something, the equipment should not be scanned as part of the keyring bag. So there is something else going on that needs to be looked into.

kanodiry · Answer 9 · 2024-06-10T13:52:43.000Z

The question we should be asking is WHY the equipment is being scanned as part of the keyring bag? Unless something is broken on their servers or Blizzard changed something, the equipment should not be scanned as part of the keyring bag. So there is something else going on that needs to be looked into.

My guess they're forgot to change GetContainerNumSlots(-2) output to 0.

From link you provided I got this:

The Keyring was removed in 4.2.0 but the constant still exists. On retail this "bag" holds the InventorySlotIds instead.

Also I got this:

In 2.0.3, the Key Ring(-2) always returns 32. The size of the bag displayed is determined by the amount of space used in the keyring.

I called C_Container.GetContainerNumSlots(-2) function on Retail and Cataclysm, and I got:
Retail = 0;
Cataclysm = 32;

Yes it was removed but the underlying code is still there and the keyring bag itself is actually still used to store some quest keys and a few other in game tokens they use. You just can't see it. In addition remember that BagSync is backwards compatible so it has to work with the previous expansions from Classic all the way to Retail.

If it's hidden, so items stored in it are inaccessible, do we really need to sync this?

I think we should wait for them to fix it.

Xruptor · Answer 10 · 2024-06-10T14:24:38.000Z

I think I just need to do a check for the unlock for the KeyRing for classic and such. Try this version of BagSync and let me know if it works. I added checks for HasKey() function which should return false for Cataclysm and Retail.

BagSync.zip

kanodiry · Answer 11 · 2024-06-10T16:07:53.000Z

It works fine.

Xruptor · Answer 12 · 2024-06-10T18:38:32.000Z

It works fine.

So no duplicate issues or reporting errors? It works fine on Retail and Cataclysm?

Xruptor · Answer 13 · 2024-06-10T19:50:51.000Z

I have added some additional checks and cleanup to the keyring processing.

Please try this version of BagSync with Classic, Cataclysm, and Retail. Let me know if it works properly.

BagSync.zip

kanodiry · Answer 14 · 2024-06-11T05:28:15.000Z

I have added some additional checks and cleanup to the keyring processing.

Please try this version of BagSync with Classic, Cataclysm, and Retail. Let me know if it works properly.

Well I've tried all versions, and it seems everything works:
No duplications on Retail and Cata and synced keys in keyring on Classic.

Xruptor · Answer 15 · 2024-06-11T13:05:05.000Z

I have added some additional checks and cleanup to the keyring processing.
Please try this version of BagSync with Classic, Cataclysm, and Retail. Let me know if it works properly.

Well I've tried all versions, and it seems everything works: No duplications on Retail and Cata and synced keys in keyring on Classic.

Thanks for testing! I'll make sure to push the changes today.

Share to