Is this distance animation code okay?

Help and Feedback Code Review
#1

What my code does:

  • Player clicks on another player.
  • RemoteFunction invoked and it checks if the distance between Player1 and Player2 are under or equal to 100.
  • If it is, then it returns true which the localscript recieves as “Confirmation”.
  • If “Confirmation” is confirmed with true, then it will play the animation.

Video:

Local Script:

image
image780×405 53.5 KB

Server Script:
image

Please share your knowledge if there’s any better ways to do this or improve it. Or share how you would do it.

2 Likes
#2

Looks good. You could also try to check [on the server] the type of MouseTarget and more sanity checks if you want.

2 Likes
#3

Hi,

Overall, great stuff! However, I would re-consider a few plausible problem scenarios:

  1. When you cast a ray with Mouse.Target or similar, you are shooting a ray out as far as it can go. In contrast, the Raycast method offers a maximum distance that you can set. This distance check by itself could be used in the client validator for when to play the animation.

  2. You are waiting for the server for validation before the animation is played. This may get slow when waiting for a server response, and since the animation is played through the client anyway, if an exploiter really wanted to, they could just play the animation right away.

  3. Try to make sure you use GetService and then WaitForChild for remotes to load in, in your case ReplicatedStorage, server and client side.

  4. Raycast offers better part filtering, so you can filter out multiple parts and models in a list rather than only ignoring that 1 model and its descendants. So, you can ignore your own player model (which you should do) and also for example certain projectiles / other that happen to stand in the way of the mouse ray.

So, something along the lines of what was written in this topic:
Part pointing towards mouse sometimes goes backwards - Help and Feedback / Scripting Support - DevForum | Roblox

4 Likes
#4

Hi, I appreciate you sharing suggestions with me! but although I do have a few questions.

You said that if an exploiter really wanted to, they can just play the animation right away. But I also heard that playing animations on the client is far better than playing them on the server.

If you dont mind answering, Is there really any way to prevent this? because what if I connect a :GetMarkerReachedSignal() to the same animation which will send something to the server as soon as it reaches that animation event.

An exploiter can just play the animation and that :GetMarkerReachedSignal() connected to that animation would still run.

If you know any solutions or suggestions to prevent this, please let me know!

1 Like
#5

It is unnecessary to round the distance. Reducing one function call is better.

Also, it is probably unnecessary to do anything on the server if you are doing client animations.

2 Likes
#6

Why not just check the distance on the client instead?

1 Like
#7

Definitely remove the InvokeServer call, it is incredibly slow to wait on the server especially for something like a distance check.

An exploiter does not need to play the animation to send GetMarkerReachedSignal, they can just send it. You need to validate on the server connection that the player is allowed to do what they want to do. We would need to see your remote event code and more context to help prevent exploits. In general the client must ask the server to do something, all events must be “can I get 100 bobux” not “give me 100 bobux”.

1 Like
#8

This does not prevent exploiters and the exploiter could just play the animation right away without the check.

1 Like
#9

You should add a check on the server to see if MouseTarget actually exists, just so it doesn’t error if an exploiter sends an invalid parameter.

1 Like