fix [C++] CHARACTER::Sync Renewal (Cannot Find Tree FIX)

[MT2Dev 1568]

INTRODUCTION & PROBLEM

Hello, we will solve the famous "Cannot Find Tree" problem in the "CHARACTER::Sync" function in the server src and also the problems that may be caused by the "Dead();" section used for everything that is not a PC in this function.

WHAT DID WE CHANGE?

All of the currently shared fixes are actually incomplete, we will make a new arrangement to solve this problem at its root, thus we will fix the problem completely.

UPDATE - 18/10/2024

   With @ Mitachi's great feedback, i rewrote the "RewardlessDead()" function.

LAST UPDATE - 23/10/2024

  I fixed a code that caused the (!new_tree) part of the function to return false under all circumstances (thanks to @ WeedHex ), also added all the nullptr checks and extra sys_err messages where necessary, expanded the content of the existing sys_err messages, these changes will not cause any performance disadvantages and there is no possibility of it being misused in any way anymore, please use the final version.

HOW TO DO?

  Without going into details such as editing the reward mechanism of the existing dead function, we will simply create a new dead function and use it in Sync. The main purpose of this function, which can only be used for mobs and metins, is that it does not give a reward (drop) after the dead function, let's get started.

 

First, open "char_battle.cpp"

This is the hidden content, please

• Sign In
• or
• Sign Up

Best regards, MT2Dev.

 

Edited September 26, 2025 by MT2Dev
CHARACTER::Sync UPDATE.

[Nazox 473]

Hey!, thanks ,  you can explain me a little about this problem I never listen about that ?

[Endymion 399]

SET_BIT(m_pointsInstant.instant_flag, INSTANT_FLAG_NO_REWARD);
Dead();

[MT2Dev 1568]

On 10/11/2024 at 11:49 AM, Nazox said:

Hey!, thanks ,  you can explain me a little about this problem I never listen about that ?

Hey, sure! Basicly this fix actually for the very old h*ck that allows to you move players with sync packet out of the map and kick them out the game, with the original code if that scenario is happen, player will kicked via PHASE_CLOSE and if it's not player (else block) function will the trigger Dead event for the destroy but there is some problem in this logic, if you use this to move mobs out of the map they will dead instantly (even if their HP is full) and they will give a full drop(reward), thats something we don't need for our servers i guess, also this fix useful for problem very fast mounts will kick players out the game, btw thats a long comment sorry for my poor grammar. 

On 10/11/2024 at 4:06 PM, Endymion said:

SET_BIT(m_pointsInstant.instant_flag, INSTANT_FLAG_NO_REWARD);
Dead();

It's up to you, can do it by that way but i'm use this new function also for other fixes, for that reason i created new func for this job.

 

#UPDATE

 

Sync function updated for better performence.

Edited October 14, 2024 by MT2Dev
Update..

[Mitachi 3368]

I'll start with a genuine thanks for sharing,

in ::Reward, (tanaka or not) there actually is:

	PIXEL_POSITION pos = GetXYZ();
	if (!SECTREE_MANAGER::instance().GetMovablePosition(GetMapIndex(), pos.x, pos.y, pos))
		return;

But, SetXYZ() is called after Dead() due can't sync, so the dead without reward make some sense here.

However, the topic should then talk about the fix of this hack/bug that players can somehow exploit, because otherwise,
reassigning sectree instead of PHASE_CLOSE is something everyone has been doing to avoid "fake launcher-crash"
(is not exactly caused by a pkt loss, even if seems impossible trigger the bug on windows localhost, tested without IPE).

About “RewardlessDead”,  weird name for a Dead with NoReward, I guess you've just tried to make different than marty somehow, maybe I am wrong

Also by copying half ::Dead function, there is a risk that if you add systems in the future that need to be triggered to death, you forget to add them in both,
it looks unmaintainable, and I see no point to force the Reward(false) if is only for npc, or you will give to the hacker the align, steal hp/sp and the quest kill
if he was also the killer (so if someone damaged him).

