Need help with making a solid foundation for creating magic attacks

Help and Feedback Code Review
#1

Include a standalone, bare-bones rbxl file with only the code you want reviewed.
FireMagicTool.rbxl (30.0 KB)
(it does come with required stuff like save files and replication event)

Hold Q with tool out to shoot a fireball.
If it seems kind of glitchy, like how your character moves when starting the attack, don’t worry about it cause it works fine in the actual game.

Provide an overview of:

  • What does the code do and what are you not satisfied with?
    How it’s incredibly bloated and messy.

  • What potential improvements have you considered?
    Moving stuff over to replicatedfirst and have it replicate from there would lower lag upon the tool being given to player, but it will make the game even more messy and complicated. I have also thought about making everything a local script but I don’t actually know how to go about using them in a way that will be clean and easy to work with.

  • How (specifically) do you want to improve the code?
    Make it cleaner, make it more efficient, leave it easy to add to for the future. More specifically, I would like to quickly be able to modify stuff on the spot (Such as attack behavior and visual effects) without having to completely rewrite large chunks of code. I wanted to be as simple and easy to add to for the future but this more or less backfired. I don’t know what to do now and I don’t really want to have to come up with another solution only to have it backfire again.

If anything’s unclear or needs clarification, please reply. I tried to include readme’s inside of the tool but I do realize they may not be sufficient for just how bad it is. Also please keep in mind that I am by no means an expert scripter, so if you find something that looks weird it’s probably not by choice but by me not knowing that there’s a better way to do it, so telling me about it would be greatly appreciated.

edit:Clarity

2 Likes
#2

I haven’t seen your code, but I’m not sure moving items to replicated first is going to reduce lag. ReplicatedFirst is primarily used for loading screens as it’s quite literally replicated first.

Anything like projectiles and effects dont need to be on the client anyways. Neither should damaging, but I assume you know that.

2 Likes
#4

Again,
I need help with making the code in the fireball tool efficient, not how it looks like.
@GreekForge as well, I don’t want to move stuff to replicatedfirst until I find a better way to organize my stuff or else I’ll end up having to completely scrap it yet again

The effects absolutely needs to be done on the client too as the code has a bunch of tweens for the attack charging (Which I have seen lag the game to an absolute crawl) and aiming the attack would lag if I had to fire a remote event every single frame.

Most importantly, the projectiles really need to be on client because to have them hit the players accurately you need to have the physics be calculated by the server which makes it lag visually for the player firing it. So I fire a fireball locally, with all of the special effects of the fireball whilst firing a real one in the server. This makes the fireball move accurately on everyone’s screen regardless of lag.

#5

This movement still needs to be replicated to the server if you are going to calculate projectile movement client side. Honestly I would just use bodymovers since physics is already replicated and saves a lot of code. TweenService is really only used for high speed projectiles that require a high level of collision detection.

1 Like
#6

One thing that developers do a lot is they like to lock themselves into systems. The problem with this is that they literally fight you in future cases where you want your code to do something else, but cannot support it because of your system. A few notable comments:

  • Folder condensation - There’s a lot of unnecessary structuring going on here. Folders inside folders that provide no immediate meaningful detail as to what said folder does is bad practice and tedious work in the long run.
    RobloxStudioBeta_2019-08-27_08-37-37
  • Unnecessary Replication - From what I’m trying to understand, your Local folder here contains vital local objects. These should not be under a tool. For every tool you create, it’ll create every children. If you’re just going to clone something like a fireball model, you might as well have an asset dump in ReplicatedStorage, or make an asset fetcher (advanced). On top of that, you have properties here that dictate size, offset, and rate. This is exploitable if Local is a client-sided folder that relies on those properties to give the server a final output. Lastly, those properties suggest that you’re always going to have a rate, max size, and offset. Are you absolutely sure that everything you’re making (or cool potential ideas that you could make) are going to have those properties?
    image

As for efficiency of your actual code, it’s very fundamental and I don’t really see how you can make it more efficient. You can condense all of this code under one server and local script though, I’m not sure why you have a separate script “InputHandler” just to connect to your Q event.

There is an issue with that input handler though. The server should never really try to understand what state your keys are in, that’s a client thing. Instead you should try seeing it as an “action” and that “action” fires it’s action function, which in turn talks to the server if needed (this is also how you don’t lock yourself into systems too btw). Because it’s working this way, and you don’t have preventative measures for how often the state changes, it’s highly exploitable by just brute forcing the remote event. Observe!

I hope this has been educational for you! Thank you for listening to my Ted Talk. :slight_smile:

2 Likes
#7

Thanks for the input! There are currently no cooldowns in place for the fireball (so you can just spam q to get same results as your video), but once it will be implemented, spamming the fireball by brute forcing the remote event should be impossible. I haven’t actually added anything to make the fireball functional as an attack yet.

The proprieties for offset, size and rate are all for the circle that appears. It is under the “Local” folder, so it is purely per client, even if someone were to modify their values to have a gigantic size it would appear normal for every other player. However, now that I think about it, it would be more efficient to just set the max size, offset and rate as their maximum size and have them start off as smaller values. I am also pretty sure they will all use these proprieties, but I am sort of worried about how to handle adding more visual effects like trails and such.

Let me explain the structure of the folders : Q is for the key that gets pressed, it automatically gets found when a key gets pressed. This is to make it easier to make new attacks. From there, the local and server folders both contain things to replicate to the clients and to the servers individually. Player is purely for things like animations to play when the player is attacking. Inside of the parameters folder, it checks for the value of the stat in the player’s save folder and then finds the highest amount to that, so the attack can change size.

Inside of that are two more folders, another local and server folder, both for the client and server script respectively. The folders inside of those can be named anything, but if you add more more will be added visually. So if you had, say, 3 folders inside of local each with their own offset, the player would end up with 3 circles. If you had 3 folders with the fireball inside of them in server, the player would shoot out 3 fireballs, if it makes sense.

I tried my best to explain it, if you know a better way to organize it for the same purpose as this please let me know.

Finally, can you go into details about the asset fetcher thing? I’d like to make things as efficient as possible before I go making a bunch of stuff I have to get rid of due to some minor oversight in the future.

@cc567 The “charging circle visual effect” tweens, the fireball is using bodymovers. The fireball wouldn’t be lagging on the client if the server wasn’t using bodymovers.

#8

Ahh ok I see.I was under the impression that you were using tweenservice for projectile movement. Good luck with this project. I’ve dabbled with magic powers myself and my best advice is to make sure the result pleases you. Not just anyone else. Chances are your standards are much higher!