Forgottenserver: [Error - MoveEvent::executeStep] Call stack overflor

[brunominervino]

If anyone interested, I was trying this:

bool MoveEvent::executeStep(Creature* creature, Item* item, const Position& pos, const Position& fromPos)
{
    //onStepIn(creature, item, pos, fromPosition)
    //onStepOut(creature, item, pos, fromPosition)
    if (!m_scriptInterface->reserveScriptEnv()) {
        std::cout << "[Error - MoveEvent::executeStep] Call stack overflow" << std::endl;
        return false;
    }

    ScriptEnvironment* env = m_scriptInterface->getScriptEnv();
    env->setScriptId(m_scriptId, m_scriptInterface);

    lua_State* L = m_scriptInterface->getLuaState();

    m_scriptInterface->pushFunction(m_scriptId);
    LuaScriptInterface::pushUserdata<Creature>(L, creature);
    LuaScriptInterface::setCreatureMetatable(L, -1, creature);
    LuaScriptInterface::pushThing(L, item);
    LuaScriptInterface::pushPosition(L, pos);
    Tile* tile = g_game.map.getTile(fromPos);
    Player* player = creature->getPlayer();
    LuaScriptInterface::pushPosition(L, !tile->getGround()->isBlocking ? fromPos : player->getTemplePosition());
    return m_scriptInterface->callFunction(4);
}

It is not working, but it can be a start ^^

[vankk]

In chest quests can have it too, right? Doing the same thing.

[brunominervino]

Anywhere that has fromPos, it happens.

[ghost]

well it was noticed : P

[darkjav]

Yes, i had the same error, and solved with "not-logout" zone there...

[ranisalt]

I think we could executeOnStep on login and, if it returns false, teleport to temple. But it should happen before the player is connected so he does not see the teleport happening.

I'm making my tries.

[PrinterLUA]

I dont understand how he end up there, did you login near that position that has a fromPosition code or what?

[ranisalt]

@PrinterLUA that tile is walkable (walkable water), but has a callback and you shouldn't normally be able to step on it. Problem is the logic that tries to find a walkable position around you if your position is occupied.

[PrinterLUA]

@ranisalt @marksamman
Ok i've tested, i can confirm the crash. When they login and get positioned on tile that has a script which teleport fromPosition.

Suggestions:

1. We should not be able to login on tile with aid or uid (In this case tp to other tile or temple)
2. Should login on the tile you logged out, instead login on a free tile. (No idea if that is client-sided.)
3. Should use return values, such as false and true. Instead of teleport fromPosition (But no idea if that would help aswell.)

[marksamman]

Your suggestions are mostly workarounds (although suggestion 3 is on our roadmap anyway). This worked fine in TFS 1.0, and perhaps 1.1 too, we somehow broke it and it needs to be fixed so it works the same way it worked in the past.

[PrinterLUA]

I thougt we had this bug from the start, but if you sure it worked on the later versions. I can analyze tfs 1.0 and try. Since there have been tons of edits since then, it will be bit difficult to narrow it down. Do you have any idea, where i should keep an extra eye on?

[marksamman]

I suspect that 2550261c2001fa98c64b72c1e0c7c6c13b5d45a8 is causing this.

[vankk]

Mark, I fixed this problem using the fix made by bruno a few months ago, dbb76ed16.

Ignore the changes made unded the movement.cpp

[marksamman]

That commit essentially reverts 2550261c2001fa98c64b72c1e0c7c6c13b5d45a8, so that's probably the commit causing this then. Ideally we would fix this by achieving the same behavior we had with lastPosition, without reintroducing lastPosition.

[PrinterLUA]

@marksamman
I can confirm that PR has to do something with the overflow. Working like a charm on TFS 1.0.

[Kamenuvol]

I agree it should execute onStep on login because you are actually stepping on the tile

[gesior]

My friend installed this fix: https://github.com/otland/forgottenserver/commit/2550261c2001fa98c64b72c1e0c7c6c13b5d45a8
but it did not fix anything. You can still login 5 MCs and login on that tile. Then you make step to the right and BAM, crash.

Don't you think that for time of login/search of 'free' position there should be 'lastPosition' [do not remove it!] equal to 'Town Temple Position', so every script like 'walkback.lua' would kick player to temple as it should do?

If you see any posibility of abuse please post.. or write commit with these changes, so we can all test it on otses :)

[gugahoa]

The way I see it, the proper solution would be the enhancement of onStepIn and onStepOut callbacks to accept return values. Other suggestions seems like just workarounds.

[marksamman]

If we can achieve the previous behavior, that's good enough for the 1.2 release. Ideally we would do that without just reverting the changes that I made in 2550261c2001fa98c64b72c1e0c7c6c13b5d45a8. Worst case, if this ends up being the only issue that blocks the 1.2 release, we'll just revert the changes in 2550261c2001fa98c64b72c1e0c7c6c13b5d45a8 that cause this.

[gesior]

I will repeat, because I don't see any problem with this solution:

Don't you think that for time of login/search of 'free' position there should be 'lastPosition' [do not remove it!] equal to 'Town Temple Position', so every script like 'walkback.lua' would kick player to temple as it should do?

First we should try to spawn player on tile without onStepIn script. Players can abuse it to teleport friend without real trap (just 1 player stand on logout position and they know which tile is first 'checked' by algorithm, so they know that walkback.lua will teleport player to temple)

Algorithm [c++ part] 'on login':

set player last position to player temple position

try to spawn player on his position from database

if player not spawned

• - - for each tile around
• - - - - - if there is no movement script
• - - - - - - - - try to spawn player on that position

if player not spawned

• - - for each tile around
• - - - - - try to spawn player on tile [if onStepIn 'return false', then check another tile]

if player not spawned

• - - spawn player in temple

[HeavenIsLost]

@marksamman this is the only thing blocking the 1.2 release?
On my own OT the solution was just checking if fromPosition is equal to toPosition. If its equal, teleport to the temple:

if checkMovementLogin(player, position, fromPosition) then
    player:teleportTo(player:getTown():getTemplePosition())
    return true
end

I have added this to walkback.lua and similar scripts.

Another fix is to change tile::postAddNotification

Position fromPos = getPosition();
if (oldParent) {
    fromPos = oldParent->getPosition();
} else if (creature && creature->getPlayer()) {
    fromPos = creature->getPlayer()->getTemplePosition();
}

//calling movement scripts
if (creature) {
    g_moveEvents->onCreatureMove(creature, this, fromPos, MOVE_EVENT_STEP_IN);
} else if (item) {
    g_moveEvents->onItemMove(item, this, true);
}

This will just make the fromPosition equals town position when player login and will fix all scripts. As far i can tell, only on login/placeCreature a creature onStepIn event got fromPosition = toPosition.

[vankk]

I fixed this issue by reverting 2550261.

[PrinterLUA]

@marksamman
Any progress in this issue? We have few suggestions, but none seems appealing?

[marksamman]

Not really. The suggestions sound interesting, but at this point it's probably better to revert the change that caused this and work on something better for 1.3.

[Kamenuvol]

There is only this issue blocking 1.2 release, at least in milestones. Will 2550261 be reverted?

[gugahoa]

Considering @marksamman comment at #1810, it will be reverted.

[marksamman]

I've reverted it in #1932, please confirm that it fixes this.