A "pure" death can be something like (not tested):

void CHARACTER::_()
{
	// just to prevent, not needed if you use it properly
	if (IsPC())
	{
		sys_log(0, "::_ Bad usage, it was a PC, called normal Dead()");
		Dead();
		return;
	}
	
	// remove the killer to avoid quest triggers may in Dead
	m_dwKillerPID = 0;
	
	// set no reward
	SET_BIT(m_pointsInstant.instant_flag, INSTANT_FLAG_NO_REWARD);
	
	// kill
	Dead();
}

be careful, here you're not only avoiding the loots like drop, gold, but even what I mentioned above.

[MT2Dev 1568]

23 minutes ago, Mitachi said:

I'll start with a genuine thanks for sharing,

in ::Reward, (tanaka or not) there actually is:

	PIXEL_POSITION pos = GetXYZ();
	if (!SECTREE_MANAGER::instance().GetMovablePosition(GetMapIndex(), pos.x, pos.y, pos))
		return;

But, SetXYZ() is called after Dead() due can't sync, so the dead without reward make some sense here.

However, the topic should then talk about the fix of this hack/bug that players can somehow exploit, because otherwise,
reassigning sectree instead of PHASE_CLOSE is something everyone has been doing to avoid "fake launcher-crash"
(is not exactly caused by a pkt loss, even if seems impossible trigger the bug on windows localhost, tested without IPE).

About “RewardlessDead”,  weird name for a Dead with NoReward, I guess you've just tried to make different than marty somehow, maybe I am wrong

Also by copying half ::Dead function, there is a risk that if you add systems in the future that need to be triggered to death, you forget to add them in both,
it looks unmaintainable, and I see no point to force the Reward(false) if is only for npc, or you will give to the hacker the align, steal hp/sp and the quest kill
if he was also the killer (so if someone damaged him).

A "pure" death can be something like (not tested):

void CHARACTER::_()
{
	// just to prevent, not needed if you use it properly
	if (IsPC())
	{
		sys_log(0, "::_ Bad usage, it was a PC, called normal Dead()");
		Dead();
		return;
	}
	
	// remove the killer to avoid quest triggers may in Dead
	m_dwKillerPID = 0;
	
	// set no reward
	SET_BIT(m_pointsInstant.instant_flag, INSTANT_FLAG_NO_REWARD);
	
	// kill
	Dead();
}

be careful, here you're not only avoiding the loots like drop, gold, but even what I mentioned above.

Thanks for your great feedback! I missed a point that it could trigger quests, also if it's a pc, redirecting it to the Dead function is a great idea, I'll rework it with your suggestions, thanks again.

Also, since I've received multiple messages about this, i need to say something about the source of this fix, after a conversation with dear @ Koray on a different TR forum, he gave me the idea for this fix, I prepared this topic for him, so i don't have an idea about Marty's latest fix, his method is probably better but I haven't seen it yet, best regards.

 

Spoiler

It's translated; 

 

Edited October 18, 2024 by Metin2 Dev International
Core X - External 2 Internal

[MT2Dev 1568]

#LAST UPDATE

The "Sync" function has been rewritten to fix a major bug that was affecting the code's operation, please use last version!

Edited October 23, 2024 by MT2Dev

[MT2Dev 1568]

#Little Update

Little update released for "bool CHARACTER::Sync" function, it's not really necessary but you should be use this last version if u ask me, so it's up to you.

[silence 0]

hello i have problem before this update i got sectree differ Macexx1 x was x diff 1
now i have this 

Feb  5 02:47:56 :: SECTREE DIFFER: Player Macexx1
Feb  5 02:47:59 ::       in same sectree
Feb  5 02:48:00 :: SECTREE DIFFER: Player Macexx1
Feb  5 02:48:03 ::       in same sectree
Feb  5 02:48:04 :: SECTREE DIFFER: Player Macexx1
Feb  5 02:48:08 ::       in same sectree
Feb  5 02:48:08 :: COMMAND: Macexx1: ride

