If anything can be done a little differently, that would be great!
If everything seems normal in the code below, please let me know!
for CupName, CupData in pairs(GrandPrixInfo) do
local GrandPrixButton = script.Example:Clone()
GrandPrixButton.Name = CupName
GrandPrixButton.CupAlias.Text = CupData.Alias
GrandPrixButton.CupImage.Image = CupData.Icon
GrandPrixButton.Parent = Frame.ContentBox
GrandPrixButton.TextButton.MouseEnter:Connect(function()
TweenService:Create(GrandPrixButton.UIStroke, ColorChangeInfo, {Color = Color3.fromRGB(254, 195, 9)}):Play()
local IsShowing = true
for MapID, MapName in pairs(CupData.Maps) do
local MapData = MapInfo[MapName]
Frame.Sidebar.Maps[MapID].Text = MapData:GetAttribute("Alias")
end
local MouseLeave: RBXScriptConnection
task.spawn(function()
while true do
for MapID, MapName in pairs(CupData.Maps) do
local MapData = MapInfo[MapName]
Frame.Sidebar.MapImage.Image = "rbxassetid://"..MapData:GetAttribute("MapImage")
Frame.Sidebar.MapImage.TextLabel.Text = MapData:GetAttribute("Alias")
task.wait(1)
if not IsShowing then
break
end
end
if not IsShowing then
MouseLeave:Disconnect()
break
end
end
end)
MouseLeave = GrandPrixButton.TextButton.MouseLeave:Connect(function()
IsShowing = false
TweenService:Create(GrandPrixButton.UIStroke, ColorChangeInfo, {Color = Color3.fromRGB(255, 255, 255)}):Play()
end)
end)
end
I think the reason that the code feels a bit hacky is because of the looping within the MouseEnter connection + disconnecting the MouseLeave connection.
What I’d recommend is getting the loop out of the MouseEntered function and have a variable determine what maps the loop should be filtering through
local currentCupData = nil
for CupName, CupData in pairs(GrandPrixInfo) do
local GrandPrixButton = script.Example:Clone() -- set properties
GrandPrixButton.Name = CupName
GrandPrixButton.CupAlias.Text = CupData.Alias
GrandPrixButton.CupImage.Image = CupData.Icon
GrandPrixButton.Parent = Frame.ContentBox
GrandPrixButton.MouseEnter:Connect(function()
currentHover = currentCupData
... -- tween service, set text labels etc
end)
GrandPrixButton.MouseLeave:Connect(function()
currentHover = nil
... -- tween service, text labels
end)
end
while true do
if currentCupData then
... -- go through currently hovered maps and show them
end
wait(1)
end
This solution doesn’t seem to match what @BillB1ox is trying to do, or at least it’s not generally true. As the loop runs in another thread, the “map updates” will be run with some delay < 1 second after the button is clicked, which means a wrong map will be shown for some time. In a more general way, intervals are commonly necessary and alternative solutions do not always fit. The approach @BillB1ox is a valid solution. This is not exactly the way I’d do it though, but the idea is right. The only thing I find weird is that you’re disconnecting the MouseLeave connection in the loop instead of directly in the event listener, which avoids the second not IsShowing condition.
personally I’d use metatables, here’s a way to do it (could be much better if you used only 1 metatable to handle it all but I am lazy but lmk if u want me to do that)
local info = {
hello = {maps = {'test', 'test2', 'test3'};};
there = {maps = {'test4', 'test5', 'test6'};};
};
local mapinfo = {
test = {icon = Color3.new()};
test2 = {icon = Color3.new(1,0,1)};
test3 = {icon = Color3.new(0,0,1)};
test4 = {icon = Color3.new(1/2,1/2,1)};
test5 = {icon = Color3.new(1/4,1/4,0)};
test6 = {icon = Color3.new(0,1/4,1/4)};
}
local function count(t)local c=0; for _, x in next, t do c+=1;end;return c;end; local max = count(mapinfo);
local par = script.Parent;
local frame = par.frame;
for name, data in next, info do
local button = par[name];
local showing = false;
local tracking_table = setmetatable({}, {
__newindex = function(self, i, v)
if v == 0 or not showing then
print('stop map selection...');
frame.BackgroundColor3 = Color3.new(1,1,1);
return;
end;
local show = v > max and 1 or v;
local name = data.maps[show];
local info = mapinfo[name];
print('selected map:', name);
frame.BackgroundColor3 = info.icon;
task.delay(1, function()
if not showing then
return;
end;
self[i] = show+1;
end);
end;
});
button.MouseEnter:Connect(function() showing = true; tracking_table[name] = 1; end);
button.MouseLeave:Connect(function() showing = false; tracking_table[name] = 0; end);
end;
This code is, conceptually, about the same as what they have done, except this one has a concurrency bug and the actual implementation is terrible. You’re using a metatable in place of a recursive function which is overkill, and even then it cannot work as expected because the state of a “thread” is shared by multiple threads.
Since I have not explicitly proposed a neat solution, I will now do so. task.cancel has lately rolled out into the engine and it will be a perfect fit for this case.
local thread
for cupName, cupData in pairs(GrandPrixInfo) do
-- ...
GrandPrixButton.MouseEnter:Connect(function ()
if thread then task.cancel(thread) end -- handle this case as well
thread = task.spawn(function ()
-- your loop here
end)
end)
GrandPrixButton.MouseLeave:Connect(function ()
if thread then
task.cancel(thread)
thread = nil
end
end)
done
We can make the thread variable global because you are sure that there must not be more than one loop running at once. This, in particular, ensures that there will be no concurrency bug unlike what the precedently posted codes did.
