[Bug]: Going 'sleep' MAY crash server

[gesior]

By submitting this bug issue, you agree to the following.

• This is a bug in the software that resides in this repository, and not a support matter (use https://otland.net/forums/support.16/ for support)
• This issue is reproducible without changes to the C++ code in this repository
• This bug has not been resolved in master branch
• There is no existing issue for this bug already

Does this bug crash tfs?

yes (not always)

Server Version

1.7 (Master)

Operation System

all (listed below)

OS Description

No response

Bug description

Going 'sleep' MAY crash server

Possible Pull Requests which are to blame

No response

Steps to reproduce

1. Click on bed (can't check it on 12+, but it may execute C++ action pointed in Actual Behavior)
2. Wait until someone 'wrap' bed item.
3. Try to start sleep.

Actual Behavior

I get report from modified TFS. I cannot run 12+ client on Linux. Can someone confirm it?

This:
https://github.com/otland/forgottenserver/blob/master/src/actions.cpp#L337
sets bedItem in Player to BedItem*:
https://github.com/otland/forgottenserver/blob/master/src/player.h#L226
but does not 'increase pointer' [old TFS] (or use Shared Pointer).

If it opens Dialog window and in meanwhile someone 'Wrap' BedItem (remove ite), it will crash, when someone close Dialog around that line:
https://github.com/otland/forgottenserver/blob/master/src/game.cpp#L5655

Expected Behavior

Not crash.

Backtrace

No response

[nekiro]

It will probably reproduce when deleting the bed while someone is having the modal open too.

[gesior]

It will probably reproduce when deleting the bed while someone is having the modal open too.

I've tested it on clean TFS 1.4. I cannot remove BedItem that is inside house, because it's blocked here:

forgottenserver/src/bed.h

Line 23 in 7573e41

bool canRemove() const override { return !house; }

Also transform to other BedItem ID does not remove item and create new as it does for ex. stackables.

To reproduce that crash on TFS 1.4 I had to first change BedItem into ID that is moveable (backpack item) and will be removed when dropped on water, and then teleport it to water position, to make engine remove it:

bedItem:transform(1998)
bedItem:moveTo(Position(132, 476, 7))

---

IDK how Wrap works on new TFS, but if it's possible to Wrap beds, then it must somehow bypass BedItem::canRemove() check.
Anyway, fix it pretty simple:
gesior@202dcf7
If BedItem is moved/removed from player screen, it will set it to nullptr for player who has open offline training modal.
To open that modal, player must stand next to BedItem (use it), so it should fix every scenario.

[Zbizu]

the problem for movable beds is that the stored memory address is no longer valid for the player that tries to sleep in the situation when the house owner wraps the bed and logs out

this is why beds are not possible to remove in older game versions

the solution I used was to store bed itemId and position when the dialog is invoked. Once the player confirms his choice, my server fetch item id from position and checks player distance to the bed. If either check fails, a return is hit.

@gesior you may reproduce this in old tfs by always returning true in canRemove() and doing /r on the bed when player2 has the modal dialog open

[gesior]

you may reproduce this in old tfs by always returning true in canRemove() and doing /r on the bed when player2 has the modal dialog open

I don't want to crash server. I want to make server crash free.
I found some Lua - pretty crazy - scenario that let me crash TFS 1.4 with BedItem and it works without C++ changes in TFS 1.4.

the solution I used was to store bed itemId and position when the dialog is invoked. Once the player confirms his choice, my server fetch item id from position and checks player distance to the bed. If either check fails, a return is hit.

It may be better solution than mine. Mine works for sure in 'normal scenario' (use item on screen of player), but it wastes some CPU for each item move (for each item move on player screen). Yours executes only, when player uses BedItem for offline training, so it does not affect normal item movement. It may save some 0.01% CPU on popular server.
I will rewrite BedItem code to your solution on my TFS 1.4 branch in next few weeks.