Resources
Place file:
DoorVisualiser.rbxl (14.7 KB)
Relevant object hierarchy:
StarterPlayer
> StarterPlayerScripts
> > DoorVisualiser
> > > CellDoor (example)
DoorVisualiser Code:
--[[
Visualises the state of doors on the client. Children are intended to facilitate the process.
colbert2677 - January 2019
--]]
local CollectionService = game:GetService("CollectionService")
local DoorTypes = script:GetChildren()
for _, DoorType in pairs(DoorTypes) do
local DoorScript = require(DoorType)
local DoorCollection = CollectionService:GetTagged(DoorType.Name)
for _, Door in pairs(DoorCollection) do
DoorScript.Attach(Door)
end
end
CellDoor Code:
--[[
CellDoor door type.
colbert2677 - January 2019
--]]
local TweenService = game:GetService("TweenService")
local Visualiser = {}
Visualiser._TweenData = TweenInfo.new(0.75, Enum.EasingStyle.Back, Enum.EasingDirection.Out)
Visualiser._RotateAngle = 100
function Visualiser.Attach(Model)
local Operator = Model.Operator
local Open = Operator.Open
local PrimaryPart = Model.Door.PrimaryPart
local PrimaryPartCFrame = PrimaryPart.CFrame
local self = {}
self._CurrentTween = nil
if Open.Value == true then
PrimaryPart.CFrame = PrimaryPartCFrame * CFrame.Angles(math.rad(Visualiser._RotateAngle), 0, 0)
end
Open.Changed:Connect(function (NewValue)
if self._CurrentTween and typeof(self._CurrentTween) == "Instance" then
self._CurrentTween:Destroy()
self._CurrentTween = nil
end
if NewValue == true then
self._CurrentTween = TweenService:Create(PrimaryPart, Visualiser._TweenData, {CFrame = PrimaryPartCFrame * CFrame.Angles(math.rad(Visualiser._RotateAngle), 0, 0)})
self._CurrentTween:Play()
else
self._CurrentTween = TweenService:Create(PrimaryPart, Visualiser._TweenData, {CFrame = PrimaryPartCFrame})
self._CurrentTween:Play()
end
end)
end
return Visualiser
Feedback Description
Description:
The code I have posted above is, as the name states, code intended to establish the visuals for doors (specifically the tweening towards its open state). Previously, I handled tweening for the door model on the server but I figure that having the client do it would be far more effective and aesthetically pleasing. The server handles authentication (as it is supposed to be a card-locked door), while the client opens it as necessary. There is currently no server-side verification: I have not accounted for exploiters for a great number of reasons irrelevant for the purposes of this thread.
Issue & Prior Attempted Solution(s):
The current issue I take up with this code is how disgusting it looks, so I’m turning to Code Review to see if there are better ways to show my code or whether it’s fine as it is. Specifically the part that’s going on in the Changed statements has me unnerved.
Open.Changed:Connect(function (NewValue)
if self._CurrentTween and typeof(self._CurrentTween) == "Instance" then
self._CurrentTween:Destroy()
self._CurrentTween = nil
end
if NewValue == true then
self._CurrentTween = TweenService:Create(PrimaryPart, Visualiser._TweenData, {CFrame = PrimaryPartCFrame * CFrame.Angles(math.rad(Visualiser._RotateAngle), 0, 0)})
self._CurrentTween:Play()
else
self._CurrentTween = TweenService:Create(PrimaryPart, Visualiser._TweenData, {CFrame = PrimaryPartCFrame})
self._CurrentTween:Play()
end
end)
Creating tweens is not an expensive operation so it’s not the optimisation part that I’m fidgety over, though I think that this isn’t proper. I’m creating new tweens every time I need to change the aesthetic of the door. Before this code manifested, I created the tweens upon attaching the code to a door (method Door.Attach) and placed them into the local environment. When the door was signalled to be open, it cancelled the closing animation if playing and played the open animation - this process, vice versa, for when the door was signalled to close. The difference for this code is destroying previously playing tweens and creating and playing a tween according to the state.
Is this code fine, or is there something better I can do?
Lots of notes to consider before providing feedback:
- I’m not changing my naming convention. Don’t suggest things like “constants should be CAPS_AND_UNDERSCORES” or any of that. I’m not interested.
- I’m not changing from ValueObjects, do not suggest I change to remotes. The only difference that’ll make is that I’ll experience communication delays.
- Remain on-topic. My query pertains to whether the code I have is fine as is, or if there is a way I can modify it so that I’m not creating tweens every time the ValueObject’s value changes.
- Though my major worry lies with the Changed section of code, I am also accepting improvements on the LocalScript
DoorVisualiser. I currently grab all children, regardless of what they are and use them to act. This goes under the assumption that the children of the LocalScript will only ever be ModuleScripts. - Tags are only used to group objects. You don’t need to know of any other code associated with tags, because I add them via Studio’s command bar. The main code for doors on the server has the list of tags hard coded, while the client bases them off of child ModuleScripts to DoorVisualiser.
- Doors are welded together using WeldConstraints. Everything except the PrimaryPart is unanchored. I’d also appreciate a brief addendum on if this is fine, as some people told me not to do it due to potential physics throttling acting on these parts. I refuse to use SetPrimaryPartCFrame due to offset issues and I have not completed my custom implementation of SetPrimaryPartCFrame yet.
