It’s definitely not crazy to change state using events, and have a dedicated loop handling the visuals like this.
However, I would remove any yields (e.g. WaitForChild) from the update loop and maybe use BindToRenderStep instead of while wait().
For the sake of terminology:
-
You (sort of) have an “immediate mode GUI”, where you update properties every frame based on some state
-
You’re considering switching to a “retained mode GUI”, where you update properties only when they change
Both work. Immediate mode tends to make nicer code that’s very declarative and easy to change, but can be slower (relatively—it’s fast enough).
The downside, and why maybe I would suggest switching to a retained-mode system, is that roblox’s GUI APIs are all retained mode anyways. Things like creating/destroying instances or using TweenService are going to be hard with the loop-based method.
Also, you have to be careful that all your if-branches set all the properties you care about. If you had something like:
while wait() do
if on then
frame.Color = Color3.new(1, 0, 0)
end
end
Now your frame will turn red when on is true, but will never undo that change. You have to start with a default:
while wait() do
frame.Color = Color3.new()
if on then
frame.Color = Color3.new(1, 0, 0)
end
end
or do something like:
while wait() do
frame.Color = on and Color3.new(1, 0, 0) or Color3.new()
end
that will probably get tedious as your GUI gets more complicated. You could wrap some of that in a framework—that’s what roact does to get a usable state-based GUI API in roblox.