I’m making an interface animation where the frames become visible. This function is imperative because there are a lot of instances with different settings for transparency. This function does not have any problems, but I’m always looking forward for better methods to do a similar one.
I cache every transparency setting of UI instances into a table to use afterwards.
local function TweenCanvas(UI)
local Cache = { }
for _, UI in ipairs(UI:GetDescendants()) do
if UI:IsA('Frame') and UI.BackgroundTransparency ~= 1 then
Cache[UI] = UI.BackgroundTransparency
UI.BackgroundTransparency = 1
TweenService:Create(UI, Info, {BackgroundTransparency = Cache[UI]}):Play()
elseif UI:IsA('TextLabel') or UI:IsA('TextButton') then
Cache[UI] = UI.TextTransparency
UI.TextTransparency = 1
TweenService:Create(UI, Info, {TextTransparency = Cache[UI]}):Play()
elseif UI:IsA('ImageLabel') or UI:IsA('ViewportFrame') then
Cache[UI] = { UI.BackgroundTransparency, UI.ImageTransparency }
UI.BackgroundTransparency, UI.ImageTransparency = 1, 1
TweenService:Create(UI, Info, {BackgroundTransparency = Cache[UI][1], ImageTransparency = Cache[UI][2]}):Play()
elseif UI:IsA('UIStroke') then
Cache[UI] = UI.Transparency
UI.Transparency = 1
TweenService:Create(UI, Info, {Transparency = Cache[UI]}):Play()
end
end
Cache = nil
end
You could create a table mapping all the relevant ui types to their respective transparency properties and then just create the tweens with data from that table. Also, what is the point of the cache? if your not referencing it from outside of the loop. Also, viewport frames don’t have an image transparency property I think it should be image label.
local TransparencyTweenData = {
{
UITypes = {"ImageButton", "ImageLabel"},
Props = {"BackgroundTransparency", "ImageTransparency"}
}
--Add the other UI types with their respective properties
}
local function TweenCanvas(UI)
for _, Descendant in ipairs(UI:GetDescendants()) do
for _, TweenData in ipairs(TransparencyTweenData) do
for _, UIType in ipairs(TweenData.UITypes) do
if (Descendant:IsA(UIType)) then
local PropsCache = {}
for _, Prop in ipairs(TweenData.Props) do
PropsCache[Prop] = Descendant[Prop]
Descendant[Prop] = 1
end
TweenService:Create(
Descendant,
Info,
PropsCache
):Play()
end
end
end
end
end
I wouldn’t necessarily call this more efficient (with the exception of the fact that the cache dictionary has no need to be outside of the loop) and by no means is there a right or wrong way to do this, but if I were to program this same thing I’d do it something like this:
local mapTypeToProperty = {
['Frame'] = {'Background'};
['TextLabel'] = {'Text'};
['TextButton'] = {'Text'};
['ImageLabel'] = {'Background', 'Image'};
['ViewportFrame'] = {'Background', 'Image'};
['UIStroke'] = {''};
};
local function tweenCanvas(ui)
for _,obj in pairs(ui:GetDescendants()) do
local properties = mapTypeToProperty[obj.ClassName];
if (properties) then
local propInfo = {};
for _,property in pairs(properties) do
local propName = property .. 'Transparency';
propInfo.propName = obj[propName];
obj[propName] = 1;
end;
tweenService:Create(obj, info, propInfo):Play();
end;
end;
end;
I haven’t tested this but you get the idea - a dictionary to map each object type to its associated transparency properties and then go through these and play a tween.
I hope this was helpful
EDIT: If the caching of properties is absolutely required for later use (I’m unsure as I don’t know about the rest of your code), you can just store the oldVal property inside a cache outside of the loop, but from the code provided I wasn’t sure whether or not it was required
EDIT 2: Updated code to make sure only a single tween is created for each object, rather than multiple for each different property (as @RemoDunks rightly pointed out).
Your solutions similar to mine (in the spirit of mapping properties to class types); however, I believe OP’s current solution is actually slightly more efficient than yours as the way you’ve set up the data array you have to have a 3rd loop within your function (thus increasing time complexity of the algorithm to a cubic). I do agree that its much more visually pleasing and has improved readability though judging from the title I believe that OP is looking for efficiency (though I may be wrong).
Good contribution though - the concept you had with the data array was the right sort of idea but I’m unsure if the code provided answers OP’s question
As I said in my initial reply, I agree that your code is much more readable and nicer to look at. I’m just unsure if OP is willing to sacrifice one for the other, especially when both can be attained simultaneously
I mentioned earlier that I agree the difference is negligible and yes, though I agree it’s over the top considering it’s such a small task, it’s what OP requested in the title
I think you’re overall misunderstanding how computers work.
Complexity is an asymptotic measure, it is meaningful only when the size of the parameters becomes significant enough compared to the computing power of a device. Additionally, a loop of fixed length is O(1), not O(n) like you’re saying.
The code using a mapping table is also in constant time, since table property access is constant.
Code is compiled, the compiler will do plenty of tweaks to your code to make it more efficient before it’s actually turned into machine code. Abstract your thoughts from what the code looks like under your eyes, and privilege readability over so-called optimisations.
Also just saying, @mayaAboveAll’s code is definitely cleaner.
I may have been misusing big O (I’m unsure if I was) but the only reason why I expressed it in terms of n is because OP may be creating UI components dynamically and because I’m unsure on the number of contents that’d be within @RemoDunks’ TransparencyTweenData array and their associated contents too. I’m not sure if that allows me to express it in terms of n, as the array given would be of fixed length but the others may change change depending on the type and and object insertion. I appreciate that it’s also fairly insignificant for such a small task that involves little complexity
Thank you for informing me of this, never realised the compiler actually tweaks code as was never taught that, was just told that it reads through and translates it into machine code Always looking to learn something new!
What is the basis for your claim that his code is cleaner? Some of the main flaws I see with their code:
How they chose to reference the properties by appending “transparency” to the property has poor readability and maintainability, since Roblox may add another transparency property that does not follow the same format.
For instances with the same properties, you would need to have unnecessary duplicate entries, like image buttons and image labels.
Directly referencing the classname is generally regarded as poor practice since you lose the ability to check for abstract parent classes.
I kind of agree with all that you said. I claimed this version is better essentially because it’s using a map whereas, based on their example, it doesn’t seem like they’d have plenty of duplicates, making it more readable, but otherwise they’d choose the other version. Regarding the use of ClassNames, they could use IsA instead.
I appreciate that it’s also fairly insignificant for such a small task that involves little complexity