[Fix] "when login" trigger before affect initialization

[Mitachi 3392]

Hi, I guess I am spending too much time with @ Syreldar.

If you would like a full explanation:

Spoiler

I don't usually explain what I do, but I've decided to start doing so. It might be useful, who knows.

This is quite an old and well-known bug, that’s why quests have always been written in a way to avoid it.
The core issue is that quests are initialized before affects, so the when login trigger fires before affects are loaded.

Have you ever tried to do something on login that involved affects?

e.g.:

• You want to check if the player has a mount when entering the OX map and dismount them.
• You want to polymorph them into mob 101.

Well, that wouldn’t work.

The common workaround everyone uses is simply adding a timer to the login trigger, even 1 second is enough.
That’s why this bug has never been considered a real problem: the workaround is simple and effective. If it works, it works.

-- BUG AFFECTED
quest bug_affect_login begin
    state start begin
        when login with pc.get_map_index() == 41 begin
            pc.polymorph(101, 300); -- poly 101 for 5 minutes
        end
    end
end

-- WORKAROUND FIX WE'RE USED TO
quest bug_affect_login begin
    state start begin
        when login with pc.get_map_index() == 41 begin
            timer("login_workaround", 1) -- WORKAROUND
        end
        
        when login_workaround.timer begin -- WORKAROUND
            pc.polymorph(101, 300); -- poly 101 for 5 minutes
        end
    end
end

However, today I started wondering why this happens. At first I blamed the "slowness" of affect loading.
I then discovered that quests are simply loaded before affects.

Swapping the order (initialize affects before quests) does work, because queries are executed in FIFO order on the SQL thread.
At first I thought this wasn’t guaranteed and considered it unsafe, but after reviewing the SQL layer it’s clear the order is deterministic.
Still, I preferred a more robust fix: always using the quest_login_event, and delaying the login trigger until both PHASE_GAME and IsLoadedAffect() are satisfied.

Eventually, I traced it back to the exact piece of code that calls the login trigger, inside QuestLoad, at the end:

		if (ch->GetDesc()->IsPhase(PHASE_GAME))
		{
			sys_log(0, "QUEST_LOAD: Login pc %d", pQuestTable[0].dwPID);
			quest::CQuestManager::instance().Login(pQuestTable[0].dwPID);
		}
		else
		{
			quest_login_event_info* info = AllocEventInfo<quest_login_event_info>();
			info->dwPID = ch->GetPlayerID();

			event_create(quest_login_event, info, PASSES_PER_SEC(1));
		}

So, during loading it now checks if the character is in PHASE_GAME.
If not, it calls the quest_login_event timer, which will keep checking and delaying by one second until IsPhase is actually PHASE_GAME, at that point we are safely in-game.

Basically, those two lines of code, the sys_log with QUEST_LOAD and the trigger call, were just copy-pasted into the event if the conditions were met.
So I decided to keep the event and remove the direct login trigger.

			quest_login_event_info* info = AllocEventInfo<quest_login_event_info>();
			info->dwPID = ch->GetPlayerID();

			event_create(quest_login_event, info, PASSES_PER_SEC(1));

Ok, now we will call the event everytime, let's see how it looks

EVENTFUNC(quest_login_event)
{
	quest_login_event_info* info = dynamic_cast<quest_login_event_info*>(event->info);

	if (!info)
	{
		sys_err("quest_login_event> <Factor> Null pointer");
		return 0;
	}

	DWORD dwPID = info->dwPID;

	LPCHARACTER ch = CHARACTER_MANAGER::instance().FindByPID(dwPID);

	if (!ch)
		return 0;

	LPDESC d = ch->GetDesc();
	if (!d)
		return 0;

	if (d->IsPhase(PHASE_HANDSHAKE) ||
		d->IsPhase(PHASE_LOGIN) ||
		d->IsPhase(PHASE_SELECT) ||
		d->IsPhase(PHASE_DEAD) ||
		d->IsPhase(PHASE_LOADING))
	{
		return PASSES_PER_SEC(1);
	}
	else if (d->IsPhase(PHASE_CLOSE))
	{
		return 0;
	}
	else if (d->IsPhase(PHASE_GAME))
	{
		sys_log(0, "QUEST_LOAD: Login pc %d by event", ch->GetPlayerID());
		quest::CQuestManager::instance().Login(ch->GetPlayerID());
		return 0;
	}
	else
	{
		sys_err(0, "input_db.cpp:quest_login_event INVALID PHASE pid %d", ch->GetPlayerID());
		return 0;
	}
}

It delays by one second until IsPhase is PHASE_GAME.
So let’s add an extra check: make it delay by one second until IsLoadedAffect() is true as well.

...
	else if (d->IsPhase(PHASE_GAME))
	{
		if (!ch->IsLoadedAffect()) // fix login event too early than affect load
		{
			return PASSES_PER_SEC(1);
		}

		sys_log(0, "QUEST_LOAD: Login pc %d by event", ch->GetPlayerID());
		quest::CQuestManager::instance().Login(ch->GetPlayerID());
		return 0;
	}
...

That's all.

If you want just the simple fix:

This is the hidden content, please

• Sign In
• or
• Sign Up

 

Edited May 23 by Mitachi

[Mitachi 3392]

If you are using marty files you don't need this additional fix, he's already doing that.

IsLoadedAffect is only set if you actually have an Affect or LoadAffect won't load, which is a problem.
Not only for this fix, but in general, IsLoadedAffect is used in anti-exploit contexts.

It is necessary that they are loaded even if you do not have affect, so, you got another fix there

CDBManager::instance().ReturnQuery(szQuery, QID_AFFECT, peer->GetHandle(), new ClientHandleInfo(dwHandle));

// to

CDBManager::instance().ReturnQuery(szQuery, QID_AFFECT, peer->GetHandle(), new ClientHandleInfo(dwHandle, packet->player_id));

//...
		case QID_AFFECT:
			sys_log(0, "QID_AFFECT %u", info->dwHandle);

			// fix: if there are no affects, make an empty one to send the packet
			if (!mysql_num_rows(pSQLResult))
			{
				TPacketAffectElement pAffElem{};
				DWORD dwCount = 1;

				peer->EncodeHeader(HEADER_DG_AFFECT_LOAD, info->dwHandle, sizeof(DWORD) + sizeof(DWORD) + sizeof(TPacketAffectElement) * dwCount);
				peer->Encode(&info->player_id, sizeof(DWORD));
				peer->Encode(&dwCount, sizeof(DWORD));
				peer->Encode(&pAffElem, sizeof(TPacketAffectElement) * dwCount);
				break;
			}

			RESULT_AFFECT_LOAD(peer, pSQLResult, info->dwHandle);
			break;

 

Edited May 23 by Mitachi