onPlayerTeleport false positive with default resources

[ghost]

Describe the bug

Hello!

We've tested this new event and it gives false positives with interiors and freeroam (F2 menu) resources. Didn't test other resources.
Another thing:

it would be cool when the event triggers, to get the position, interior, dimension from & to where the player teleports as params.

Steps to reproduce

1. start interiors or freeroam
2. copy & paste these lines into a script file, then start it

addEventHandler("onPlayerTeleport", root, function() 
    outputServerLog(getPlayerName(source) .. " teleporting.")
end)

3. enter into an interior anywhere on the map. Or press F2 and teleport anywhere on the map.

Version

Client & Server: 22934

Additional context

No response

Relevant log output

Security Policy

• I have read and understood the Security Policy and this issue is not security related.

[imfelipedev]

Describe the bug

Hello!

We've tested this new event and it gives false positives with interiors and freeroam (F2 menu) resources. Didn't test other resources. Another thing:

it would be cool when the event triggers, to get the position, interior, dimension from & to where the player teleports as params.

Steps to reproduce

1. start interiors or freeroam
2. copy & paste these lines into a script file, then start it

addEventHandler("onPlayerTeleport", root, function()
outputServerLog(getPlayerName(source) .. " teleporting.")
end)
3. enter into an interior anywhere on the map. Or press F2 and teleport anywhere on the map.

Version

Client & Server: 22934

Additional context

No response

Relevant log output

Security Policy

• I have read and understood the Security Policy and this issue is not security related.

Thanks for the report! The false positive happens due to improper use of the setElementPosition function on the client-side, causing the trigger to fire incorrectly. I've submitted a PR to fix this in Freeroam script (multitheftauto/mtasa-resources#599) and I'm currently refactoring the interiors script.

[CArg22]

What is the point of doing it? now cheater just use this newly created trigger to teleport ...

[imfelipedev]

What is the point of doing it? now cheater just use this newly created trigger to teleport ...

In the Freeroam script, any player can teleport anywhere. As discussed in the PR for the event addition, it was designed to help developers and server owners identify cheaters who teleport without any server-side action, not to protect the use of the setElementPosition function on the client side.

[ghost]

Thanks for the report! The false positive happens due to improper use of the setElementPosition function on the client-side, causing the trigger to fire incorrectly. I've submitted a PR to fix this in Freeroam script (multitheftauto/mtasa-resources#599) and I'm currently refactoring the interiors script.

Some of the set* functions like setElementPosition shouldn't work on clientside anyway / shouldn't work with players, (non-local elements)

I didn't check what other default resources are affected by this, I only tested freeroam & interiors

[CArg22]

Thanks for the report! The false positive happens due to improper use of the setElementPosition function on the client-side, causing the trigger to fire incorrectly. I've submitted a PR to fix this in Freeroam script (multitheftauto/mtasa-resources#599) and I'm currently refactoring the interiors script.

Some of the set* functions like setElementPosition shouldn't work on clientside anyway / shouldn't work with players, (non-local elements)

I didn't check what other default resources are affected by this, I only tested freeroam & interiors

Maybe worth to add some option to set element authority, for example you can set that given element can be edited only from server side

[ghost]

Maybe worth to add some option to set element authority, for example you can set that given element can be edited only from server side

Lets not off-topic this thread more. A proposal issue about disabling clientside set functions for non-local elements should be opened anyway and we could argue/agree on if this would be okay or not. (we could also talk about what should be done with these functions, for example your idea)

@imfelipedev
So we have tested this thing a bit longer today.

If there is a cheater who can change his/her position, then that cheater can also add himself/herself a vehicle or just get into an already spawned one and teleport with that, this way it won't get triggered.

As I noted in the first post, knowing the positions from & to the player teleports would be more comfortable than any ugly way of retrieving these uppon this event trigger. Getting these as event params for the handler function. Dimensions and Interiors can also be passed for more comfort but not important.

function handlePlayerTeleporting(fromX, fromY, fromZ, toX, toY, toZ, fromDim, fromInt, toDim, toInt) -- fromDim, fromInt, toDim, toInt are not important to add
    iprint(source) -- player who triggered
    -- TODO
end
addEventHandler("onPlayerTeleport", root, handlePlayerTeleporting)

I don't know how will you solve the vehicle teleporting problem, (if you plan to 'fix' it) but I think that this event in this form is not ideal.

[imfelipedev]

Maybe worth to add some option to set element authority, for example you can set that given element can be edited only from server side

Lets not off-topic this thread more. A proposal issue about disabling clientside set functions for non-local elements should be opened anyway and we could argue/agree on if this would be okay or not. (we could also talk about what should be done with these functions, for example your idea)

@imfelipedev So we have tested this thing a bit longer today.

If there is a cheater who can change his/her position, then that cheater can also add himself/herself a vehicle or just get into an already spawned one and teleport with that, this way it won't get triggered.

As I noted in the first post, knowing the positions from & to the player teleports would be more comfortable than any ugly way of retrieving these uppon this event trigger. Getting these as event params for the handler function. Dimensions and Interiors can also be passed for more comfort but not important.

function handlePlayerTeleporting(fromX, fromY, fromZ, toX, toY, toZ, fromDim, fromInt, toDim, toInt) -- fromDim, fromInt, toDim, toInt are not important to add
iprint(source) -- player who triggered
-- TODO
end
addEventHandler("onPlayerTeleport", root, handlePlayerTeleporting)
I don't know how will you solve the vehicle teleporting problem, (if you plan to 'fix' it) but I think that this event in this form is not ideal.

I think it's totally valid to block functions like this on the client side. The problem is that it would break many scripts, and I'm not sure if something like that would be accepted by the team. As for vehicle teleportation and parameters, I'm working on that.