If you don’t mind me asking how does this work with the image handle adornments and stuff?
2 Likes
Any update on this v2?
Great resource, and thanks for opensourcing this!
This has far better effects than I could ever create!
3 Likes
2024 EDIT:
I used to make (unofficial) gears for other people previously, and I would store this module inside the gear for it to be used in VFX, obviously this is terrible practise but it is supposed to be like that because I was supposed to give these “gears” to other people, gears on Roblox have to rely on itself for asset storage and not the game its in so yeah, multiple copies of this gear inside a game would basically just not work well with this module which is why I called out “memory leak” which was the wrong thing to do.
I still use this module to this day and it works fine with minimal memory leaks because I use it properly now lol
I made this edit now cuz people still like this reply even though its baseless and wrong so now if people read this then yeah.
Heartbeat Waiting is pretty harmful to the server and client (Free permanent FPS drop) especially if its used on a long lasting script.
LightningBolt.Loop = game:GetService("RunService").Heartbeat:Connect(function ()
if not (script:IsDescendantOf(game)) then
LightningBolt.Loop:Disconnect();
LightningBolt.Loop = nil
return
end
for _, ThisBranch in pairs(ActiveBranches) do
-- E.c.t
its also pretty useful to return the loop, so it can be stopped manually.
EDIT:
This alternative above WONT stop the full memory leaks, it will just stop it if the modulescript gets deleted
2 Likes
I looked at the code in the linked repository and it’s perfectly fine. It only considers active effects on each iteration, so if there are no active effects it will be a no-op. This is so little compute waste that it’s not worth worrying about.
This makes no sense to me. Whether you connect and disconnect to Heartbeat many times or do that same work in 1 connection, it’s similar workload.
2 Likes
The Memory leaks appear fine once you run it a couple of times or two, but in a full server where the effects are constantly appearing, then it could lag the client/server,
1 Like
I don’t see any potential for the code in the Github repository to memory leak – care to point out the exact line for me?
1 Like
I believe he is talking about the Heartbeat loop. Technically he’s right in that the loop is in the body of your script, so it can unnecessarily hold outside variables (I’m not sure how this behaves currently but I last checked this about two months ago). That can be fixed by wrapping the loop in a do end block so the contexts are separated.
Additionally, I’m not really sure on the details as usually I try to very strongly avoid this, but, having a constant event connection does seem to lead to gradual performance issues, albeit very minor. I think it has to do with the thread scheduler though I’m not familiar with the internals of that, I’m wondering if what happens is the thread scheduler slowly falls behind so it slowly fills up the queue over the course of hours and hours of a server running.
I never have a heartbeat connection open long enough in any of my games to worry about it but I would definitely say that the thread scheduler is wack and you probably shouldn’t have an event connection open 100% of the time because even if the code is costing nothing Roblox seems to have a lot of issues regarding how it queues its threads up internally. You can see this effecting games in terms of general remote calls, but, also you can see this really and I mean really effect games that use spawn for things.
For some reason in a lot of games a few extra active threads that are doing nothing can kill performance in a certain scenario. It’s a pain to test since its not within my control, but, yeah, definitely something to look out for. (If you pay attention to the script’s rate in the activity it will pretty much sit constant at 60/s and I think this mechanism for monitoring these rates is related to the issue)
1 Like
Cool so in short: the point I’m trying to get across to the other person is that leaky code in a Heartbeat connection can obviously cause memory leak issues. The code in OP is not leaky.
It is totally misguided to advice not using static Heartbeat connections to avoid memory leaks. That’s looking for the problem in the wrong place.
Tl;dr Wrap the Heartbeat connection in a do end loop and it fixes a potential memory leak in your code (And maybe consider connecting only when you need it because of thread scheduler jank)
This was meant for this very specific situation so do not assume this is a fix all solution and make your code super messy with a bunch of do end blocks. This is not a magic fix for memory leaks, do not just go around doing this, you will hate yourself.
It’s not anything wrong with the code its more so just to do with stuff that probably hasn’t been looked at too close for quite a while sitting in the engine and being very strange like I mentioned. Its been really annoying for me to figure out a lot of this.
Your ActiveBranches table is referenced in the Heartbeat connection, and its referenced in LightningBolt. Even though you nil out the data in ActiveBranches the garbage collector seems to sometimes be slow to pick up on those cases and sometimes doesn’t seem to pick them up at all when you have an active event like that. Not sure why it happens, but, using weak keys/values will fix this issue and using a do end block will too.
Just to be clear for @CrazyCats84 and anyone else reading my reply above,
I didn’t mean to imply you should avoid Heartbeat. You should just avoid a single constant Hearbeat connection all of the time unless you need to. I say that just due to thread scheduler jank/garbage collector jank, which, changes a lot when they update luau. Having one event that’s firing a lot forever can cause problems after hours of running which is only a concern if that event will be running for that many hours.
The best practice is to connect your event only when you need it, and disconnect it when you don’t (and before you connect a new one). That will avoid most issues beyond ones related to your code. Heartbeat connections and stuff don’t cause performance issues on their own unless you’re running really expensive code in them.
3 Likes
This is talking about ghost problems that may or may not exist. None of this discussion is helpful or originating from an actual issue both of you have seen that the OP should resolve. Adding a do-end to a Heartbeat connection only adds another scope and doesn’t change the memory or performance characteristics of the code. It’s incorrect to suggest this would somehow improve performance or memory usage in the way you suggest, as a blanket statement or just in general.
File a bug report if you see anything wrong with static signal connections that run every frame.
@OP I recommend ignoring the advice given in recent posts.
4 Likes
a do-end won’t fix anything what
i have no idea where you go that idea but that’s not true at all
For some reason I thought you were the OP, that’s my bad. I am going to edit my messages here to be a little more clear on what I’m trying to say as I am not suggesting anyone follow this as a blanket statement, its not intended to be a blanket statement at all.
I should clarify I mean in this very very specific case the do end block has had the potential to fix a few very specific issues from at most about three weeks ago and I am not aware if they are still in the engine or not.
It’s a bit hard to explain why it works because to be fair I don’t know the full details in and out, all I know is I have created a test where I can consistently get a hard pass or fail over at least about a 5 minute period.
The technical explanation of it is when you wrap something in a do end block it seems to work like an upvalue where its taking the value from the higher context in to the lower context. Without the do end block the engine treats the context as being the same and this in combination with an event connection can cause data to persist for (at least) a very long time under very specific circumstances, don’t go around adding do end blocks in your code please guys. 
Read the last paragraph in my previous post. (That’s the best explanation I can give)
Please don’t add random do end blocks in your code because of my suggestion. (It will not generally help you. I am serious.)
1 Like
There’s been a few claims around the devspace the past few months saying that this module is prone to memory leaks. I decided to run a continuous loop on my client overnight to see if there was any merit to these claims. Spoiler alert: there was none
After creating one bolt, spark, & explosion every 4 seconds over the course of 10 hours (rendering ovar 9000 of each on my client); this was my result upon waking up this morning
Here is the repro file. It’s nothing special, but the point was to test them in isolation.
lightning_quasiduck.rbxl (34.4 KB)
If you use these modules correctly, you will not experience memory leaks. Please do not make baseless claims about others creations.
It won’t help you but it’s also no detriment either. Like buildthomas said, it’s just adding a scope; neither performance nor memory are effected. Policing others style practices is… weird, to say the least
8 Likes
I’m not referring to a style practice or saying its bad to use a lot of do end blocks (I use them a lot in very big pieces of code) I’m just clarifying that my post is not encouraging people to add a bunch of random do end blocks everywhere just because it might help in the specific case I mentioned. See the conversation below my post, that explains why I made that edit.
There’s nothing wrong with using them or using them for organization, I use them all of the time in plenty of places for organization and for shortening scopes because of their properties with the GC.
And yeah, absolutely no performance/memory cost to these to worry about, it already costs less than an if statement or for loop and in most cases I think it should cost even less than allocating a local afaik, its really not a bad thing to use even in large quantity.
To explain how and why a do end block can be useful since I didn’t really go into that before, here’s a little rundown. A do end block creates an extra scope, which, basically means you can define local values inside and they exist only inside the do end block (inside its scope). The reason that’s useful is because the garbage collector works scope-wise, stuff inside of a scope that’s been passed over already is considered out of the scope of any executing code, because, well, it is.
Sometimes the way that the GC handles scopes is wacky when it comes to other cases and it seems to very rarely change but that prior fact should never change, even if how the GC handles out of scope data does change.
I’m not sure how that effects GC performance but I would think it might help stability if the GC can GC variables earlier, it would have less variables to worry about later, less to deallocate at once, and scopes would theoretically become free quicker. On the other hand I guess too many scopes to take into account might be bad but again I don’t really know how that works, it shouldn’t be a problem.
Can I ask why I cannot delete the bolt after a certain amount of time? I try to use LightningBolt:Destroy() as shown below but it just throws out an error.
local LightningSparks = require(game.ReplicatedStorage.LightningBolt.LightningSparks)
local LightningExplosion = require(game.ReplicatedStorage.LightningBolt.LightningExplosion)
local LightningBolt = require(game.ReplicatedStorage.LightningBolt)
local function zap(waittime, part1, part2, a1, a2, debris)
print(60)
local ranCF = CFrame.fromAxisAngle((part2.Position - part1.Position).Unit, 2*math.random()*math.pi)
local A1, A2 = {}, {}
local timeElapsed = 0
A1.WorldPosition, A1.WorldAxis = a1.WorldPosition, a1.WorldAxis
A2.WorldPosition, A2.WorldAxis = a2.WorldPosition, a2.WorldAxis
coroutine.resume(coroutine.create(function()
while true do
if timeElapsed == waittime then break end
local NewBolt = LightningBolt.new(A1, A2, 20)
NewBolt.PulseSpeed = 2
NewBolt.PulseLength = 0.5
NewBolt.FadeLength = 0.25
NewBolt.Color = Color3.new(math.random(), math.random(), math.random())
--NewBolt.MaxRadius = 1
wait(0.1)
timeElapsed = timeElapsed + 0.1
coroutine.resume(coroutine.create(function()
wait(debris)
LightningBolt:Destroy(NewBolt)
end))
end
end))
end
game.ReplicatedStorage.Effect.OnClientEvent:Connect(function(waittime, part1, part2, a1, a2, debris)
zap(waittime, part1, part2, a1, a2, debris)
end)
1 Like
Is there any way to tween the values used in the lighting bolt? to get things like a smooth thickness transition or a smooth transparency transition?
Nevermind, found an alternative solution to that issue.
has a bad memory leak! 
Proof? Have you got a rbxl that I can go into and take a look?
Also, what version is this for? Both v1 and v1.1 should have no memory leaks as far as testing goes.
The only documented bug (with actual proof) I’ve heard of is LightningSparks sometimes deleting slower than usual which will be fixed in v2 anyway. Regardless, even in v1 it does not lead to an unbounded increase in memory usage for LightningSparks.
If you find anything in v1.1, PR’s are also welcome but otherwise, document the issue on github and I’ll get around to it.
6 Likes
I’m not sure who “pineapple” is. Why didn’t “pineapple” just tell you the specific memory leak to post here or on github as an issue?