I’m wondering if this is the most performant and efficient way of highlighting the chest lids when the camera passes over them
local Services = {
Players = game:GetService("Players");
RunService = game:GetService("RunService");
CollectService = game:GetService("CollectionService");
Camera = workspace.CurrentCamera;
}
local Highlight = script:WaitForChild("Highlight")
local Chests = Services.CollectService:GetTagged("Chest")
local RayProps = RaycastParams.new()
RayProps.FilterType = Enum.RaycastFilterType.Whitelist
RayProps.FilterDescendantsInstances = {table.unpack(Chests)}
Services.RunService:BindToRenderStep("Selection", Enum.RenderPriority.Camera.Value, function()
local RayCast = workspace:Raycast(Services.Camera.CFrame.Position, Services.Camera.CFrame.LookVector*10, RayProps)
if RayCast then
if Highlight.Adornee ~= RayCast.Instance then
print("highlighting "..RayCast.Instance.Parent.Name) -- Incase you don't have RenderHighlightPass FFlag enabled
Highlight.Adornee = RayCast.Instance
end
else
if Highlight.Adornee ~= nil then
Highlight.Adornee = nil
end
end
end)
local Services = {
Players = game:GetService("Players");
RunService = game:GetService("RunService");
CollectService = game:GetService("CollectionService");
Camera = workspace.CurrentCamera;
}
This is definitely a bad practice.
Other than that, your code is fine. I’m letting you know that Mouse.Target does the exact same thing, except the Mouse module is deprecated.
Don’t have a services dictionary. I don’t get the need for that. It encourges unnecessary repetition of what should be already out in the open as a variable at the top of the script.
You don’t need to have Highlight.Adornee ~= nil, but that’s more of a style issue rather than something that’s an issue with your code.
Turn that else (new line) if into an elseif.
Use UserInputService (or ContextActionService) to raycast from the cursor/center of screen if you want to raycast on every mouse movement - raycasting when the player isn’t moving their mouse is unnecessary calculation.
The reason I do that is because it provides organization and automatic intellisense that (at least for me) is much easier to understand than opposed to just having plain variables
Connecting it to .InputChanged saved a lot of processing power, thanks!
It effectively turns what should would be a service into now you requiring to fetch the service, which you can now just say is you just doing :GetService() over and over again. You might as well:
local Services = game;
print(Services.ReplicatedStorage);
print(Services.CollectionService);
…and so on.
Food for thought though, doubt it’s really impactful whether you abandon it or not.
This is not actually the same functionality. Not all services exist when the game starts; the utility of GetService is that it will create services that haven’t yet been added. To complicate matters further, not all services are named accordingly with how they are retrieved; OP uses RunService, which is actually named “Run Service” (with a space).
I agree that using tables for this purpose is not optimal, but I wouldn’t claim it to be ‘bad practice’, considering that these lookups are commonplace in Lua based OOP, and by extension the existing scripting API in its entirety. Yes, these lookups can theoretically be expensive, but in order to introduce practical complexity, the size of the table would have to be massive; OP’s table only has a mere four members.
On basis of Roblox Lua Style guide and what is generally agreed on within the developer community, it definitely isn’t a good practice. It’s not adding “more organization”, and if your intellisense works better with this then it has a serious problem. Neither is it a matter of performance, adding to that the fact that tables are hashtables, so have constant time access. It’s a matter of convention.
The Roblox Lua Style guide is merely the guide that Roblox uses internally; it should not be touted as a set of definitive rules for all developer written code. Within the first few paragraphs, it even notes that there is no one correct way to format code, and that their rules comprise a “somewhat arbitrary standard”.
To further this sentiment, many developers (myself included) create and adhere to their own personal style guides; others must conform to unique circumstances, incentivized by the needs of the projects they work on–needs which commonly differ from Roblox’s own. Are these developers therefore bad scripters? No. Because their code does not break any principles of responsible code writing, and to say otherwise, purely on the basis of style, would be blatant nitpicking.
To be frank, I hoped that this was merely a case of poor wording, and that you were simply making a comment on optimization; the OP did after all mention that micro-optimizations were welcome. But to take the stance the habit must be considered bad practice, solely for its deviation from a set of superfluous structuring conventions, is wholly inexcusable.
For I come from the web development community and have worked within the industry, I know it is genuinely vital to follow some code styling ethic that all developers share. If everybody used different conventions, you wouldn’t be able to build on top of anyone else’s work, yet this is fundamental. While Roblox is a bit special as it has never been clear for anyone what conventions to follow, you will nonetheless find different “categories” of styling in the literature. In particular, the mainstream one is directly inherited from the web development industry. Of course, one will tend to develop its really own styling, and that’s normal, but it becomes a problem when stepping too far away from the common. A good piece of code is not only one that works, that is readable by you, but one that is readable by everyone, so that it can be peer-reviewed without getting a headache and be maintained by other developers in your team. Yes, consistency is cornerstone, but you have to get along with the idea that conventions already exist (not specifically talking about Roblox’s guide) and you have to follow them.
Nowhere does it mean you’re a bad programmer, it simply means you have to polish your code writing and this comes by receiving advice or by inspiring from existing literature.
I don’t wanna turn this thread into a styling argument so I’m gonna stop here. If you believe I’m wrong, you’ll figure out once your development skills are mature enough to understand how essential conventions are.
Readability is indeed one principle of responsible code writing. But the habit in question in no way damages readability; it’s perfectly suitable code by any metric other than stylistic preference. To call it bad practice would be an egregious overreaction, no matter which way you slice it.
Further discussion
If the lookup was substantial, I could definitely see that being a valid concern. However these services are in a table that’s completely 1 dimensional, with too few members to ever likely suffer from collisions. In this case, it could be substantiated that the habit might even aid work efficiency, as it might help OP narrow down results in autocomplete. I really don’t think there’s any sense in splitting hairs here.
I agree, and I never would have raised any objections, had the recommendation been introduced as one. However micro-optimizations are just that: negligible; their omission does not constitute ‘bad practice’. And this optimization in particular, would be quite micro. Keep in mind that globals are indexed from their function environment, and Workspace is a service that already has its own global–which is used extremely frequently, without complaint. Even the Lua API thinks it right to add a second dimension to all of this, by then giving libraries their own subtables.
I think this totally is bad practice. After all, things are considered bad practice when you’re hindering how efficient you can be whilst programming. For example, a form of bad practice is when you consistently path through the same thing over and over again. If you constantly see game.ReplicatedStorage all around your script, maybe it’s time to have ReplicatedStorage as a variable in the top of your script. Having a Services dictionary not only makes the amount you have to type more to get to said ReplicatedStorage reference, but now you’ve typed more just to get that dictionary defined. If you’re talking microoptimizations, now you have to allocate a wee bit more just to get that dictionary AND you have to index that dictionary every single time you want to access that service.
If we put it that way, yes, it’s bad practice. These things aren’t as “superfluous” as you make them out to be. A style guide not only encourages you to make readable code, but it encourages you to be more efficient. These things are said not by preference of the writer, but by reason.