fix [Bugfix] Sorting item proto tables when loading via txt

[Sonitex 1406]

Ayo,

recently I've come across an issue with a customer who had item proto entries all over the place and when the database tried to sort that mess, it messed it up even more, eventually, it messed up this very Monday for me as I spent hours debugging this.

Long story short: It first inserts key-value to map m_map_itemTableByVnum where the key was an integer (item vnum) and the value was a pointer that pointed to an item table object at position X in m_vec_itemTable but at the end, it sorted this very vector while values in map still pointed to the same positions in the vector. Guessed what happened? An item with vnum 19709 had an item table of the item with vnum 20009 because you donkey were too lazy to put new proto entries in the correct order. 

Go over to server source, database/ClientManagerBoot.cpp, at the end of the following function:

bool CClientManager::InitializeItemTable()
{
	[...]
	m_map_itemTableByVnum.clear();

	auto it = m_vec_itemTable.begin();

	while (it != m_vec_itemTable.end())
	{
		TItemTable * item_table = &(*(it++));

		sys_log(1, "ITEM: #%-5lu %-24s %-24s VAL: %ld %ld %ld %ld %ld %ld WEAR %lu ANTI %lu IMMUNE %lu REFINE %lu REFINE_SET %u MAGIC_PCT %u", 
				item_table->dwVnum,
				item_table->szName,
				item_table->szLocaleName,
				item_table->alValues[0],
				item_table->alValues[1],
				item_table->alValues[2],
				item_table->alValues[3],
				item_table->alValues[4],
				item_table->alValues[5],
				item_table->dwWearFlags,
				item_table->dwAntiFlags,
				item_table->dwImmuneFlag,
				item_table->dwRefinedVnum,
				item_table->wRefineSet,
				item_table->bAlterToMagicItemPct);

		m_map_itemTableByVnum.insert(std::map<DWORD, TItemTable *>::value_type(item_table->dwVnum, item_table));
	}
	sort(m_vec_itemTable.begin(), m_vec_itemTable.end(), FCompareVnum());
	return true;

Now we'll do some dark magic, we'll move the sort function at the beginning of previously shown code, like so:

bool CClientManager::InitializeItemTable()
{
	[...]
	m_map_itemTableByVnum.clear();

  	// -------------------------------------------------------------> Wohoo I am here now!
  	sort(m_vec_itemTable.begin(), m_vec_itemTable.end(), FCompareVnum());
	auto it = m_vec_itemTable.begin();

	while (it != m_vec_itemTable.end())
	{
		TItemTable * item_table = &(*(it++));

		sys_log(1, "ITEM: #%-5lu %-24s %-24s VAL: %ld %ld %ld %ld %ld %ld WEAR %lu ANTI %lu IMMUNE %lu REFINE %lu REFINE_SET %u MAGIC_PCT %u", 
				item_table->dwVnum,
				item_table->szName,
				item_table->szLocaleName,
				item_table->alValues[0],
				item_table->alValues[1],
				item_table->alValues[2],
				item_table->alValues[3],
				item_table->alValues[4],
				item_table->alValues[5],
				item_table->dwWearFlags,
				item_table->dwAntiFlags,
				item_table->dwImmuneFlag,
				item_table->dwRefinedVnum,
				item_table->wRefineSet,
				item_table->bAlterToMagicItemPct);

		m_map_itemTableByVnum.insert(std::map<DWORD, TItemTable *>::value_type(item_table->dwVnum, item_table));
	}
	return true;

 

[Denizeri24 35]

bool CClientManager::InitializeItemTable()
{
	std::map<int32_t, const char *> localMap;
	cCsvTable nameData;

	if (!nameData.Load("item_names.txt", '\t'))
	{
		fmt::fprintf(stderr, "item_names.txt couldn't be loaded or its format is incorrect.\n");
		return false; // There's no reason to continue without names for us (i dont like korean)
	}

	nameData.Next(); // skip the description

	while (nameData.Next())
	{
		localMap[static_cast<int32_t>(std::strtol(nameData.AsStringByIndex(0), nullptr, 0))] = nameData.AsStringByIndex(1);
	}

	cCsvTable data;

	if (!data.Load("item_proto.txt", '\t'))
	{
		fmt::fprintf(stderr, "item_proto.txt couldn't be loaded or the format is incorrect \n");
		return false;
	}

	data.Next(); // skip first row (descriptions)
	m_vec_itemTable.resize(data.m_File.GetRowCount() - 1); // set the size of the vector
	memset(&m_vec_itemTable[0], 0, sizeof(TItemTable) * m_vec_itemTable.size()); // zero initialize
	auto item_table = &m_vec_itemTable[0];

	for (; data.Next(); ++item_table)
	{
		if (!Set_Proto_Item_Table(item_table, data, localMap))
		{
			fmt::fprintf(stderr, "Invalid item table. VNUM: %u\n", item_table->dwVnum);
		}

		m_map_itemTableByVnum.emplace(item_table->dwVnum, item_table);
	}

	std::sort(std::execution::par, m_vec_itemTable.begin(), m_vec_itemTable.end(), FCompareVnum());
	data.Destroy();
	nameData.Destroy();
	return true;
}

There's zero initialization here. So I don't think your fix is needed..

[xGalardo 11]

25 minutes ago, Denizeri24 said:

bool CClientManager::InitializeItemTable()
{
	std::map<int32_t, const char *> localMap;
	cCsvTable nameData;

	if (!nameData.Load("item_names.txt", '\t'))
	{
		fmt::fprintf(stderr, "item_names.txt couldn't be loaded or its format is incorrect.\n");
		return false; // There's no reason to continue without names for us (i dont like korean)
	}

	nameData.Next(); // skip the description

	while (nameData.Next())
	{
		localMap[static_cast<int32_t>(std::strtol(nameData.AsStringByIndex(0), nullptr, 0))] = nameData.AsStringByIndex(1);
	}

	cCsvTable data;

	if (!data.Load("item_proto.txt", '\t'))
	{
		fmt::fprintf(stderr, "item_proto.txt couldn't be loaded or the format is incorrect \n");
		return false;
	}

	data.Next(); // skip first row (descriptions)
	m_vec_itemTable.resize(data.m_File.GetRowCount() - 1); // set the size of the vector
	memset(&m_vec_itemTable[0], 0, sizeof(TItemTable) * m_vec_itemTable.size()); // zero initialize
	auto item_table = &m_vec_itemTable[0];

	for (; data.Next(); ++item_table)
	{
		if (!Set_Proto_Item_Table(item_table, data, localMap))
		{
			fmt::fprintf(stderr, "Invalid item table. VNUM: %u\n", item_table->dwVnum);
		}

		m_map_itemTableByVnum.emplace(item_table->dwVnum, item_table);
	}

	std::sort(std::execution::par, m_vec_itemTable.begin(), m_vec_itemTable.end(), FCompareVnum());
	data.Destroy();
	nameData.Destroy();
	return true;
}

There's zero initialization here. So I don't think your fix is needed..

This post is for those who may have a similar problem. So not you.

[Sonitex 1406]

28 minutes ago, Denizeri24 said:

bool CClientManager::InitializeItemTable()
{
	std::map<int32_t, const char *> localMap;
	cCsvTable nameData;

	if (!nameData.Load("item_names.txt", '\t'))
	{
		fmt::fprintf(stderr, "item_names.txt couldn't be loaded or its format is incorrect.\n");
		return false; // There's no reason to continue without names for us (i dont like korean)
	}

	nameData.Next(); // skip the description

	while (nameData.Next())
	{
		localMap[static_cast<int32_t>(std::strtol(nameData.AsStringByIndex(0), nullptr, 0))] = nameData.AsStringByIndex(1);
	}

	cCsvTable data;

	if (!data.Load("item_proto.txt", '\t'))
	{
		fmt::fprintf(stderr, "item_proto.txt couldn't be loaded or the format is incorrect \n");
		return false;
	}

	data.Next(); // skip first row (descriptions)
	m_vec_itemTable.resize(data.m_File.GetRowCount() - 1); // set the size of the vector
	memset(&m_vec_itemTable[0], 0, sizeof(TItemTable) * m_vec_itemTable.size()); // zero initialize
	auto item_table = &m_vec_itemTable[0];

	for (; data.Next(); ++item_table)
	{
		if (!Set_Proto_Item_Table(item_table, data, localMap))
		{
			fmt::fprintf(stderr, "Invalid item table. VNUM: %u\n", item_table->dwVnum);
		}

		m_map_itemTableByVnum.emplace(item_table->dwVnum, item_table);
	}

	std::sort(std::execution::par, m_vec_itemTable.begin(), m_vec_itemTable.end(), FCompareVnum());
	data.Destroy();
	nameData.Destroy();
	return true;
}

There's zero initialization here. So I don't think your fix is needed..

Yep, you're correct, the issue is only present on default metin2 sources.

[Denizeri24 35]

39 minutes ago, xGalardo said:

This post is for those who may have a similar problem. So not you.

idk, i use mainline sources too (razuning v2 from turkmmo) i just clean unnecessary codes from this function..

[Sonitex 1406]

49 minutes ago, Denizeri24 said:

idk, i use mainline sources too (razuning v2 from turkmmo) i just clean unnecessary codes from this function..

Then you have it fixed without knowing

[Abel(Tiger) 1138]

2 hours ago, Denizeri24 said:

bool CClientManager::InitializeItemTable()
{
	std::map<int32_t, const char *> localMap;
	cCsvTable nameData;

	if (!nameData.Load("item_names.txt", '\t'))
	{
		fmt::fprintf(stderr, "item_names.txt couldn't be loaded or its format is incorrect.\n");
		return false; // There's no reason to continue without names for us (i dont like korean)
	}

	nameData.Next(); // skip the description

	while (nameData.Next())
	{
		localMap[static_cast<int32_t>(std::strtol(nameData.AsStringByIndex(0), nullptr, 0))] = nameData.AsStringByIndex(1);
	}

	cCsvTable data;

	if (!data.Load("item_proto.txt", '\t'))
	{
		fmt::fprintf(stderr, "item_proto.txt couldn't be loaded or the format is incorrect \n");
		return false;
	}

	data.Next(); // skip first row (descriptions)
	m_vec_itemTable.resize(data.m_File.GetRowCount() - 1); // set the size of the vector
	memset(&m_vec_itemTable[0], 0, sizeof(TItemTable) * m_vec_itemTable.size()); // zero initialize
	auto item_table = &m_vec_itemTable[0];

	for (; data.Next(); ++item_table)
	{
		if (!Set_Proto_Item_Table(item_table, data, localMap))
		{
			fmt::fprintf(stderr, "Invalid item table. VNUM: %u\n", item_table->dwVnum);
		}

		m_map_itemTableByVnum.emplace(item_table->dwVnum, item_table);
	}

	std::sort(std::execution::par, m_vec_itemTable.begin(), m_vec_itemTable.end(), FCompareVnum());
	data.Destroy();
	nameData.Destroy();
	return true;
}

There's zero initialization here. So I don't think your fix is needed..

If you use the code from the leaked Rubinum source, it does not mean you have solved the problem. Also, if you use modern coding techniques from C++ new standards, it does not mean all the code will fix itself. Your vector is still sorted, and the pointer in the map is still pointing to the old vector position.

Add this code after the std::sort, and you will be surprised:

 

	for (auto it = m_map_itemTableByVnum.begin(); it != m_map_itemTableByVnum.end(); ++it)
	{
		DWORD key = it->first;               // Access the key
		TItemTable* value = it->second;      // Access the value
		
		if(value->dwVnum != key)
		{
			sys_err("Map Key %u --- Value vnum %u", key, value->dwVnum);
		}
	}

 

[TAUMP 802]

Again in this thread he shows who is the bigger idiot. And who comes up with a better fix.

[Denizeri24 35]

1 hour ago, Abel(Tiger) said:

If you use the code from the leaked Rubinum source, it does not mean you have solved the problem. Also, if you use modern coding techniques from C++ new standards, it does not mean all the code will fix itself. Your vector is still sorted, and the pointer in the map is still pointing to the old vector position.

Add this code after the std::sort, and you will be surprised:

 

	for (auto it = m_map_itemTableByVnum.begin(); it != m_map_itemTableByVnum.end(); ++it)
	{
		DWORD key = it->first;               // Access the key
		TItemTable* value = it->second;      // Access the value
		
		if(value->dwVnum != key)
		{
			sys_err("Map Key %u --- Value vnum %u", key, value->dwVnum);
		}
	}

 

I dont any use code from rubinum source, I using mainline source (razuning v2 from turkmmo). I just clear unnecessary from this function.