Feb  5 02:48:24 :: SECTREE DIFFER: Player Macexx1
Feb  5 02:48:27 :: SECTREE DIFFER: Player Macexx1

 

thanks to all 

[Anielle Noir 137]

The posted version has the right main idea: mobs/metin stones should not call normal Dead() when sync fails, because that can trigger unwanted rewards or drops. RewardlessDead() is the important fix.

However, I would avoid the fallback part:
 

x = GetX();
y = GetY();
NewTree = GetSectree();

If the new position has no valid sectree, the sync should fail immediately. Falling back to the old sectree hides invalid movement/sync issues and makes debugging harder. It also changes the standard behavior for players, because instead of closing the connection on invalid sync, it silently keeps the character in the old tree.

The cleaner version keeps the useful part of the fix, but stays stricter:

1. invalid target sectree = return false

2. no SetXYZ() on invalid coordinates

3. mobs/metin stones use RewardlessDead()

4. players keep the standard invalid-sync handling

5. fewer unnecessary null checks

6. less live log spam

7. better debug information with old and new coordinates

So the posted version fixes the reward/drop problem, but the stricter version fixes it without masking invalid sync problems.

This is my final version for the sync:

bool CHARACTER::Sync(long x, long y)
{
	LPSECTREE old_tree = GetSectree();

	if (!old_tree)
	{
		if (test_server)
			sys_err("<CHARACTER::Sync> Invalid current sectree. Name: %s Map: %ld X: %ld Y: %ld",
				GetName(), GetMapIndex(), GetX(), GetY());

		return false;
	}

	if (IsPC() && IsDead())
	{
		if (test_server)
			sys_log(0, "<CHARACTER::Sync> Dead player tried to sync. Name: %s Map: %ld X: %ld Y: %ld",
				GetName(), GetMapIndex(), x, y);

		return false;
	}

	LPSECTREE new_tree = SECTREE_MANAGER::instance().Get(GetMapIndex(), x, y);

	if (!new_tree)
	{
		if (GetDesc())
		{
			sys_err("[SYNC] Cannot find sectree. Name: %s Map: %ld X: %ld Y: %ld OldX: %ld OldY: %ld",
				GetName(), GetMapIndex(), x, y, GetX(), GetY());

			GetDesc()->SetPhase(PHASE_CLOSE);
		}
		else
		{
			sys_err("[SYNC] Entity has no sectree. Name: %s Map: %ld X: %ld Y: %ld OldX: %ld OldY: %ld",
				GetName(), GetMapIndex(), x, y, GetX(), GetY());

			if (IsMonster() || IsStone())
				RewardlessDead();
			else if (!IsDead())
				Dead();
		}

		return false;
	}

	SetRotationToXY(x, y);
	SetXYZ(x, y, 0);

	if (GetDungeon())
	{
		const int iLastEventAttr = m_iEventAttr;
		m_iEventAttr = new_tree->GetEventAttribute(x, y);

		if (m_iEventAttr != iLastEventAttr)
		{
			const DWORD dwPID = GetParty() ? GetParty()->GetLeaderPID() : GetPlayerID();

			quest::CQuestManager::instance().AttrOut(dwPID, this, iLastEventAttr);
			quest::CQuestManager::instance().AttrIn(dwPID, this, m_iEventAttr);
		}
	}

	if (old_tree != new_tree)
	{
		if (!IsNPC())
		{
			const SECTREEID id = new_tree->GetID();
			const SECTREEID old_id = old_tree->GetID();

			const float fDist = DISTANCE_SQRT(
				id.coord.x - old_id.coord.x,
				id.coord.y - old_id.coord.y
			);

			if (test_server)
			{
				sys_log(0, "SECTREE DIFFER: %s %dx%d was %dx%d dist %.1fm",
					GetName(),
					id.coord.x, id.coord.y,
					old_id.coord.x, old_id.coord.y,
					fDist);
			}
		}

		new_tree->InsertEntity(this);
	}

	return true;
}

thanks for the heads-up tho