From 6ae0bf366a901789b3dfb093dd3a4c72928092d7 Mon Sep 17 00:00:00 2001 From: boatbomber Date: Mon, 18 Jul 2022 16:36:38 -0700 Subject: [PATCH] Use singleton settings outside the Roact tree (#576) * Use singleton settings outside the Roact tree * Cleanup listener on unmount * Refactor setting page components * Fix willUnmount being added to the wrong table * Remove bindings in favor of state --- plugin/src/App/PluginSettings.lua | 123 --------- plugin/src/App/StatusPages/Settings.lua | 246 ------------------ .../src/App/StatusPages/Settings/Setting.lua | 149 +++++++++++ plugin/src/App/StatusPages/Settings/init.lua | 121 +++++++++ plugin/src/App/init.lua | 22 +- plugin/src/Settings.lua | 78 ++++++ 6 files changed, 356 insertions(+), 383 deletions(-) delete mode 100644 plugin/src/App/PluginSettings.lua delete mode 100644 plugin/src/App/StatusPages/Settings.lua create mode 100644 plugin/src/App/StatusPages/Settings/Setting.lua create mode 100644 plugin/src/App/StatusPages/Settings/init.lua create mode 100644 plugin/src/Settings.lua diff --git a/plugin/src/App/PluginSettings.lua b/plugin/src/App/PluginSettings.lua deleted file mode 100644 index 366eb87a..00000000 --- a/plugin/src/App/PluginSettings.lua +++ /dev/null @@ -1,123 +0,0 @@ ---[[ - Persistent plugin settings that can be accessed via Roact context. -]] - -local Rojo = script:FindFirstAncestor("Rojo") - -local Roact = require(Rojo.Roact) - -local defaultSettings = { - openScriptsExternally = false, - twoWaySync = false, - showNotifications = true, - playSounds = true, -} - -local Settings = {} -Settings.__index = Settings - -function Settings.fromPlugin(plugin) - local values = {} - - for name, defaultValue in pairs(defaultSettings) do - local savedValue = plugin:GetSetting("Rojo_" .. name) - - if savedValue == nil then - plugin:SetSetting("Rojo_" .. name, defaultValue) - values[name] = defaultValue - else - values[name] = savedValue - end - end - - return setmetatable({ - __values = values, - __plugin = plugin, - __updateListeners = {}, - }, Settings) -end - -function Settings:get(name) - if defaultSettings[name] == nil then - error("Invalid setings name " .. tostring(name), 2) - end - - return self.__values[name] -end - -function Settings:set(name, value) - self.__plugin:SetSetting("Rojo_" .. name, value) - self.__values[name] = value - - for callback in pairs(self.__updateListeners) do - callback(name, value) - end -end - -function Settings:onUpdate(newCallback) - local newListeners = {} - for callback in pairs(self.__updateListeners) do - newListeners[callback] = true - end - - newListeners[newCallback] = true - self.__updateListeners = newListeners - - return function() - local newListeners = {} - for callback in pairs(self.__updateListeners) do - if callback ~= newCallback then - newListeners[callback] = true - end - end - - self.__updateListeners = newListeners - end -end - -local Context = Roact.createContext(nil) - -local StudioProvider = Roact.Component:extend("StudioProvider") - -function StudioProvider:init() - self.settings = Settings.fromPlugin(self.props.plugin) -end - -function StudioProvider:render() - return Roact.createElement(Context.Provider, { - value = self.settings, - }, self.props[Roact.Children]) -end - -local InternalConsumer = Roact.Component:extend("InternalConsumer") - -function InternalConsumer:render() - return self.props.render(self.props.settings) -end - -function InternalConsumer:didMount() - self.disconnect = self.props.settings:onUpdate(function() - -- Trigger a dummy state update to update the settings consumer. - self:setState({}) - end) -end - -function InternalConsumer:willUnmount() - self.disconnect() -end - -local function with(callback) - return Roact.createElement(Context.Consumer, { - render = function(settings) - return Roact.createElement(InternalConsumer, { - settings = settings, - render = callback, - }) - end, - }) -end - -return { - StudioProvider = StudioProvider, - with = with, -} diff --git a/plugin/src/App/StatusPages/Settings.lua b/plugin/src/App/StatusPages/Settings.lua deleted file mode 100644 index 77074b67..00000000 --- a/plugin/src/App/StatusPages/Settings.lua +++ /dev/null @@ -1,246 +0,0 @@ -local TextService = game:GetService("TextService") - -local Rojo = script:FindFirstAncestor("Rojo") -local Plugin = Rojo.Plugin - -local Roact = require(Rojo.Roact) - -local Assets = require(Plugin.Assets) -local Theme = require(Plugin.App.Theme) -local PluginSettings = require(Plugin.App.PluginSettings) - -local Checkbox = require(Plugin.App.Components.Checkbox) -local IconButton = require(Plugin.App.Components.IconButton) -local ScrollingFrame = require(Plugin.App.Components.ScrollingFrame) - -local e = Roact.createElement - -local DIVIDER_FADE_SIZE = 0.1 - -local function getTextBounds(text, textSize, font, lineHeight, bounds) - local textBounds = TextService:GetTextSize(text, textSize, font, bounds) - - local lineCount = textBounds.Y / textSize - local lineHeightAbsolute = textSize * lineHeight - - return Vector2.new(textBounds.X, lineHeightAbsolute * lineCount - (lineHeightAbsolute - textSize)) -end - -local function Navbar(props) - return Theme.with(function(theme) - theme = theme.Settings.Navbar - - return e("Frame", { - Size = UDim2.new(1, 0, 0, 46), - LayoutOrder = props.layoutOrder, - BackgroundTransparency = 1, - }, { - Back = e(IconButton, { - icon = Assets.Images.Icons.Back, - iconSize = 24, - color = theme.BackButtonColor, - transparency = props.transparency, - - position = UDim2.new(0, 0, 0.5, 0), - anchorPoint = Vector2.new(0, 0.5), - - onClick = props.onBack, - }), - - Text = e("TextLabel", { - Text = "Settings", - Font = Enum.Font.Gotham, - TextSize = 18, - TextColor3 = theme.TextColor, - TextTransparency = props.transparency, - - Size = UDim2.new(1, 0, 1, 0), - - BackgroundTransparency = 1, - }) - }) - end) -end - -local Setting = Roact.Component:extend("Setting") - -function Setting:init() - self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0)) - self.containerSize, self.setContainerSize = Roact.createBinding(Vector2.new(0, 0)) -end - -function Setting:render() - return Theme.with(function(theme) - theme = theme.Settings - - return PluginSettings.with(function(settings) - return e("Frame", { - Size = self.contentSize:map(function(value) - return UDim2.new(1, 0, 0, 20 + value.Y + 20) - end), - LayoutOrder = self.props.layoutOrder, - BackgroundTransparency = 1, - - [Roact.Change.AbsoluteSize] = function(object) - self.setContainerSize(object.AbsoluteSize) - end, - }, { - Checkbox = e(Checkbox, { - active = settings:get(self.props.id), - transparency = self.props.transparency, - position = UDim2.new(1, 0, 0.5, 0), - anchorPoint = Vector2.new(1, 0.5), - onClick = function() - local currentValue = settings:get(self.props.id) - settings:set(self.props.id, not currentValue) - end, - }), - - Text = e("Frame", { - Size = UDim2.new(1, 0, 1, 0), - BackgroundTransparency = 1, - }, { - Name = e("TextLabel", { - Text = self.props.name, - Font = Enum.Font.GothamBold, - TextSize = 17, - TextColor3 = theme.Setting.NameColor, - TextXAlignment = Enum.TextXAlignment.Left, - TextTransparency = self.props.transparency, - - Size = UDim2.new(1, 0, 0, 17), - - LayoutOrder = 1, - BackgroundTransparency = 1, - }), - - Description = e("TextLabel", { - Text = self.props.description, - Font = Enum.Font.Gotham, - LineHeight = 1.2, - TextSize = 14, - TextColor3 = theme.Setting.DescriptionColor, - TextXAlignment = Enum.TextXAlignment.Left, - TextTransparency = self.props.transparency, - TextWrapped = true, - - Size = self.containerSize:map(function(value) - local textBounds = getTextBounds( - self.props.description, 14, Enum.Font.Gotham, 1.2, - Vector2.new(value.X - 50, math.huge) - ) - return UDim2.new(1, -50, 0, textBounds.Y) - end), - - LayoutOrder = 2, - BackgroundTransparency = 1, - }), - - Layout = e("UIListLayout", { - VerticalAlignment = Enum.VerticalAlignment.Center, - FillDirection = Enum.FillDirection.Vertical, - SortOrder = Enum.SortOrder.LayoutOrder, - Padding = UDim.new(0, 6), - - [Roact.Change.AbsoluteContentSize] = function(object) - self.setContentSize(object.AbsoluteContentSize) - end, - }), - - Padding = e("UIPadding", { - PaddingTop = UDim.new(0, 20), - PaddingBottom = UDim.new(0, 20), - }), - }), - - Divider = e("Frame", { - BackgroundColor3 = theme.DividerColor, - BackgroundTransparency = self.props.transparency, - Size = UDim2.new(1, 0, 0, 1), - BorderSizePixel = 0, - }, { - Gradient = e("UIGradient", { - Transparency = NumberSequence.new({ - NumberSequenceKeypoint.new(0, 1), - NumberSequenceKeypoint.new(DIVIDER_FADE_SIZE, 0), - NumberSequenceKeypoint.new(1 - DIVIDER_FADE_SIZE, 0), - NumberSequenceKeypoint.new(1, 1), - }), - }), - }), - }) - end) - end) -end - -local SettingsPage = Roact.Component:extend("SettingsPage") - -function SettingsPage:init() - self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0)) -end - -function SettingsPage:render() - return Theme.with(function(theme) - theme = theme.Settings - - return e(ScrollingFrame, { - size = UDim2.new(1, 0, 1, 0), - contentSize = self.contentSize, - transparency = self.props.transparency, - }, { - Navbar = e(Navbar, { - onBack = self.props.onBack, - transparency = self.props.transparency, - layoutOrder = 0, - }), - - OpenScriptsExternally = e(Setting, { - id = "openScriptsExternally", - name = "Open Scripts Externally", - description = "Attempt to open scripts in an external editor", - transparency = self.props.transparency, - layoutOrder = 1, - }), - - ShowNotifications = e(Setting, { - id = "showNotifications", - name = "Show Notifications", - description = "Popup notifications in viewport", - transparency = self.props.transparency, - layoutOrder = 2, - }), - - PlaySounds = e(Setting, { - id = "playSounds", - name = "Play Sounds", - description = "Toggle sound effects", - transparency = self.props.transparency, - layoutOrder = 3, - }), - - TwoWaySync = e(Setting, { - id = "twoWaySync", - name = "Two-Way Sync", - description = "EXPERIMENTAL! Editing files in Studio will sync them into the filesystem", - transparency = self.props.transparency, - layoutOrder = 4, - }), - - Layout = e("UIListLayout", { - FillDirection = Enum.FillDirection.Vertical, - SortOrder = Enum.SortOrder.LayoutOrder, - - [Roact.Change.AbsoluteContentSize] = function(object) - self.setContentSize(object.AbsoluteContentSize) - end, - }), - - Padding = e("UIPadding", { - PaddingLeft = UDim.new(0, 20), - PaddingRight = UDim.new(0, 20), - }), - }) - end) -end - -return SettingsPage diff --git a/plugin/src/App/StatusPages/Settings/Setting.lua b/plugin/src/App/StatusPages/Settings/Setting.lua new file mode 100644 index 00000000..89fc8ad5 --- /dev/null +++ b/plugin/src/App/StatusPages/Settings/Setting.lua @@ -0,0 +1,149 @@ +local TextService = game:GetService("TextService") + +local Rojo = script:FindFirstAncestor("Rojo") +local Plugin = Rojo.Plugin + +local Roact = require(Rojo.Roact) + +local Settings = require(Plugin.Settings) +local Theme = require(Plugin.App.Theme) + +local Checkbox = require(Plugin.App.Components.Checkbox) + +local e = Roact.createElement + +local DIVIDER_FADE_SIZE = 0.1 + +local function getTextBounds(text, textSize, font, lineHeight, bounds) + local textBounds = TextService:GetTextSize(text, textSize, font, bounds) + + local lineCount = textBounds.Y / textSize + local lineHeightAbsolute = textSize * lineHeight + + return Vector2.new(textBounds.X, lineHeightAbsolute * lineCount - (lineHeightAbsolute - textSize)) +end + +local Setting = Roact.Component:extend("Setting") + +function Setting:init() + self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0)) + self.containerSize, self.setContainerSize = Roact.createBinding(Vector2.new(0, 0)) + + self:setState({ + setting = Settings:get(self.props.id), + }) + + self.changedCleanup = Settings:onChanged(self.props.id, function(value) + self:setState({ + setting = value, + }) + end) +end + +function Setting:willUnmount() + self.changedCleanup() +end + +function Setting:render() + return Theme.with(function(theme) + theme = theme.Settings + + return e("Frame", { + Size = self.contentSize:map(function(value) + return UDim2.new(1, 0, 0, 20 + value.Y + 20) + end), + LayoutOrder = self.props.layoutOrder, + BackgroundTransparency = 1, + + [Roact.Change.AbsoluteSize] = function(object) + self.setContainerSize(object.AbsoluteSize) + end, + }, { + Checkbox = e(Checkbox, { + active = self.state.setting, + transparency = self.props.transparency, + position = UDim2.new(1, 0, 0.5, 0), + anchorPoint = Vector2.new(1, 0.5), + onClick = function() + local currentValue = Settings:get(self.props.id) + Settings:set(self.props.id, not currentValue) + end, + }), + + Text = e("Frame", { + Size = UDim2.new(1, 0, 1, 0), + BackgroundTransparency = 1, + }, { + Name = e("TextLabel", { + Text = self.props.name, + Font = Enum.Font.GothamBold, + TextSize = 17, + TextColor3 = theme.Setting.NameColor, + TextXAlignment = Enum.TextXAlignment.Left, + TextTransparency = self.props.transparency, + + Size = UDim2.new(1, 0, 0, 17), + + LayoutOrder = 1, + BackgroundTransparency = 1, + }), + + Description = e("TextLabel", { + Text = self.props.description, + Font = Enum.Font.Gotham, + LineHeight = 1.2, + TextSize = 14, + TextColor3 = theme.Setting.DescriptionColor, + TextXAlignment = Enum.TextXAlignment.Left, + TextTransparency = self.props.transparency, + TextWrapped = true, + + Size = self.containerSize:map(function(value) + local textBounds = getTextBounds( + self.props.description, 14, Enum.Font.Gotham, 1.2, + Vector2.new(value.X - 50, math.huge) + ) + return UDim2.new(1, -50, 0, textBounds.Y) + end), + + LayoutOrder = 2, + BackgroundTransparency = 1, + }), + + Layout = e("UIListLayout", { + VerticalAlignment = Enum.VerticalAlignment.Center, + FillDirection = Enum.FillDirection.Vertical, + SortOrder = Enum.SortOrder.LayoutOrder, + Padding = UDim.new(0, 6), + + [Roact.Change.AbsoluteContentSize] = function(object) + self.setContentSize(object.AbsoluteContentSize) + end, + }), + + Padding = e("UIPadding", { + PaddingTop = UDim.new(0, 20), + PaddingBottom = UDim.new(0, 20), + }), + }), + + Divider = e("Frame", { + BackgroundColor3 = theme.DividerColor, + BackgroundTransparency = self.props.transparency, + Size = UDim2.new(1, 0, 0, 1), + BorderSizePixel = 0, + }, { + Gradient = e("UIGradient", { + Transparency = NumberSequence.new({ + NumberSequenceKeypoint.new(0, 1), + NumberSequenceKeypoint.new(DIVIDER_FADE_SIZE, 0), + NumberSequenceKeypoint.new(1 - DIVIDER_FADE_SIZE, 0), + NumberSequenceKeypoint.new(1, 1), + }), + }), + }), + }) + end) +end + +return Setting diff --git a/plugin/src/App/StatusPages/Settings/init.lua b/plugin/src/App/StatusPages/Settings/init.lua new file mode 100644 index 00000000..4ffe226e --- /dev/null +++ b/plugin/src/App/StatusPages/Settings/init.lua @@ -0,0 +1,121 @@ +local Rojo = script:FindFirstAncestor("Rojo") +local Plugin = Rojo.Plugin + +local Roact = require(Rojo.Roact) + +local Assets = require(Plugin.Assets) +local Theme = require(Plugin.App.Theme) + +local IconButton = require(Plugin.App.Components.IconButton) +local ScrollingFrame = require(Plugin.App.Components.ScrollingFrame) +local Setting = require(script.Setting) + +local e = Roact.createElement + +local function Navbar(props) + return Theme.with(function(theme) + theme = theme.Settings.Navbar + + return e("Frame", { + Size = UDim2.new(1, 0, 0, 46), + LayoutOrder = props.layoutOrder, + BackgroundTransparency = 1, + }, { + Back = e(IconButton, { + icon = Assets.Images.Icons.Back, + iconSize = 24, + color = theme.BackButtonColor, + transparency = props.transparency, + + position = UDim2.new(0, 0, 0.5, 0), + anchorPoint = Vector2.new(0, 0.5), + + onClick = props.onBack, + }), + + Text = e("TextLabel", { + Text = "Settings", + Font = Enum.Font.Gotham, + TextSize = 18, + TextColor3 = theme.TextColor, + TextTransparency = props.transparency, + + Size = UDim2.new(1, 0, 1, 0), + + BackgroundTransparency = 1, + }) + }) + end) +end + +local SettingsPage = Roact.Component:extend("SettingsPage") + +function SettingsPage:init() + self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0)) +end + +function SettingsPage:render() + return Theme.with(function(theme) + theme = theme.Settings + + return e(ScrollingFrame, { + size = UDim2.new(1, 0, 1, 0), + contentSize = self.contentSize, + transparency = self.props.transparency, + }, { + Navbar = e(Navbar, { + onBack = self.props.onBack, + transparency = self.props.transparency, + layoutOrder = 0, + }), + + OpenScriptsExternally = e(Setting, { + id = "openScriptsExternally", + name = "Open Scripts Externally", + description = "Attempt to open scripts in an external editor", + transparency = self.props.transparency, + layoutOrder = 1, + }), + + ShowNotifications = e(Setting, { + id = "showNotifications", + name = "Show Notifications", + description = "Popup notifications in viewport", + transparency = self.props.transparency, + layoutOrder = 2, + }), + + PlaySounds = e(Setting, { + id = "playSounds", + name = "Play Sounds", + description = "Toggle sound effects", + transparency = self.props.transparency, + layoutOrder = 3, + }), + + TwoWaySync = e(Setting, { + id = "twoWaySync", + name = "Two-Way Sync", + description = "EXPERIMENTAL! Editing files in Studio will sync them into the filesystem", + transparency = self.props.transparency, + layoutOrder = 4, + }), + + Layout = e("UIListLayout", { + FillDirection = Enum.FillDirection.Vertical, + SortOrder = Enum.SortOrder.LayoutOrder, + + [Roact.Change.AbsoluteContentSize] = function(object) + self.setContentSize(object.AbsoluteContentSize) + end, + }), + + Padding = e("UIPadding", { + PaddingLeft = UDim.new(0, 20), + PaddingRight = UDim.new(0, 20), + }), + }) + end) +end + +return SettingsPage diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 9b4b60fb..9693220b 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -7,6 +7,7 @@ local Log = require(Rojo.Log) local Assets = require(Plugin.Assets) local Version = require(Plugin.Version) local Config = require(Plugin.Config) +local Settings = require(Plugin.Settings) local strict = require(Plugin.strict) local Dictionary = require(Plugin.Dictionary) local ServeSession = require(Plugin.ServeSession) @@ -14,7 +15,6 @@ local ApiContext = require(Plugin.ApiContext) local preloadAssets = require(Plugin.preloadAssets) local soundPlayer = require(Plugin.soundPlayer) local Theme = require(script.Theme) -local PluginSettings = require(script.PluginSettings) local Page = require(script.Page) local Notifications = require(script.Notifications) @@ -52,7 +52,7 @@ function App:init() end function App:addNotification(text: string, timeout: number?) - if not self.props.settings:get("showNotifications") then + if not Settings:get("showNotifications") then return end @@ -91,8 +91,8 @@ function App:startSession() local host, port = self:getHostAndPort() local sessionOptions = { - openScriptsExternally = self.props.settings:get("openScriptsExternally"), - twoWaySync = self.props.settings:get("twoWaySync"), + openScriptsExternally = Settings:get("openScriptsExternally"), + twoWaySync = Settings:get("twoWaySync"), } local baseUrl = ("http://%s:%s"):format(host, port) @@ -345,15 +345,9 @@ function App:render() end return function(props) - return e(PluginSettings.StudioProvider, { - plugin = props.plugin, - }, { - App = PluginSettings.with(function(settings) - local mergedProps = Dictionary.merge(props, { - settings = settings, - soundPlayer = soundPlayer.new(settings), - }) - return e(App, mergedProps) - end), + local mergedProps = Dictionary.merge(props, { + soundPlayer = soundPlayer.new(Settings), }) + + return e(App, mergedProps) end diff --git a/plugin/src/Settings.lua b/plugin/src/Settings.lua new file mode 100644 index 00000000..6513d41c --- /dev/null +++ b/plugin/src/Settings.lua @@ -0,0 +1,78 @@ +--[[ + Persistent plugin settings. +]] + +local plugin = plugin or script:FindFirstAncestorWhichIsA("Plugin") +local Rojo = script:FindFirstAncestor("Rojo") + +local Log = require(Rojo.Log) + +local defaultSettings = { + openScriptsExternally = false, + twoWaySync = false, + showNotifications = true, + playSounds = true, +} + +local Settings = {} + +Settings._values = table.clone(defaultSettings) +Settings._updateListeners = {} + +if plugin then + for name, defaultValue in pairs(Settings._values) do + local savedValue = plugin:GetSetting("Rojo_" .. name) + + if savedValue == nil then + -- plugin:SetSetting hits disc instead of memory, so it can be slow. Spawn so we don't hang. + task.spawn(plugin.SetSetting, plugin, "Rojo_" .. name, defaultValue) + Settings._values[name] = defaultValue + else + Settings._values[name] = savedValue + end + end + Log.trace("Loaded settings from plugin store") +end + +function Settings:get(name) + if defaultSettings[name] == nil then + error("Invalid setings name " .. tostring(name), 2) + end + + return self._values[name] +end + +function Settings:set(name, value) + self._values[name] = value + + if plugin then + -- plugin:SetSetting hits disc instead of memory, so it can be slow. Spawn so we don't hang. + task.spawn(plugin.SetSetting, plugin, "Rojo_" .. name, value) + end + + if self._updateListeners[name] then + for callback in pairs(self._updateListeners[name]) do + task.spawn(callback, value) + end + end + + Log.trace(string.format("Set setting '%s' to '%s'", name, tostring(value))) +end + +function Settings:onChanged(name, callback) + local listeners = self._updateListeners[name] + if listeners == nil then + listeners = {} + self._updateListeners[name] = listeners + end + listeners[callback] = true + + Log.trace(string.format("Added listener for setting '%s' changes", name)) + + return function() + listeners[callback] = nil + Log.trace(string.format("Removed listener for setting '%s' changes", name)) + end +end + +return Settings