From 16248a0072888c893927ff887a8d79a8be7ee331 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 22 Mar 2020 15:21:58 +0000 Subject: [PATCH 01/49] Add server check for plugin migration notification --- lua/shine/core/shared/base_plugin/config.lua | 26 +++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lua/shine/core/shared/base_plugin/config.lua b/lua/shine/core/shared/base_plugin/config.lua index f7b5ecd0b..eb7b37e13 100644 --- a/lua/shine/core/shared/base_plugin/config.lua +++ b/lua/shine/core/shared/base_plugin/config.lua @@ -259,18 +259,20 @@ function ConfigModule:MigrateConfig( Config ) self.__Name, CurrentConfigVersion, self.Version or "1.0" ) - Shine.SystemNotifications:AddNotification( { - Type = Shine.SystemNotifications.Type.INFO, - Message = { - Source = "Core", - TranslationKey = "INFO_PLUGIN_VERSION_UPDATE", - Context = tostring( OurVersion ) - }, - Source = { - Type = Shine.SystemNotifications.Source.PLUGIN, - ID = self.__Name - } - } ) + if Server then + Shine.SystemNotifications:AddNotification( { + Type = Shine.SystemNotifications.Type.INFO, + Message = { + Source = "Core", + TranslationKey = "INFO_PLUGIN_VERSION_UPDATE", + Context = tostring( OurVersion ) + }, + Source = { + Type = Shine.SystemNotifications.Source.PLUGIN, + ID = self.__Name + } + } ) + end Config.__Version = self.Version or "1.0" From 084c65efe91ff7d29c47c9e772888b188284d548 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 28 Mar 2020 10:56:05 +0000 Subject: [PATCH 02/49] Add system notification for plugin init errors --- locale/shine/core/enGB.json | 1 + lua/shine/core/shared/extensions.lua | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/locale/shine/core/enGB.json b/locale/shine/core/enGB.json index 0b5d6c541..be93ebb4d 100644 --- a/locale/shine/core/enGB.json +++ b/locale/shine/core/enGB.json @@ -82,6 +82,7 @@ "ERROR_INVALID_JSON_IN_BASE_CONFIG": "The base configuration file has invalid JSON: {Context}", "ERROR_INVALID_JSON_IN_PLUGIN_CONFIG": "Configuration file has invalid JSON: {Context}", + "ERROR_PLUGIN_INIT_ERROR": "An error occurred while attempting to enable the plugin:\n{Context}", "WARNING_USER_CONFIG_VALIDATION_ERRORS": "Provided user configuration had validation errors which have been corrected.", "WARNING_REMOTE_USER_CONFIG_VALIDATION_ERRORS": "Remote user configuration had validation errors which have been corrected. Check the local UserConfig.json file for the corrected data.", "WARNING_INVALID_JSON_IN_REMOTE_USER_CONFIG": "Remote user configuration has invalid JSON: {Context}. User data has been loaded from locally cached data instead.", diff --git a/lua/shine/core/shared/extensions.lua b/lua/shine/core/shared/extensions.lua index 0a6cd5408..d7ea90ce5 100644 --- a/lua/shine/core/shared/extensions.lua +++ b/lua/shine/core/shared/extensions.lua @@ -22,6 +22,7 @@ local StringLower = string.lower local TableQuickCopy = table.QuickCopy local TableSort = table.sort local ToDebugString = table.ToDebugString +local tostring = tostring local Traceback = debug.traceback local xpcall = xpcall @@ -434,6 +435,23 @@ do end end + local function NotifyInitError( Name, Err ) + if not Server then return end + + Shine.SystemNotifications:AddNotification( { + Type = Shine.SystemNotifications.Type.ERROR, + Message = { + Source = "Core", + TranslationKey = "ERROR_PLUGIN_INIT_ERROR", + Context = tostring( Err ) + }, + Source = { + Type = Shine.SystemNotifications.Source.PLUGIN, + ID = Name + } + } ) + end + -- Shared extensions need to be enabled once the server tells it to. function Shine:EnableExtension( Name, DontLoadConfig ) Shine.TypeCheck( Name, "string", 1, "EnableExtension" ) @@ -475,6 +493,7 @@ do if Plugin.HasConfig and not DontLoadConfig then local Success, Err = xpcall( Plugin.LoadConfig, OnInitError, Plugin ) if not Success then + NotifyInitError( Name, Err ) return false, StringFormat( "Error while loading config: %s", Err ) end end @@ -487,6 +506,8 @@ do MarkAsDisabled( Plugin, FirstEnable ) + NotifyInitError( Name, Loaded ) + return false, StringFormat( "Lua error: %s", Loaded ) end @@ -502,6 +523,7 @@ do Success, Err = xpcall( Plugin.BroadcastModuleEvent, OnInitError, Plugin, "Initialise" ) if not Success then + NotifyInitError( Name, Err ) return false, StringFormat( "Lua error: %s", Err ) end end From 353fe3925653121ea03d58a94132f8e670eeb7e9 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 31 Mar 2020 16:59:46 +0100 Subject: [PATCH 03/49] Improve error handling in improved chat plugin * Handle missing GUIChat element gracefully. * Handle local player classes that have no GetTeamNumber method when checking commander tag visibility. --- lua/shine/extensions/improvedchat/client.lua | 22 ++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/lua/shine/extensions/improvedchat/client.lua b/lua/shine/extensions/improvedchat/client.lua index a12642572..9b2209e5b 100644 --- a/lua/shine/extensions/improvedchat/client.lua +++ b/lua/shine/extensions/improvedchat/client.lua @@ -400,6 +400,8 @@ Plugin.ConfigGroup = { } function Plugin:Initialise() + self:BroadcastModuleEvent( "Initialise" ) + self.ChatTagDefinitions = {} self.ChatTags = {} self.MessagesInTransit = {} @@ -485,7 +487,7 @@ function Plugin:GetFontSize() end function Plugin:SetChatOffset( Offset ) - local Panel = self.GUIChat.Panel + local Panel = self.GUIChat and self.GUIChat.Panel if not SGUI.IsValid( Panel ) then return end Panel:SetPos( ComputeChatOffset( self.GUIChat:GetOffset(), Offset ) ) @@ -666,7 +668,7 @@ end -- Replace adding standard messages to use ChatLine elements and the altered display behaviour. function Plugin:OnChatAddMessage( GUIChat, PlayerColour, PlayerName, MessageColour, MessageText, IsCommander, IsRookie ) - if not GUIChat.AddChatLine then return end + if not GUIChat.AddChatLine or self.GUIChat ~= GUIChat then return end if IsCommander then TagData = { @@ -693,7 +695,7 @@ function Plugin:OnChatAddMessage( GUIChat, PlayerColour, PlayerName, MessageColo end local function IsVisibleToLocalPlayer( Player, TeamNumber ) - local PlayerTeam = Player:GetTeamNumber() + local PlayerTeam = Player.GetTeamNumber and Player:GetTeamNumber() return PlayerTeam == TeamNumber or PlayerTeam == kSpectatorIndex or PlayerTeam == kTeamReadyRoom end @@ -776,7 +778,7 @@ function Plugin:OnChatMessageReceived( Data ) Hook.Call( "OnChatMessageParsed", Data, Contents ) - self:AddRichTextMessage( { + return self:AddRichTextMessage( { Source = { Type = ChatAPI.SourceTypeName.PLAYER, ID = Data.SteamID, @@ -784,11 +786,15 @@ function Plugin:OnChatMessageReceived( Data ) }, Message = Contents } ) - - return true end function Plugin:AddRichTextMessage( MessageData ) + if not self.GUIChat then + -- This shouldn't happen, but fail gracefully if it does. + self.Logger:Warn( "GUIChat not available, unable to display chat message." ) + return + end + if self.GUIChat:AddRichTextMessage( MessageData.Message ) then local Player = Client.GetLocalPlayer() if Player and not MessageData.SuppressSound and Player.GetChatSound then @@ -797,6 +803,8 @@ function Plugin:AddRichTextMessage( MessageData ) Hook.Call( "OnRichTextChatMessageDisplayed", MessageData ) end + + return true end do @@ -946,6 +954,8 @@ function Plugin:Cleanup() return self.BaseClass.Cleanup( self ) end +Shine.LoadPluginModule( "logger.lua", Plugin ) + Plugin.ClientConfigSettings = { { ConfigKey = "AnimateMessages", From 9dbc59e086fac3404ba64455c21e57e7176c3eec Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 4 Apr 2020 14:24:06 +0100 Subject: [PATCH 04/49] Add package.loader for require on startup The existing require usage depends on a package loader added in TraceTracker.lua, which doesn't return error messages. This provides the same behaviour, but with the added benefit of a clear error message when loading fails. --- lua/shine/startup.lua | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lua/shine/startup.lua b/lua/shine/startup.lua index a93f2b0f6..6e0a4ad3f 100644 --- a/lua/shine/startup.lua +++ b/lua/shine/startup.lua @@ -8,6 +8,28 @@ local Trace = debug.traceback() if Trace:find( "Main.lua" ) or Trace:find( "Loading.lua" ) then return end +do + local loadfile = loadfile + local pcall = pcall + local StringFormat = string.format + local StringGSub = string.gsub + + -- Allow use of require to load mounted Lua files and indicate errors in loading them. + package.loaders[ #package.loaders + 1 ] = function( Name ) + local FilePath = StringFormat( "lua/%s.lua", ( StringGSub( Name, "%.", "/" ) ) ) + local Success, Func, Err = pcall( loadfile, FilePath ) + if Success then + if Func then + return Func + end + + return StringFormat( "\n\tfailed to load '%s' from mounted filesystem: %s", FilePath, Err ) + end + + return StringFormat( "\n\terror attempting to load file '%s' from mounted filesystem: %s", FilePath, Func ) + end +end + -- I have no idea why it's called this. Shine = {} From f337f8219d0015bd4a816606cc6adcc03716ac3f Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 4 Apr 2020 17:11:31 +0100 Subject: [PATCH 05/49] Fix trace aborts in various code paths In places that handle dispatching arbitrary numbers of arguments, use code generation to provide fixed argument size variations of dispatchers rather than taking a vararg. This avoids NYI aborts, allowing these code paths to be compiled. Additionally, a few other places that caused aborts have been fixed, such as colour checking (getmetatable on cdata aborts traces) and extension event insertion (replacing closures with callable tables). The overall result of this is an order of magnitude performance improvement in hook calling/broadcasting and SGUI child event propagation, as well as other minor improvements from LuaJIT being more free to compile code. --- lua/shine/core/shared/extensions.lua | 45 ++- lua/shine/core/shared/hook.lua | 354 ++++++++++++++------- lua/shine/lib/class.lua | 7 +- lua/shine/lib/codegen.lua | 140 ++++++++ lua/shine/lib/colour.lua | 22 +- lua/shine/lib/gui.lua | 79 +++-- lua/shine/lib/gui/base_control.lua | 46 ++- lua/shine/lib/objects/event_dispatcher.lua | 43 ++- test/core/shared/hook.lua | 1 + test/lib/codegen.lua | 103 ++++++ test/lib/colour.lua | 12 + 11 files changed, 657 insertions(+), 195 deletions(-) create mode 100644 lua/shine/lib/codegen.lua create mode 100644 test/lib/codegen.lua diff --git a/lua/shine/core/shared/extensions.lua b/lua/shine/core/shared/extensions.lua index d7ea90ce5..eb42edd67 100644 --- a/lua/shine/core/shared/extensions.lua +++ b/lua/shine/core/shared/extensions.lua @@ -625,6 +625,9 @@ local AllPluginsArray = {} Shine.AllPluginsArray = AllPluginsArray do + local CodeGen = require "shine/lib/codegen" + local select = select + Dispatcher = Shine.EventDispatcher( AllPluginsArray ) function Dispatcher:GetListener( PluginName ) @@ -635,27 +638,39 @@ do return Plugin and Plugin.Enabled and IsType( Plugin[ Event ], "function" ) end - function Dispatcher:CallEvent( Plugin, Method, OnError, Event, ... ) - local Success, a, b, c, d, e, f = xpcall( Method, OnError, Plugin, ... ) + local Callers = CodeGen.MakeFunctionGenerator( { + Template = [[local Shine, xpcall = ... + return function( Plugin, Method, OnError, Event{Arguments} ) + local Success, a, b, c, d, e, f = xpcall( Method, OnError, Plugin{Arguments} ) - if not Success then - Plugin.__HookErrors = ( Plugin.__HookErrors or 0 ) + 1 - Shine:DebugPrint( "[Hook Error] %s hook failed from plugin '%s'. Error count: %i.", - true, Event, Plugin.__Name, Plugin.__HookErrors ) + if not Success then + Plugin.__HookErrors = ( Plugin.__HookErrors or 0 ) + 1 + Shine:DebugPrint( "[Hook Error] %s hook failed from plugin '%s'. Error count: %i.", + true, Event, Plugin.__Name, Plugin.__HookErrors ) - if Plugin.__HookErrors >= 10 then - Shine:DebugPrint( "Unloading plugin '%s' for too many hook errors (%i).", - true, Plugin.__Name, Plugin.__HookErrors ) + if Plugin.__HookErrors >= 10 then + Shine:DebugPrint( "Unloading plugin '%s' for too many hook errors (%i).", + true, Plugin.__Name, Plugin.__HookErrors ) - Plugin.__HookErrors = 0 + Plugin.__HookErrors = 0 - Shine:UnloadExtension( Plugin.__Name ) - end + Shine:UnloadExtension( Plugin.__Name ) + end - return nil - end + return nil + end - return a, b, c, d, e, f + return a, b, c, d, e, f + end + ]], + ChunkName = "lua/shine/core/shared/extensions.lua/Dispatcher:CallEvent", + -- This should match the value used in the hook system. + InitialSize = 10, + Args = { Shine, xpcall } + } ) + + function Dispatcher:CallEvent( Plugin, Method, OnError, Event, ... ) + return Callers[ select( "#", ... ) ]( Plugin, Method, OnError, Event, ... ) end --[[ diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index df5b7b7c1..99e77d511 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -2,17 +2,24 @@ Shine internal hook system. ]] +local CodeGen = require "shine/lib/codegen" + local Shine = Shine local Clamp = math.Clamp +local DebugGetInfo = debug.getinfo +local DebugGetMetaTable = debug.getmetatable local DebugSetUpValue = debug.setupvalue local Floor = math.floor +local Huge = math.huge local IsCallable = Shine.IsCallable local IsType = Shine.IsType -local xpcall = xpcall +local Max = math.max local ReplaceMethod = Shine.ReplaceClassMethod +local select = select local StringExplode = string.Explode local StringFormat = string.format +local xpcall = xpcall local LinkedList = Shine.LinkedList @@ -28,37 +35,54 @@ local HookNodes = {} local HooksByEventAndIndex = {} local OnError = Shine.BuildErrorHandler( "Hook error" ) +local Hooks +do + -- Use callable tables to avoid creating closures during hook calls. + -- This helps to avoid LuaJIT blacklisting the hook call/broadcast functions. + local PluginCaller = { + __call = function( self, ... ) + return Shine:CallExtensionEvent( self[ 1 ], OnError, ... ) + end, + __tostring = function( self ) + return StringFormat( "CallExtensionEvent - %s", self[ 1 ] ) + end + } + local PluginBroadcaster = { + __call = function( self, ... ) + return Shine:BroadcastExtensionEvent( self[ 1 ], OnError, ... ) + end, + __tostring = function( self ) + return StringFormat( "BroadcastExtensionEvent - %s", self[ 1 ] ) + end + } + + Hooks = setmetatable( {}, { + -- On first call/addition of an event, setup a default hook to call the event on extensions. + __index = function( self, Event ) + local HooksByIndex = LinkedList() + local Node = HooksByIndex:Add( { + -- This emulates the old behaviour, placing the event between -20 and -19. + -- No client of the public API can set non-integer priorities. + Priority = -19.5, + Callback = setmetatable( { Event }, PluginCaller ), + BroadcastCallback = setmetatable( { Event }, PluginBroadcaster ), + Index = ExtensionIndex + } ) -local Hooks = setmetatable( {}, { - -- On first call/addition of an event, setup a default hook to call the event on extensions. - __index = function( self, Event ) - local HooksByIndex = LinkedList() - local Node = HooksByIndex:Add( { - -- This emulates the old behaviour, placing the event between -20 and -19. - -- No client of the public API can set non-integer priorities. - Priority = -19.5, - Callback = function( ... ) - return Shine:CallExtensionEvent( Event, OnError, ... ) - end, - BroadcastCallback = function( ... ) - return Shine:BroadcastExtensionEvent( Event, OnError, ... ) - end, - Index = ExtensionIndex - } ) - - HookNodes[ Event ] = { - [ ExtensionIndex ] = Node - } - HooksByEventAndIndex[ Event ] = { - [ ExtensionIndex ] = Node.Value.Callback - } + HookNodes[ Event ] = { + [ ExtensionIndex ] = Node + } + HooksByEventAndIndex[ Event ] = { + [ ExtensionIndex ] = Node.Value.Callback + } - -- Save the list on the table to avoid invoking this again. - self[ Event ] = HooksByIndex + -- Save the list on the table to avoid invoking this again. + self[ Event ] = HooksByIndex - return HooksByIndex - end -} ) + return HooksByIndex + end + } ) +end -- Sort nodes by priority. Equal priority nodes will be placed in insertion order -- as the linked list will insert before the first node that is strictly after the @@ -152,55 +176,94 @@ if not Shine.BroadcastExtensionEvent then Shine.BroadcastExtensionEvent = function() end end ---[[ - Calls an internal Shine hook. - Inputs: Event name, arguments to pass. -]] -local function Call( Event, ... ) - local Callbacks = Hooks[ Event ] - - for Node in Callbacks:IterateNodes() do - local Entry = Node.Value - local Success, a, b, c, d, e, f = xpcall( Entry.Callback, OnError, ... ) - if not Success then - -- If the error came from calling extension events, don't remove the hook - -- (though it should never happen). - if Entry.Index ~= ExtensionIndex then - Shine:DebugPrint( "[Hook Error] %s hook '%s' failed, removing.", - true, Event, Entry.Index ) - - Remove( Event, Entry.Index ) +local Call +do + -- See the comment in codegen.lua for the reasoning of this seemingly bizarre way of calling hooks. + -- In a nutshell, it's to avoid LuaJIT traces aborting with NYI when handling varargs. + local Callers = CodeGen.MakeFunctionGenerator( { + Template = [[local Shine, Hooks, OnError, Remove = ... + return function( Event{Arguments} ) + local Callbacks = Hooks[ Event ] + + for Node in Callbacks:IterateNodes() do + local Entry = Node.Value + local Success, a, b, c, d, e, f = xpcall( Entry.Callback, OnError{Arguments} ) + if not Success then + -- If the error came from calling extension events, don't remove the hook + -- (though it should never happen). + if Entry.Index ~= ExtensionIndex then + Shine:DebugPrint( "[Hook Error] %s hook '%s' failed, removing.", + true, Event, Entry.Index ) + + Remove( Event, Entry.Index ) + end + elseif a ~= nil then + return a, b, c, d, e, f + end end - elseif a ~= nil then - return a, b, c, d, e, f + end]], + ChunkName = "@lua/shine/core/shared/hook.lua/Call", + -- This should equal the largest number of arguments seen by a hook to avoid lazy-generation which can impact + -- compilation results. + InitialSize = 10, + Args = { Shine, Hooks, OnError, Remove }, + OnFunctionGenerated = function( NumArguments, Caller ) + Hook[ StringFormat( "CallWith%dArg%s", NumArguments, NumArguments == 1 and "" or "s" ) ] = Caller end + } ) + + --[[ + Calls an internal Shine hook. + Inputs: Event name, arguments to pass. + ]] + Call = function( Event, ... ) + -- LuaJIT happily compiles this despite it passing down a vararg, which lets us avoid having to go crazy with + -- special case CallWithXArgs everywhere. The performance difference between this and using the specific + -- variation is negligible. + return Callers[ select( "#", ... ) ]( Event, ... ) end end Hook.Call = Call ---[[ - Broadcasts an internal Shine hook, ignoring return values from callbacks. +local Broadcast +do + local Broadcasters = CodeGen.MakeFunctionGenerator( { + Template = [[local Shine, Hooks, OnError, Remove = ... + return function( Event{Arguments} ) + local Callbacks = Hooks[ Event ] + + for Node in Callbacks:IterateNodes() do + local Entry = Node.Value + local Success = xpcall( Entry.BroadcastCallback, OnError{Arguments} ) + if not Success then + -- If the error came from calling extension events, don't remove the hook + -- (though it should never happen). + if Entry.Index ~= ExtensionIndex then + Shine:DebugPrint( "[Hook Error] %s hook '%s' failed, removing.", + true, Event, Entry.Index ) + + Remove( Event, Entry.Index ) + end + end + end + end]], + ChunkName = "@lua/shine/core/shared/hook.lua/Broadcast", + InitialSize = 10, + Args = { Shine, Hooks, OnError, Remove }, + OnFunctionGenerated = function( NumArguments, Caller ) + Hook[ StringFormat( "BroadcastWith%dArg%s", NumArguments, NumArguments == 1 and "" or "s" ) ] = Caller + end + } ) - This always calls every added callback. + --[[ + Broadcasts an internal Shine hook, ignoring return values from callbacks. - Inputs: Event name, arguments to pass. -]] -local function Broadcast( Event, ... ) - local Callbacks = Hooks[ Event ] + This always calls every added callback. - for Node in Callbacks:IterateNodes() do - local Entry = Node.Value - local Success = xpcall( Entry.BroadcastCallback, OnError, ... ) - if not Success then - -- If the error came from calling extension events, don't remove the hook - -- (though it should never happen). - if Entry.Index ~= ExtensionIndex then - Shine:DebugPrint( "[Hook Error] %s hook '%s' failed, removing.", - true, Event, Entry.Index ) - - Remove( Event, Entry.Index ) - end - end + Inputs: Event name, arguments to pass. + ]] + Broadcast = function( Event, ... ) + return Broadcasters[ select( "#", ... ) ]( Event, ... ) end end Hook.Broadcast = Broadcast @@ -238,20 +301,61 @@ function Hook.GetTable() return HooksByEventAndIndex end +local function GetNumArguments( Func ) + local Offset = 0 + if IsType( Func, "table" ) then + -- Handle callable tables here too. + local Meta = DebugGetMetaTable( Func ) + if Meta and IsType( Meta.__call, "function" ) then + Func = Meta.__call + -- __call has a self argument which isn't seen by the caller. + Offset = 1 + else + return Huge + end + end + + Shine.AssertAtLevel( IsType( Func, "function" ), "Attempted to hook a non-function value!", 5 ) + + local Info = DebugGetInfo( Func ) + if not Info or Info.isvararg then + return Huge + end + + return Max( ( Info.nparams or Huge ) - Offset, 0 ) +end + --[[ Replaces the given method in the given class with ReplacementFunc. Inputs: Class name, method name, replacement function. Output: Original function. ]] -local function AddClassHook( ReplacementFunc, Class, Method ) - local OldFunc - - OldFunc = ReplaceMethod( Class, Method, function( ... ) - return ReplacementFunc( OldFunc, ... ) - end ) +local function AddClassHook( ReplacementFuncTemplate, HookName, Caller, Class, Method ) + local OldFunc = Shine.GetClassMethod( Class, Method ) + Shine.AssertAtLevel( OldFunc, "Unknown class/method: %s:%s()", 4, Class, Method ) + + local NumArguments = GetNumArguments( OldFunc ) + local ReplacementFunc + if not IsType( ReplacementFuncTemplate, "string" ) then + ReplacementFunc = CodeGen.GenerateFunctionWithArguments( + [[local OldFunc, ReplacementFunc = ... + return function( {FunctionArguments} ) + return ReplacementFunc( OldFunc{Arguments} ) + end]], + NumArguments, + "@lua/shine/core/shared/hook.lua/CustomClassHook", + OldFunc, + ReplacementFuncTemplate + ) + else + ReplacementFunc = CodeGen.GenerateFunctionWithArguments( + ReplacementFuncTemplate, NumArguments, "@lua/shine/core/shared/hook.lua/ClassHook", + HookName, Caller, OldFunc + ) + end - return OldFunc + return ReplaceMethod( Class, Method, ReplacementFunc ) end --[[ @@ -260,7 +364,7 @@ end Inputs: Global function name, replacement function. Output: Original function. ]] -local function AddGlobalHook( ReplacementFunc, FuncName ) +local function AddGlobalHook( ReplacementFunc, HookName, Caller, FuncName ) local Path = StringExplode( FuncName, ".", true ) local Func = _G @@ -277,8 +381,24 @@ local function AddGlobalHook( ReplacementFunc, FuncName ) i = i + 1 until not Path[ i ] - Prev[ Path[ i - 1 ] ] = function( ... ) - return ReplacementFunc( Func, ... ) + local NumArguments = GetNumArguments( Func ) + if not IsType( ReplacementFunc, "string" ) then + -- Maintain backwards compatibility with custom hook handlers while still helping to remove var-args. + Prev[ Path[ i - 1 ] ] = CodeGen.GenerateFunctionWithArguments( + [[local OldFunc, ReplacementFunc = ... + return function( {FunctionArguments} ) + return ReplacementFunc( OldFunc{Arguments} ) + end]], + NumArguments, + "@lua/shine/core/shared/hook.lua/CustomGlobalHook", + Func, + ReplacementFunc + ) + else + Prev[ Path[ i - 1 ] ] = CodeGen.GenerateFunctionWithArguments( + ReplacementFunc, NumArguments, "@lua/shine/core/shared/hook.lua/GlobalHook", + HookName, Caller, Func + ) end return Func @@ -307,61 +427,65 @@ end ]] local HookModes = { Replace = function( Adder, HookName, ... ) - return Adder( function( OldFunc, ... ) - return Call( HookName, ... ) - end, ... ) + return Adder( [[local HookName, Call = ... + return function( {FunctionArguments} ) + return Call( HookName{Arguments} ) + end]], HookName, Call, ... ) end, PassivePre = function( Adder, HookName, ... ) - return Adder( function( OldFunc, ... ) - Broadcast( HookName, ... ) + return Adder( [[local HookName, Broadcast, OldFunc = ... + return function( {FunctionArguments} ) + Broadcast( HookName{Arguments} ) - return OldFunc( ... ) - end, ... ) + return OldFunc( {FunctionArguments} ) + end]], HookName, Broadcast, ... ) end, PassivePost = function( Adder, HookName, ... ) - return Adder( function( OldFunc, ... ) - local a, b, c, d, e, f = OldFunc( ... ) + return Adder( [[local HookName, Broadcast, OldFunc = ... + return function( {FunctionArguments} ) + local a, b, c, d, e, f = OldFunc( {FunctionArguments} ) - Broadcast( HookName, ... ) + Broadcast( HookName{Arguments} ) - return a, b, c, d, e, f - end, ... ) + return a, b, c, d, e, f + end]], HookName, Broadcast, ... ) end, ActivePre = function( Adder, HookName, ... ) - return Adder( function( OldFunc, ... ) - local a, b, c, d, e, f = Call( HookName, ... ) + return Adder( [[local HookName, Call, OldFunc = ... + return function( {FunctionArguments} ) + local a, b, c, d, e, f = Call( HookName{Arguments} ) - if a ~= nil then - return a, b, c, d, e, f - end + if a ~= nil then + return a, b, c, d, e, f + end - return OldFunc( ... ) - end, ... ) + return OldFunc( {FunctionArguments} ) + end]], HookName, Call, ... ) end, ActivePost = function( Adder, HookName, ... ) - return Adder( function( OldFunc, ... ) - local a, b, c, d, e, f = OldFunc( ... ) + return Adder( [[local HookName, Call, OldFunc = ... + return function( {FunctionArguments} ) + local a, b, c, d, e, f = OldFunc( {FunctionArguments} ) + local g, h, i, j, k, l = Call( HookName{Arguments} ) - local g, h, i, j, k, l = Call( HookName, ... ) - - if g ~= nil then - return g, h, i, j, k, l - end + if g ~= nil then + return g, h, i, j, k, l + end - return a, b, c, d, e, f - end, ... ) + return a, b, c, d, e, f + end]], HookName, Call, ... ) end, Halt = function( Adder, HookName, ... ) - return Adder( function( OldFunc, ... ) - local Ret = Call( HookName, ... ) - - if Ret ~= nil then return end + return Adder( [[local HookName, Call, OldFunc = ... + return function( {FunctionArguments} ) + local Ret = Call( HookName{Arguments} ) + if Ret ~= nil then return end - return OldFunc( ... ) - end, ... ) + return OldFunc( {FunctionArguments} ) + end]], HookName, Call, ... ) end } @@ -424,7 +548,7 @@ local function SetupClassHook( Class, Method, HookName, Mode, Options ) local OldFunc if IsType( Mode, "function" ) then - OldFunc = AddClassHook( Mode, Class, Method ) + OldFunc = AddClassHook( Mode, HookName, nil, Class, Method ) else local HookFunc = HookModes[ Mode ] if not HookFunc then @@ -463,7 +587,7 @@ local function SetupGlobalHook( FuncName, HookName, Mode, Options ) local OldFunc if IsType( Mode, "function" ) then - OldFunc = AddGlobalHook( Mode, FuncName ) + OldFunc = AddGlobalHook( Mode, HookName, nil, FuncName ) else local HookFunc = HookModes[ Mode ] if not HookFunc then diff --git a/lua/shine/lib/class.lua b/lua/shine/lib/class.lua index 624a9f7c0..bd0966add 100644 --- a/lua/shine/lib/class.lua +++ b/lua/shine/lib/class.lua @@ -18,9 +18,12 @@ local function RecursivelyReplaceMethod( Class, Method, Func, Original ) end end -function Shine.ReplaceClassMethod( Class, Method, Func ) - local Original = _G[ Class ] and _G[ Class ][ Method ] +function Shine.GetClassMethod( Class, Method ) + return _G[ Class ] and _G[ Class ][ Method ] +end +function Shine.ReplaceClassMethod( Class, Method, Func ) + local Original = Shine.GetClassMethod( Class, Method ) if not Original then return nil, "class method does not exist." end RecursivelyReplaceMethod( Class, Method, Func, Original ) diff --git a/lua/shine/lib/codegen.lua b/lua/shine/lib/codegen.lua new file mode 100644 index 000000000..ef260c0c8 --- /dev/null +++ b/lua/shine/lib/codegen.lua @@ -0,0 +1,140 @@ +--[[ + Code generation helpers. +]] + +local Huge = math.huge +local load = load +local select = select +local StringFormat = string.format +local StringGSub = string.gsub +local TableConcat = table.concat +local type = type +local unpack = unpack + +local CodeGen = {} + +--[[ + Generates a function from a template with the given number of arguments. + + This is useful to use the same function template to handle varying number of arguments without needing a vararg + which can cause traces to abort. + + This is an expensive operation, and thus should be done upfront or lazily where possible. The intention is to + spend a bit more time upfront to give the compiler a much easier time later. + + Inputs: + 1. The function code to be used as a template. This should be a valid Lua string with placeholders for: + * FunctionArguments - the generated arguments for the function, without a ", " at the front. + * Arguments - the generated arguments, with a ", " at the front to pass in front of known static arguments. + It is expected that the given chunk returns a function. + 2. The number of arguments to generate the function with. Can be math.huge to indicate a vararg should be used. + 3. The source to give the loaded function (shown in error tracebacks). + ... Any values to pass to the chunk (e.g. to provide upvalues). + Output: + The generated function. +]] +local function GenerateFunctionWithArguments( FunctionCode, NumArguments, ChunkName, ... ) + Shine.TypeCheck( FunctionCode, "string", 1, "GenerateFunctionWithArguments" ) + Shine.TypeCheck( NumArguments, "number", 2, "GenerateFunctionWithArguments" ) + Shine.TypeCheck( ChunkName, { "string", "nil" }, 3, "GenerateFunctionWithArguments" ) + + local Arguments = { "" } + if NumArguments < Huge then + for i = 1, NumArguments do + Arguments[ i + 1 ] = StringFormat( "Arg%d", i ) + end + else + Arguments[ 2 ] = "..." + end + + local ArgumentsList = TableConcat( Arguments, ", " ) + local ArgumentsWithoutPrefix = TableConcat( Arguments, ", ", 2 ) + local GeneratedFunctionCode = StringGSub( FunctionCode, "{([^}]+)}", { + Arguments = ArgumentsList, + FunctionArguments = ArgumentsWithoutPrefix + } ) + + return load( GeneratedFunctionCode, ChunkName )( ... ) +end +CodeGen.GenerateFunctionWithArguments = GenerateFunctionWithArguments + +local NO_ARGS = {} + +--[[ + Provides a simple means of generating functions from a template that expect a specific number of arguments. + + By using more specialised functions, var-args can be translated into a defined number of arguments which avoids + an NYI on the VARG bytecode instruction. + + Input: Options table with the following keys: + { + -- Arguments to be passed to the function template (for use as upvalues in the returned function). + Args = { ... }, + + -- An optional name to give to each compiled function (used as the source name in error messages). + -- If omitted, the chunk's generated content is the source. + ChunkName = "...", + + -- The maximum number of argument variations to generate upfront without lazy loading. + -- This generates variations for [0, InitialSize] number of arguments. + -- Ideally this should cover all expected use cases. + InitialSize = 5, + + -- The number of arguments in the Args table. Can usually be omitted unless an argument in the middle is nil. + NumArgs = 1, + + -- The Lua code template to be used when generating functions. + -- This should return a function which will be the generated function. Arguments are available under "...". + -- There are 2 template variables: + -- * FunctionArguments - the generated arguments for the function, without a ", " at the front. + -- * Arguments - the generated arguments, with a ", " at the front to pass in front of known static arguments. + Template = "return function( {FunctionArguments} ) return pcall( Something{Arguments} ) end", + + -- An optional callback that is called whenever a new function is generated. + -- This will be called for all of the initial variations, and any later lazily generated versions. + -- The third argument will be true for lazily generated variations. + OnFunctionGenerated = function( NumArguments, Function, WasLazilyGenerated ) end + } + Output: + A table that provides a variation of the given template taking n arguments for each number key n. + If accessing an argument count that has not yet been generated, it is generated automatically. +]] +function CodeGen.MakeFunctionGenerator( Options ) + local Args = Shine.TypeCheckField( Options, "Args", { "table", "nil" }, "Options" ) or NO_ARGS + local ChunkName = Shine.TypeCheckField( Options, "ChunkName", { "string", "nil" }, "Options" ) + local NumArgs = Shine.TypeCheckField( Options, "NumArgs", { "number", "nil" }, "Options" ) or #Args + local Template = Shine.TypeCheckField( Options, "Template", "string", "Options" ) + + local OnFunctionGenerated = Options.OnFunctionGenerated + local HasCallback = Shine.IsCallable( OnFunctionGenerated ) + + local Functions = setmetatable( {}, { + __index = function( self, NumArguments ) + if type( NumArguments ) ~= "number" then + return nil + end + + local Function = GenerateFunctionWithArguments( + Template, NumArguments, ChunkName, unpack( Args, 1, NumArgs ) + ) + + self[ NumArguments ] = Function + + if HasCallback then + OnFunctionGenerated( NumArguments, Function, true ) + end + + return Function + end + } ) + for i = 0, Options.InitialSize do + Functions[ i ] = GenerateFunctionWithArguments( Template, i, ChunkName, unpack( Args, 1, NumArgs ) ) + if HasCallback then + OnFunctionGenerated( i, Functions[ i ], false ) + end + end + + return Functions +end + +return CodeGen diff --git a/lua/shine/lib/colour.lua b/lua/shine/lib/colour.lua index 1ffc9ce3f..d0f1be088 100644 --- a/lua/shine/lib/colour.lua +++ b/lua/shine/lib/colour.lua @@ -8,18 +8,32 @@ Colour = Color --I'm British, I can't stand writing Color. local Colour = Colour local Floor = math.floor -local getmetatable = getmetatable local Max = math.max local Min = math.min +local type = type + +local IsColour +do + local FFILoaded, FFI = pcall( require, "ffi" ) + if FFILoaded and FFI and FFI.istype then + local Success, PassedCheck = pcall( FFI.istype, "Color", Colour( 1, 1, 1 ) ) + if Success and PassedCheck then + local IsType = FFI.istype + IsColour = function( Colour ) return IsType( "Color", Colour ) end + end + end -local ColourMetatable = getmetatable( Colour( 0, 0, 0 ) ) + if not IsColour then + -- This is more risky as cdata can be anything. + IsColour = function( Colour ) return Colour:isa( "Color" ) end + end +end --[[ Determines if the passed in object is a colour. ]] function SGUI.IsColour( Object ) - -- Apparently vectors and colours share the same metatable... - return getmetatable( Object ) == ColourMetatable and Object.r + return type( Object ) == "cdata" and IsColour( Object ) end --[[ diff --git a/lua/shine/lib/gui.lua b/lua/shine/lib/gui.lua index 534a19000..c84cb9d84 100644 --- a/lua/shine/lib/gui.lua +++ b/lua/shine/lib/gui.lua @@ -485,45 +485,60 @@ function SGUI:AddPostEventAction( Action ) self.PostEventActions[ #self.PostEventActions + 1 ] = Action end ---[[ - Passes an event to all active SGUI windows. - - If an SGUI object is classed as a window, it MUST call all events on its children. - Then its children must call their events on their children and so on. - - Inputs: Event name, arguments. -]] -function SGUI:CallEvent( FocusChange, Name, ... ) - local Windows = SGUI.Windows - local WindowCount = #Windows - - --The focused window is the last in the list, so we call backwards. - for i = WindowCount, 1, - 1 do - local Window = Windows[ i ] +do + local CodeGen = require "shine/lib/codegen" + local select = select + + local Callers = CodeGen.MakeFunctionGenerator( { + Template = [[local OnError, xpcall = ... + return function( self, FocusChange, Name{Arguments} ) + local Windows = self.Windows + local WindowCount = #Windows + + -- The focused window is the last in the list, so we call backwards. + for i = WindowCount, 1, -1 do + local Window = Windows[ i ] + + if Window and Window[ Name ] and Window:GetIsVisible() then + local Success, Result, Control = xpcall( Window[ Name ], OnError, Window{Arguments} ) + + if Success then + if Result ~= nil then + if i ~= WindowCount and FocusChange and self.IsValid( Window ) then + self:SetWindowFocus( Window ) + end + + self:PostCallEvent( Result, Control ) + + return Result, Control + end + else + Window:Destroy() + end + end + end - if Window and Window[ Name ] and Window:GetIsVisible() then - local Success, Result, Control = xpcall( Window[ Name ], OnError, Window, ... ) + self:PostCallEvent() + end + ]], + ChunkName = "@lua/shine/lib/gui.lua/SGUI:CallEvent", + InitialSize = 2, + Args = { OnError, xpcall } + } ) - if Success then - if Result ~= nil then - if i ~= WindowCount and FocusChange and self.IsValid( Window ) then - self:SetWindowFocus( Window ) - end + --[[ + Passes an event to all active SGUI windows. - self:PostCallEvent( Result, Control ) + If an SGUI object is classed as a window, it MUST call all events on its children. + Then its children must call their events on their children and so on. - return Result, Control - end - else - Window:Destroy() - end - end + Inputs: Event name, arguments. + ]] + function SGUI:CallEvent( FocusChange, Name, ... ) + return Callers[ select( "#", ... ) ]( self, FocusChange, Name, ... ) end - - self:PostCallEvent() end - do SGUI.MouseObjects = 0 diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index 1f53e1813..34882ab3a 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -2,6 +2,8 @@ Base SGUI control. All controls inherit from this. ]] +local CodeGen = require "shine/lib/codegen" + local SGUI = Shine.GUI local ControlMeta = SGUI.BaseControl local Set = Shine.Set @@ -12,6 +14,7 @@ local IsType = Shine.IsType local IsGUIItemValid = debug.isvalid local Max = math.max local pairs = pairs +local select = select local StringFormat = string.format local TableNew = require "table.new" local TableRemoveByValue = table.RemoveByValue @@ -421,26 +424,37 @@ function ControlMeta:SetTopLevelWindow( Window ) end end ---[[ - Calls an SGUI event on every child of the object. +do + local Callers = CodeGen.MakeFunctionGenerator( { + Template = [[return function( self, Name{Arguments} ) + if not self.Children then return nil end + + -- Call the event on every child of this object in the order they were added. + for Child in self.Children:Iterate() do + if Child[ Name ] and not Child._CallEventsManually then + local Result, Control = Child[ Name ]( Child{Arguments} ) + + if Result ~= nil then + return Result, Control + end + end + end - Ignores children with the _CallEventsManually flag. -]] -function ControlMeta:CallOnChildren( Name, ... ) - if not self.Children then return nil end + return nil + end + ]], + ChunkName = "@lua/shine/lib/gui/base_control.lua/ControlMeta:CallOnChildren", + InitialSize = 2 + } ) - --Call the event on every child of this object in the order they were added. - for Child in self.Children:Iterate() do - if Child[ Name ] and not Child._CallEventsManually then - local Result, Control = Child[ Name ]( Child, ... ) + --[[ + Calls an SGUI event on every child of the object. - if Result ~= nil then - return Result, Control - end - end + Ignores children with the _CallEventsManually flag. + ]] + function ControlMeta:CallOnChildren( Name, ... ) + return Callers[ select( "#", ... ) ]( self, Name, ... ) end - - return nil end function ControlMeta:ForEach( TableKey, MethodName, ... ) diff --git a/lua/shine/lib/objects/event_dispatcher.lua b/lua/shine/lib/objects/event_dispatcher.lua index b768246db..324f071df 100644 --- a/lua/shine/lib/objects/event_dispatcher.lua +++ b/lua/shine/lib/objects/event_dispatcher.lua @@ -3,7 +3,10 @@ on a list of tables which may or may not have a callback for it. ]] +local CodeGen = require "shine/lib/codegen" + local IsType = Shine.IsType +local select = select local TableEmpty = table.Empty local EventDispatcher = Shine.TypeDef() @@ -70,6 +73,21 @@ function EventDispatcher:GetListenersForEvent( Event ) return Listeners end +local Dispatchers = CodeGen.MakeFunctionGenerator( { + Template = [[return function( self, Event{Arguments} ) + local Listeners = self:GetListenersForEvent( Event ) + for i = 1, Listeners.Count do + local a, b, c, d, e, f = self:CallEvent( Listeners[ i ], Listeners[ i ][ Event ]{Arguments} ) + if a ~= nil then + return a, b, c, d, e, f + end + end + end + ]], + ChunkName = "@lua/shine/lib/objects/event_dispatcher.lua/DispatchEvent", + InitialSize = 12 +} ) + --[[ Dispatches an event to all listeners that are listening for it. @@ -77,15 +95,21 @@ end Subsequent calls use the cached list until the cache is flushed. ]] function EventDispatcher:DispatchEvent( Event, ... ) - local Listeners = self:GetListenersForEvent( Event ) - for i = 1, Listeners.Count do - local a, b, c, d, e, f = self:CallEvent( Listeners[ i ], Listeners[ i ][ Event ], ... ) - if a ~= nil then - return a, b, c, d, e, f - end - end + return Dispatchers[ select( "#", ... ) ]( self, Event, ... ) end +local Broadcasters = CodeGen.MakeFunctionGenerator( { + Template = [[return function( self, Event{Arguments} ) + local Listeners = self:GetListenersForEvent( Event ) + for i = 1, Listeners.Count do + self:CallEvent( Listeners[ i ], Listeners[ i ][ Event ]{Arguments} ) + end + end + ]], + ChunkName = "@lua/shine/lib/objects/event_dispatcher.lua/BroadcastEvent", + InitialSize = 12 +} ) + --[[ Broadcasts an event to all listeners that are listening for it. @@ -93,10 +117,7 @@ end from continuing. All listeners are guaranteed to receive the event. ]] function EventDispatcher:BroadcastEvent( Event, ... ) - local Listeners = self:GetListenersForEvent( Event ) - for i = 1, Listeners.Count do - self:CallEvent( Listeners[ i ], Listeners[ i ][ Event ], ... ) - end + return Broadcasters[ select( "#", ... ) ]( self, Event, ... ) end --[[ diff --git a/test/core/shared/hook.lua b/test/core/shared/hook.lua index b305d0e9e..d6375e7b5 100644 --- a/test/core/shared/hook.lua +++ b/test/core/shared/hook.lua @@ -124,6 +124,7 @@ UnitTest:Test( "SetupClassHook replaces functions only once", function( Assert ) Assert.Equals( "Function returned from SetupClassHook should be the original method", TestFunction, OldFunc ) local NewHookedFunction = _G[ ClassName ].TestMethod + Assert.NotEquals( "Should have replaced the class method", NewHookedFunction, OldFunc ) -- Hooking the same function twice with the same hook name and mode should do nothing and return -- the original function from the first setup call. diff --git a/test/lib/codegen.lua b/test/lib/codegen.lua new file mode 100644 index 000000000..e2689a0f5 --- /dev/null +++ b/test/lib/codegen.lua @@ -0,0 +1,103 @@ +--[[ + Code generation tests. +]] + +local CodeGen = require "shine/lib/codegen" +local UnitTest = Shine.UnitTest + +local DebugGetInfo = debug.getinfo + +local Template = [[local NumArgs = select( "#", ... ) +assert( NumArgs == 2, "Received "..NumArgs.." argument(s)!" ) + +local InjectedValue1, InjectedValue2 = ... +return function( {FunctionArguments} ) + return math.max( InjectedValue1, InjectedValue2{Arguments} ) +end]] + +UnitTest:Test( "GenerateFunctionWithArguments - Generates a 0-arguments function as expected", function( Assert ) + local GeneratedFunction = CodeGen.GenerateFunctionWithArguments( Template, 0, nil, 1, 2 ) + local Info = DebugGetInfo( GeneratedFunction ) + Assert.False( "Generated function should not be a vararg", Info.isvararg ) + Assert.Equals( "Generated function should have 0 parameters", 0, Info.nparams ) + Assert.Equals( "Should return the max of the injected values", 2, GeneratedFunction( 3 ) ) +end ) + +UnitTest:Test( "GenerateFunctionWithArguments - Generates a finite-argument function as expected", function( Assert ) + local GeneratedFunction = CodeGen.GenerateFunctionWithArguments( Template, 2, nil, 1, 2 ) + local Info = DebugGetInfo( GeneratedFunction ) + Assert.False( "Generated function should not be a vararg", Info.isvararg ) + Assert.Equals( "Generated function should have 2 parameters", 2, Info.nparams ) + Assert.Equals( "Should return the max of the injected values and arguments", 4, GeneratedFunction( 3, 4 ) ) +end ) + +UnitTest:Test( "GenerateFunctionWithArguments - Generates a vararg if given math.huge", function( Assert ) + local GeneratedFunction = CodeGen.GenerateFunctionWithArguments( Template, math.huge, nil, 1, 2 ) + local Info = DebugGetInfo( GeneratedFunction ) + Assert.True( "Generated function should be a vararg function", Info.isvararg ) + Assert.Equals( "Should return the max of the injected values and arguments", 4, GeneratedFunction( 3, 4 ) ) +end ) + +UnitTest:Test( "MakeFunctionGenerator - Generates a table of functions with arguments", function( Assert ) + local OnFunctionGenerated = UnitTest.MockFunction() + local Functions = CodeGen.MakeFunctionGenerator( { + Template = Template, + ChunkName = "@test/lib/codegen.lua", + Args = { 1, 2 }, + InitialSize = 2, + OnFunctionGenerated = OnFunctionGenerated + } ) + for i = 0, 3 do + Assert.Equals( "Should return the max of the arguments the function can see", i + 2, Functions[ i ]( 3, 4, 5 ) ) + Assert.Same( "Should cache functions after the first read", Functions[ i ], Functions[ i ] ) + + local Info = DebugGetInfo( Functions[ i ] ) + Assert.False( "Should not be a vararg", Info.isvararg ) + Assert.Equals( "Should have the expected number of parameters", i, Info.nparams ) + end + + Assert.DeepEquals( "Should have invoked the OnFunctionGenerated callback as expected", { + { + 0, + Functions[ 0 ], + false, + ArgCount = 3 + }, + { + 1, + Functions[ 1 ], + false, + ArgCount = 3 + }, + { + 2, + Functions[ 2 ], + false, + ArgCount = 3 + }, + { + 3, + Functions[ 3 ], + true, + ArgCount = 3 + } + }, OnFunctionGenerated.Invocations ) +end ) + +UnitTest:Test( "MakeFunctionGenerator - Respects the NumArgs parameter", function( Assert ) + local OnFunctionGenerated = UnitTest.MockFunction() + local Functions = CodeGen.MakeFunctionGenerator( { + Template = Template, + ChunkName = "@test/lib/codegen.lua", + Args = { 1, 2, 3, 4 }, + InitialSize = 3, + NumArgs = 2 + } ) + for i = 0, 3 do + Assert.Equals( "Should return the max of the arguments the function can see", i + 2, Functions[ i ]( 3, 4, 5 ) ) + + local Info = DebugGetInfo( Functions[ i ] ) + Assert.False( "Should not be a vararg", Info.isvararg ) + Assert.Equals( "Should have the expected number of parameters", i, Info.nparams ) + end +end ) diff --git a/test/lib/colour.lua b/test/lib/colour.lua index 25be50ffe..764d84278 100644 --- a/test/lib/colour.lua +++ b/test/lib/colour.lua @@ -9,6 +9,18 @@ local SGUI = Shine.GUI Script.Load( "lua/shine/lib/colour.lua" ) +UnitTest:Test( "IsColour - Returns true for a colour object", function( Assert ) + Assert.True( "Should return true for a colour object", SGUI.IsColour( Colour( 1, 1, 1 ) ) ) +end ) + +UnitTest:Test( "IsColour - Returns false for a different cdata type", function( Assert ) + Assert.False( "Should return false for a vector object", SGUI.IsColour( Vector( 1, 1, 1 ) ) ) +end ) + +UnitTest:Test( "IsColour - Returns false for a non-cdata type", function( Assert ) + Assert.False( "Should return false for a non-cdata object", SGUI.IsColour( "not a colour" ) ) +end ) + local function AssertColoursEqual( Assert, A, B ) Assert.Equals( "Red values must match", A.r, B.r ) Assert.Equals( "Green values must match", A.g, B.g ) From bd1fd3ad903008001b2812f7ebd68fe0ce8a74d2 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 4 Apr 2020 17:16:33 +0100 Subject: [PATCH 06/49] Fix arguments for generated hook functions --- lua/shine/core/shared/hook.lua | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index 99e77d511..2118a6aba 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -181,7 +181,7 @@ do -- See the comment in codegen.lua for the reasoning of this seemingly bizarre way of calling hooks. -- In a nutshell, it's to avoid LuaJIT traces aborting with NYI when handling varargs. local Callers = CodeGen.MakeFunctionGenerator( { - Template = [[local Shine, Hooks, OnError, Remove = ... + Template = [[local Shine, Hooks, OnError, Remove, ExtensionIndex = ... return function( Event{Arguments} ) local Callbacks = Hooks[ Event ] @@ -206,7 +206,7 @@ do -- This should equal the largest number of arguments seen by a hook to avoid lazy-generation which can impact -- compilation results. InitialSize = 10, - Args = { Shine, Hooks, OnError, Remove }, + Args = { Shine, Hooks, OnError, Remove, ExtensionIndex }, OnFunctionGenerated = function( NumArguments, Caller ) Hook[ StringFormat( "CallWith%dArg%s", NumArguments, NumArguments == 1 and "" or "s" ) ] = Caller end @@ -228,7 +228,7 @@ Hook.Call = Call local Broadcast do local Broadcasters = CodeGen.MakeFunctionGenerator( { - Template = [[local Shine, Hooks, OnError, Remove = ... + Template = [[local Shine, Hooks, OnError, Remove, ExtensionIndex = ... return function( Event{Arguments} ) local Callbacks = Hooks[ Event ] @@ -249,7 +249,7 @@ do end]], ChunkName = "@lua/shine/core/shared/hook.lua/Broadcast", InitialSize = 10, - Args = { Shine, Hooks, OnError, Remove }, + Args = { Shine, Hooks, OnError, Remove, ExtensionIndex }, OnFunctionGenerated = function( NumArguments, Caller ) Hook[ StringFormat( "BroadcastWith%dArg%s", NumArguments, NumArguments == 1 and "" or "s" ) ] = Caller end From 85ba9f865c91af316268fcf6139bea3d6314d329 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 4 Apr 2020 17:48:16 +0100 Subject: [PATCH 07/49] Print warnings rather than throwing in hook setup This maintains backwards compatibility, even if it isn't ideal behaviour. --- lua/shine/core/shared/hook.lua | 33 ++++++++++++------ test/core/shared/hook.lua | 61 ++++++++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 14 deletions(-) diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index 2118a6aba..dc845c199 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -315,9 +315,7 @@ local function GetNumArguments( Func ) end end - Shine.AssertAtLevel( IsType( Func, "function" ), "Attempted to hook a non-function value!", 5 ) - - local Info = DebugGetInfo( Func ) + local Info = IsType( Func, "function" ) and DebugGetInfo( Func ) if not Info or Info.isvararg then return Huge end @@ -333,7 +331,11 @@ end ]] local function AddClassHook( ReplacementFuncTemplate, HookName, Caller, Class, Method ) local OldFunc = Shine.GetClassMethod( Class, Method ) - Shine.AssertAtLevel( OldFunc, "Unknown class/method: %s:%s()", 4, Class, Method ) + if not OldFunc then + -- For backwards compatibility's sake, just print a warning here and do nothing. + Print( "[Shine] [Warn] Attempted to hook class/method %s:%s() which does not exist!", Class, Method ) + return nil + end local NumArguments = GetNumArguments( OldFunc ) local ReplacementFunc @@ -368,23 +370,32 @@ local function AddGlobalHook( ReplacementFunc, HookName, Caller, FuncName ) local Path = StringExplode( FuncName, ".", true ) local Func = _G - local i = 1 + local NumSegments = #Path local Prev - repeat + for i = 1, NumSegments do Prev = Func Func = Func[ Path[ i ] ] -- Doesn't exist! - if not Func then return nil end + if not Func then + Print( "[Shine] [Warn] Attempted to hook global value %s which does not exist!", FuncName ) + return nil + end + end - i = i + 1 - until not Path[ i ] + if not Shine.IsCallable( Func ) then + -- Ideally this should throw an error, but there may be cases where old code hooks a non-callable global + -- value and throwing here would break it. + Print( + "[Shine] [Warn] Hooking global value %s which is not callable (type: %s)!", FuncName, type( Func ) + ) + end local NumArguments = GetNumArguments( Func ) if not IsType( ReplacementFunc, "string" ) then -- Maintain backwards compatibility with custom hook handlers while still helping to remove var-args. - Prev[ Path[ i - 1 ] ] = CodeGen.GenerateFunctionWithArguments( + Prev[ Path[ NumSegments ] ] = CodeGen.GenerateFunctionWithArguments( [[local OldFunc, ReplacementFunc = ... return function( {FunctionArguments} ) return ReplacementFunc( OldFunc{Arguments} ) @@ -395,7 +406,7 @@ local function AddGlobalHook( ReplacementFunc, HookName, Caller, FuncName ) ReplacementFunc ) else - Prev[ Path[ i - 1 ] ] = CodeGen.GenerateFunctionWithArguments( + Prev[ Path[ NumSegments ] ] = CodeGen.GenerateFunctionWithArguments( ReplacementFunc, NumArguments, "@lua/shine/core/shared/hook.lua/GlobalHook", HookName, Caller, Func ) diff --git a/test/core/shared/hook.lua b/test/core/shared/hook.lua index d6375e7b5..c2b6b278d 100644 --- a/test/core/shared/hook.lua +++ b/test/core/shared/hook.lua @@ -93,7 +93,7 @@ Hook.Clear( "Test" ) local FunctionName = "TestFunction"..os.time() -UnitTest:Test( "SetupGlobalHook replaces functions only once", function( Assert ) +UnitTest:Test( "SetupGlobalHook - Replaces functions only once", function( Assert ) local TestFunction = function() end _G[ FunctionName ] = TestFunction @@ -102,22 +102,64 @@ UnitTest:Test( "SetupGlobalHook replaces functions only once", function( Assert Assert.Equals( "Function returned from SetupGlobalHook should be the global value", TestFunction, OldFunc ) local NewHookedFunction = _G[ FunctionName ] + Assert.NotEquals( "Should have replaced the global value", NewHookedFunction, OldFunc ) + Assert.IsType( "Should have replaced the global value with a function", NewHookedFunction, "function" ) -- Hooking the same function twice with the same hook name and mode should do nothing and return -- the original function from the first setup call. OldFunc = Hook.SetupGlobalHook( FunctionName, "Test", "PassivePost" ) Assert.Equals( "Function returned from SetupGlobalHook should still be the global value", TestFunction, OldFunc ) Assert.Equals( "Global function should be unchanged", NewHookedFunction, _G[ FunctionName ] ) + + local Callback = UnitTest.MockFunction() + Hook.Add( "Test", Callback ) + + NewHookedFunction() + + Assert.DeepEquals( "Calling the replaced function should run the hook", { + { ArgCount = 0 } + }, Callback.Invocations ) end ) +Hook.Clear( "Test" ) + +local NestedName = "TestHolder"..os.time() + +UnitTest:Test( "SetupGlobalHook - Handles nested global values", function( Assert ) + local TestFunction = function( Arg1, Arg2, Arg3 ) end + _G[ NestedName ] = { + [ FunctionName ] = TestFunction + } + + local OldFunc = Hook.SetupGlobalHook( NestedName.."."..FunctionName, "Test", "PassivePost" ) + Assert.Equals( "Function returned from SetupGlobalHook should be the nested global value", TestFunction, OldFunc ) + + local NewHookedFunction = _G[ NestedName ][ FunctionName ] + Assert.NotEquals( "Should have replaced the nested global value", NewHookedFunction, OldFunc ) + Assert.IsType( "Should have replaced the nested global value with a function", NewHookedFunction, "function" ) + + local Callback = UnitTest.MockFunction() + Hook.Add( "Test", Callback ) + + -- Ignores the 4th argument as the original function had only 3. + NewHookedFunction( 1, 2, 3, 4 ) + + Assert.DeepEquals( "Calling the replaced function should run the hook", { + { ArgCount = 3, 1, 2, 3 } + }, Callback.Invocations ) +end ) + +Hook.Clear( "Test" ) + +_G[ NestedName ] = nil _G[ FunctionName ] = nil local ClassName = "HookTestClass"..os.time() -UnitTest:Test( "SetupClassHook replaces functions only once", function( Assert ) +UnitTest:Test( "SetupClassHook - Replaces functions only once", function( Assert ) class( ClassName ) - local TestFunction = function() end + local TestFunction = function( self ) end _G[ ClassName ].TestMethod = TestFunction local OldFunc = Hook.SetupClassHook( ClassName, "TestMethod", "TestClassTestMethod", "PassivePost" ) @@ -125,12 +167,25 @@ UnitTest:Test( "SetupClassHook replaces functions only once", function( Assert ) local NewHookedFunction = _G[ ClassName ].TestMethod Assert.NotEquals( "Should have replaced the class method", NewHookedFunction, OldFunc ) + Assert.IsType( "Should have replaced the class method with a function", NewHookedFunction, "function" ) -- Hooking the same function twice with the same hook name and mode should do nothing and return -- the original function from the first setup call. OldFunc = Hook.SetupClassHook( ClassName, "TestMethod", "TestClassTestMethod", "PassivePost" ) Assert.Equals( "Function returned from SetupClassHook should still be the original method", TestFunction, OldFunc ) Assert.Equals( "Class method should be unchanged", NewHookedFunction, _G[ ClassName ].TestMethod ) + + local Callback = UnitTest.MockFunction() + Hook.Add( "TestClassTestMethod", Callback ) + + local Arg = {} + NewHookedFunction( Arg ) + + Assert.DeepEquals( "Calling the replaced function should run the hook", { + { ArgCount = 1, Arg } + }, Callback.Invocations ) end ) +Hook.Clear( "TestClassTestMethod" ) + _G[ ClassName ] = nil From 4f074aa8c1b2edce930ac6fb88ad61e8d4fd8d4d Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 4 Apr 2020 18:43:39 +0100 Subject: [PATCH 08/49] Fix handling of extra hook arguments Use the maximum of a custom handler's argument count, and the original function's argument count. --- lua/shine/core/shared/hook.lua | 6 ++++++ test/core/shared/hook.lua | 25 +++++++++++++++++-------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index dc845c199..af80180a9 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -340,6 +340,9 @@ local function AddClassHook( ReplacementFuncTemplate, HookName, Caller, Class, M local NumArguments = GetNumArguments( OldFunc ) local ReplacementFunc if not IsType( ReplacementFuncTemplate, "string" ) then + -- Allow custom handlers to add extra arguments (taking 1 less as the first argument is OldFunc). + NumArguments = Max( NumArguments, GetNumArguments( ReplacementFuncTemplate ) - 1 ) + ReplacementFunc = CodeGen.GenerateFunctionWithArguments( [[local OldFunc, ReplacementFunc = ... return function( {FunctionArguments} ) @@ -394,6 +397,9 @@ local function AddGlobalHook( ReplacementFunc, HookName, Caller, FuncName ) local NumArguments = GetNumArguments( Func ) if not IsType( ReplacementFunc, "string" ) then + -- Allow custom handlers to add extra arguments (taking 1 less as the first argument is OldFunc). + NumArguments = Max( NumArguments, GetNumArguments( ReplacementFunc ) - 1 ) + -- Maintain backwards compatibility with custom hook handlers while still helping to remove var-args. Prev[ Path[ NumSegments ] ] = CodeGen.GenerateFunctionWithArguments( [[local OldFunc, ReplacementFunc = ... diff --git a/test/core/shared/hook.lua b/test/core/shared/hook.lua index c2b6b278d..a8ec11ac8 100644 --- a/test/core/shared/hook.lua +++ b/test/core/shared/hook.lua @@ -131,7 +131,11 @@ UnitTest:Test( "SetupGlobalHook - Handles nested global values", function( Asser [ FunctionName ] = TestFunction } - local OldFunc = Hook.SetupGlobalHook( NestedName.."."..FunctionName, "Test", "PassivePost" ) + -- Having more arguments in the replacement should mean the hooked function has 4 parameters. + local OldFunc = Hook.SetupGlobalHook( NestedName.."."..FunctionName, "Test", function( OldFunc, Arg1, Arg2, Arg3, Arg4 ) + OldFunc( Arg1, Arg2, Arg3, Arg4 ) + Hook.Broadcast( "Test", Arg1, Arg2, Arg3, Arg4 ) + end ) Assert.Equals( "Function returned from SetupGlobalHook should be the nested global value", TestFunction, OldFunc ) local NewHookedFunction = _G[ NestedName ][ FunctionName ] @@ -141,11 +145,11 @@ UnitTest:Test( "SetupGlobalHook - Handles nested global values", function( Asser local Callback = UnitTest.MockFunction() Hook.Add( "Test", Callback ) - -- Ignores the 4th argument as the original function had only 3. - NewHookedFunction( 1, 2, 3, 4 ) + -- Ignores the 5th argument as the number of arguments is the max of the original and replacement. + NewHookedFunction( 1, 2, 3, 4, 5 ) Assert.DeepEquals( "Calling the replaced function should run the hook", { - { ArgCount = 3, 1, 2, 3 } + { ArgCount = 4, 1, 2, 3, 4 } }, Callback.Invocations ) end ) @@ -162,7 +166,11 @@ UnitTest:Test( "SetupClassHook - Replaces functions only once", function( Assert local TestFunction = function( self ) end _G[ ClassName ].TestMethod = TestFunction - local OldFunc = Hook.SetupClassHook( ClassName, "TestMethod", "TestClassTestMethod", "PassivePost" ) + local function Handler( OldFunc, self, Arg1 ) + OldFunc( self ) + Hook.Broadcast( "TestClassTestMethod", self, Arg1 ) + end + local OldFunc = Hook.SetupClassHook( ClassName, "TestMethod", "TestClassTestMethod", Handler ) Assert.Equals( "Function returned from SetupClassHook should be the original method", TestFunction, OldFunc ) local NewHookedFunction = _G[ ClassName ].TestMethod @@ -171,18 +179,19 @@ UnitTest:Test( "SetupClassHook - Replaces functions only once", function( Assert -- Hooking the same function twice with the same hook name and mode should do nothing and return -- the original function from the first setup call. - OldFunc = Hook.SetupClassHook( ClassName, "TestMethod", "TestClassTestMethod", "PassivePost" ) + OldFunc = Hook.SetupClassHook( ClassName, "TestMethod", "TestClassTestMethod", Handler ) Assert.Equals( "Function returned from SetupClassHook should still be the original method", TestFunction, OldFunc ) Assert.Equals( "Class method should be unchanged", NewHookedFunction, _G[ ClassName ].TestMethod ) local Callback = UnitTest.MockFunction() Hook.Add( "TestClassTestMethod", Callback ) + -- Should pass through 2 arguments, as that's the max of the original and replacement functions. local Arg = {} - NewHookedFunction( Arg ) + NewHookedFunction( Arg, 1, 2 ) Assert.DeepEquals( "Calling the replaced function should run the hook", { - { ArgCount = 1, Arg } + { ArgCount = 2, Arg, 1 } }, Callback.Invocations ) end ) From 6723608e47d87ab818a5d3cb01c1369445adbe91 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Mon, 6 Apr 2020 17:48:39 +0100 Subject: [PATCH 09/49] Add votedraw plugin This provides a vote to end the current round as a draw. It only considers votes from players that are playing in the round, and has a lengthy delay and high vote count requirement by default. --- locale/shine/extensions/votedraw/enGB.json | 10 ++ lua/shine/core/server/config.lua | 1 + lua/shine/extensions/votedraw/client.lua | 36 ++++++ lua/shine/extensions/votedraw/server.lua | 122 +++++++++++++++++++++ lua/shine/extensions/votedraw/shared.lua | 46 ++++++++ lua/shine/lib/objects/votes.lua | 9 +- lua/shine/modules/sh_vote.lua | 27 +++-- test/lib/objects/votes.lua | 4 +- 8 files changed, 244 insertions(+), 11 deletions(-) create mode 100644 locale/shine/extensions/votedraw/enGB.json create mode 100644 lua/shine/extensions/votedraw/client.lua create mode 100644 lua/shine/extensions/votedraw/server.lua create mode 100644 lua/shine/extensions/votedraw/shared.lua diff --git a/locale/shine/extensions/votedraw/enGB.json b/locale/shine/extensions/votedraw/enGB.json new file mode 100644 index 000000000..1fe786c5f --- /dev/null +++ b/locale/shine/extensions/votedraw/enGB.json @@ -0,0 +1,10 @@ +{ + "NOTIFY_PREFIX": "[Draw Vote]", + "VOTEMENU_BUTTON": "Vote to Draw", + "PLAYER_VOTED": "{PlayerName} voted to end the round as a draw ({VotesNeeded} more {VotesNeeded:Pluralise:vote|votes} needed)", + "PLAYER_VOTED_PRIVATE": "You voted to end the round as a draw ({VotesNeeded} more {VotesNeeded:Pluralise:vote|votes} needed)", + "ERROR_ALREADY_VOTED": "You have already voted to end the round as a draw.", + "ERROR_MUST_WAIT": "You must wait for {SecondsToWait:Duration} before voting to end the round as a draw.", + "ERROR_CANNOT_VOTE_ON_CURRENT_TEAM": "You cannot vote to end the round as a draw on your current team.", + "ERROR_ROUND_NOT_STARTED": "A round must be started to vote to end it as a draw." +} \ No newline at end of file diff --git a/lua/shine/core/server/config.lua b/lua/shine/core/server/config.lua index fa7870b4c..d33a7b0bd 100644 --- a/lua/shine/core/server/config.lua +++ b/lua/shine/core/server/config.lua @@ -62,6 +62,7 @@ local DefaultConfig = { unstuck = true, usermanagement = true, votealltalk = false, + votedraw = false, voterandom = false, votesurrender = true, welcomemessages = false, diff --git a/lua/shine/extensions/votedraw/client.lua b/lua/shine/extensions/votedraw/client.lua new file mode 100644 index 000000000..314c0d2ac --- /dev/null +++ b/lua/shine/extensions/votedraw/client.lua @@ -0,0 +1,36 @@ +--[[ + Draw vote client side. +]] + +local Plugin = ... + +Plugin.VoteButtonName = "VoteDraw" + +do + local RichTextFormat = require "shine/lib/gui/richtext/format" + local RichTextMessageOptions = {} + local VoteMessageOptions = { + Colours = { + PlayerName = function( Values ) + return RichTextFormat.GetColourForPlayer( Values.PlayerName ) + end + } + } + + RichTextMessageOptions[ "PLAYER_VOTED" ] = VoteMessageOptions + + Plugin.RichTextMessageOptions = RichTextMessageOptions +end + +Shine.VoteMenu:EditPage( "Main", Plugin:WrapCallback( function( VoteMenu ) + local ButtonText = Plugin:GetPhrase( "VOTEMENU_BUTTON" ) + + local Button = VoteMenu:AddSideButton( ButtonText, function() + VoteMenu.GenericClick( "sh_votedraw" ) + end ) + + -- Allow the button to be retrieved to have its counter updated. + Button.Plugin = Plugin.VoteButtonName + Button.DefaultText = ButtonText + Button.CheckMarkXScale = 0.75 +end ) ) diff --git a/lua/shine/extensions/votedraw/server.lua b/lua/shine/extensions/votedraw/server.lua new file mode 100644 index 000000000..fc500348c --- /dev/null +++ b/lua/shine/extensions/votedraw/server.lua @@ -0,0 +1,122 @@ +--[[ + Vote to draw the current round. +]] + +local Plugin = ... + +local assert = assert +local Ceil = math.ceil +local SharedTime = Shared.GetTime + +Plugin.Version = "1.0" + +Plugin.HasConfig = true +Plugin.ConfigName = "VoteDraw.json" +Plugin.CheckConfig = true +Plugin.CheckConfigTypes = true + +Plugin.DefaultConfig = { + -- How long into a round to wait before allowing a vote to draw the game. + EnableAfterRoundStartMinutes = 20, + + -- Whether to notify everyone of votes, or just the voting player (vote menu button updates regardless). + NotifyOnVote = true, + + -- The fraction of players on both playing teams needing to vote for it to pass. + FractionNeededToPass = 0.9, + + -- How long to wait between votes before the vote is reset. + VoteTimeoutInSeconds = 60, + + -- Standard vote settings. + VoteSettings = { + ConsiderAFKPlayersInVotes = true, + AFKTimeInSeconds = 60 + } +} + +-- Show all votes to draw the game, even the last that triggers it. +Plugin.ShowLastVote = true +Plugin.VoteCommand = { + ConCommand = "sh_votedraw", + ChatCommand = "votedraw", + Help = "Votes to end the current round as a draw." +} + +function Plugin:OnVotePassed() + local Gamerules = GetGamerules() + assert( Gamerules, "Couldn't find the gamerules!" ) + + Gamerules:DrawGame() +end + +function Plugin:CanStartVote() + local Gamerules = GetGamerules() + if not Gamerules or not Gamerules:GetGameStarted() then + return false, "ERROR_ROUND_NOT_STARTED" + end + + local StartTime = Gamerules:GetGameStartTime() + local TimeTillVoteAllowed = StartTime + self.Config.EnableAfterRoundStartMinutes * 60 - SharedTime() + if TimeTillVoteAllowed > 0 then + return false, "ERROR_MUST_WAIT", { + SecondsToWait = Ceil( TimeTillVoteAllowed ) + } + end + + return true +end + +local function GetNumPlayersOnTeam( Team, AFKTime, AFKKick ) + local Count = 0 + + Team:ForEachPlayer( function( Player ) + local Client = Player:GetClient() + if Client and Client:GetIsVirtual() then return end + + if not Client or not ( AFKTime and AFKKick and AFKKick:IsAFKFor( Client, AFKTime ) ) then + Count = Count + 1 + end + end ) + + return Count +end + +function Plugin:PostJoinTeam( Gamerules, Player, OldTeam, NewTeam ) + if not Shine.IsPlayingTeam( OldTeam ) or Shine.IsPlayingTeam( NewTeam ) then return end + + -- If a player goes to the ready room or spectate, remove their vote. + local Client = Player:GetClient() + if self.Vote:RemoveVote( Client ) and not self.Vote:CheckForSuccess() then + self:UpdateVoteCounters( self.Vote ) + self:NotifyVoteReset( Client ) + end +end + +function Plugin:GetPlayerCountForVote() + local Gamerules = GetGamerules() + assert( Gamerules, "Cannot get player count without the gamerules!" ) + + local AFKTime = not self.Config.VoteSettings.ConsiderAFKPlayersInVotes and self.Config.VoteSettings.AFKTimeInSeconds + local Enabled, AFKKick = Shine:IsExtensionEnabled( "afkkick" ) + + local Count = 0 + for i = 1, 2 do + Count = Count + GetNumPlayersOnTeam( Gamerules:GetTeam( i ), AFKTime, Enabled and AFKKick ) + end + + return Count +end + +function Plugin:CanClientVote( Client ) + if Client:GetIsVirtual() then + return false, "ERROR_BOT_CANNOT_VOTE" + end + + local Player = Client:GetControllingPlayer() + if not Player or not Player.GetTeamNumber or not Shine.IsPlayingTeam( Player:GetTeamNumber() ) then + return false, "ERROR_CANNOT_VOTE_ON_CURRENT_TEAM" + end + + return true +end diff --git a/lua/shine/extensions/votedraw/shared.lua b/lua/shine/extensions/votedraw/shared.lua new file mode 100644 index 000000000..42d0a55c1 --- /dev/null +++ b/lua/shine/extensions/votedraw/shared.lua @@ -0,0 +1,46 @@ +--[[ + Draw vote shared. +]] + +local Plugin = Shine.Plugin( ... ) +Plugin.NotifyPrefixColour = { + 224, 255, 210 +} +Plugin.UseCustomVoteTiming = true +Plugin.HandlesVoteConfig = true +Plugin.FractionConfigKey = "FractionNeededToPass" + +function Plugin:SetupDataTable() + self:CallModuleEvent( "SetupDataTable" ) + + local MessageTypes = { + PlayerVote = { + PlayerName = self:GetNameNetworkField(), + VotesNeeded = "integer" + }, + PrivateVote = { + VotesNeeded = "integer" + }, + VoteWaitTime = { + SecondsToWait = "integer" + } + } + + self:AddNetworkMessages( "AddTranslatedNotify", { + [ MessageTypes.PlayerVote ] = { + "PLAYER_VOTED" + }, + [ MessageTypes.PrivateVote ] = { + "PLAYER_VOTED_PRIVATE" + } + } ) + self:AddNetworkMessages( "AddTranslatedError", { + [ MessageTypes.VoteWaitTime ] = { + "ERROR_MUST_WAIT" + } + } ) +end + +Shine.LoadPluginModule( "sh_vote.lua", Plugin, true ) + +return Plugin diff --git a/lua/shine/lib/objects/votes.lua b/lua/shine/lib/objects/votes.lua index 8ce0adf45..08db3463e 100644 --- a/lua/shine/lib/objects/votes.lua +++ b/lua/shine/lib/objects/votes.lua @@ -61,7 +61,10 @@ function VoteMeta:RemoveVote( Client ) if self.Voted[ Client ] then self.Voted[ Client ] = nil self.Votes = Max( self.Votes - 1, 0 ) + return true end + + return false end function VoteMeta:ClientDisconnect( Client ) @@ -90,13 +93,17 @@ function VoteMeta:AddVote( Client ) end function VoteMeta:CheckForSuccess() - if self.Votes <= 0 then return end + if self.Votes <= 0 then return false end if self.Votes >= self.VotesNeeded() then self.LastSuccessTime = SharedTime() self.OnSuccess() self:Reset() + + return true end + + return false end function VoteMeta:HasSucceededOnLastVote() diff --git a/lua/shine/modules/sh_vote.lua b/lua/shine/modules/sh_vote.lua index beb803cb7..e0c627355 100644 --- a/lua/shine/modules/sh_vote.lua +++ b/lua/shine/modules/sh_vote.lua @@ -15,7 +15,7 @@ if Server then function Module:ResetVoteCounters() self.dt.CurrentVotes = 0 self.dt.RequiredVotes = 0 - self:SendNetworkMessage( nil, "HasVoted", { HasVoted = false }, true ) + self:NotifyVoteReset( nil ) end function Module:UpdateVoteCounters( VoteObject ) @@ -27,6 +27,10 @@ if Server then self:SendNetworkMessage( Client, "HasVoted", { HasVoted = true }, true ) end + function Module:NotifyVoteReset( Client ) + self:SendNetworkMessage( Client, "HasVoted", { HasVoted = false }, true ) + end + if UseStandardBehaviour then local Ceil = math.ceil local next = next @@ -35,12 +39,16 @@ if Server then Shine.LoadPluginModule( "vote.lua", Plugin ) + local FractionKey = Plugin.FractionConfigKey or "PercentNeeded" + Module.DefaultConfig = { - BlockUntilSecondsIntoMap = 0, VoteTimeoutInSeconds = 60, - PercentNeeded = 0.6, + [ FractionKey ] = 0.6, NotifyOnVote = true } + if not Plugin.UseCustomVoteTiming then + Module.DefaultConfig.BlockUntilSecondsIntoMap = 0 + end function Module:Initialise() assert( Shine.IsCallable( self.OnVotePassed ), "Plugin has not provided OnVotePassed method" ) @@ -66,7 +74,7 @@ if Server then function Module:GetVotesNeeded() local PlayerCount = self:GetPlayerCountForVote() - return Ceil( PlayerCount * self.Config.PercentNeeded ) + return Ceil( PlayerCount * self.Config[ FractionKey ] ) end function Module:ClientConnect( Client ) @@ -128,10 +136,11 @@ if Server then return end - if self.Vote:HasSucceededOnLastVote() then return end + local Succeeded = self.Vote:HasSucceededOnLastVote() + if Succeeded and not self.ShowLastVote then return end local MessageParams = self:GetVoteNotificationParams() - MessageParams.VotesNeeded = self.Vote:GetVotesNeeded() + MessageParams.VotesNeeded = Succeeded and 0 or self.Vote:GetVotesNeeded() if self.Config.NotifyOnVote then MessageParams.PlayerName = PlayerName @@ -140,8 +149,10 @@ if Server then self:SendTranslatedNotify( Client, "PLAYER_VOTED_PRIVATE", MessageParams ) end - self:UpdateVoteCounters( self.Vote ) - self:NotifyVoted( Client ) + if not Succeeded then + self:UpdateVoteCounters( self.Vote ) + self:NotifyVoted( Client ) + end end, true ):Help( self.VoteCommand.Help ) end end diff --git a/test/lib/objects/votes.lua b/test/lib/objects/votes.lua index 1658801a8..acd513ffd 100644 --- a/test/lib/objects/votes.lua +++ b/test/lib/objects/votes.lua @@ -43,7 +43,7 @@ UnitTest:Test( "Removing votes", function( Assert ) Assert.Equals( "Expected one more vote required to pass", 1, Vote:GetVotesNeeded() ) Assert.False( "Vote should not have passed yet", Success ) - Vote:RemoveVote( 1 ) + Assert.True( "Should have removed client 1's vote", Vote:RemoveVote( 1 ) ) Assert.False( "Client 1 should not have voted", Vote:GetHasClientVoted( 1 ) ) Assert.Equals( "Expected vote count to be reset", 0, Vote:GetVotes() ) @@ -51,7 +51,7 @@ UnitTest:Test( "Removing votes", function( Assert ) Assert.False( "Vote should not have passed yet", Success ) -- Removing the vote again should do nothing. - Vote:RemoveVote( 1 ) + Assert.False( "Should not have removed a non-existent vote", Vote:RemoveVote( 1 ) ) Assert.Equals( "Expected vote count to be reset", 0, Vote:GetVotes() ) Assert.Equals( "Expected two more vote required to pass", 2, Vote:GetVotesNeeded() ) From 930210cf0dc3b966e1ff2673eee7ec6e5d22d8a0 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 7 Apr 2020 16:26:09 +0100 Subject: [PATCH 10/49] Add config validation to vote draw plugin --- lua/shine/extensions/votedraw/server.lua | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lua/shine/extensions/votedraw/server.lua b/lua/shine/extensions/votedraw/server.lua index fc500348c..1e2590b0c 100644 --- a/lua/shine/extensions/votedraw/server.lua +++ b/lua/shine/extensions/votedraw/server.lua @@ -35,6 +35,18 @@ Plugin.DefaultConfig = { } } +do + local Validator = Shine.Validator() + + Validator:AddFieldRule( "FractionNeededToPass", Validator.Clamp( 0, 1 ) ) + Validator:AddFieldRule( "EnableAfterRoundStartMinutes", Validator.Min( 0 ) ) + Validator:AddFieldRule( "VoteTimeoutInSeconds", Validator.Min( 0 ) ) + Validator:CheckTypesAgainstDefault( "VoteSettings", Plugin.DefaultConfig.VoteSettings ) + Validator:AddFieldRule( "VoteSettings.AFKTimeInSeconds", Validator.Min( 0 ) ) + + Plugin.ConfigValidator = Validator +end + -- Show all votes to draw the game, even the last that triggers it. Plugin.ShowLastVote = true Plugin.VoteCommand = { From 4bdd5706c6b67bb41a5b67bcbd3c9ad445e367da Mon Sep 17 00:00:00 2001 From: Person8880 Date: Fri, 10 Apr 2020 11:28:15 +0100 Subject: [PATCH 11/49] Improve alpha easing for ColourLabel Make the background the only element that eases. --- lua/shine/lib/gui/objects/colour_label.lua | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/lua/shine/lib/gui/objects/colour_label.lua b/lua/shine/lib/gui/objects/colour_label.lua index 5475641ec..a0a2c06d3 100644 --- a/lua/shine/lib/gui/objects/colour_label.lua +++ b/lua/shine/lib/gui/objects/colour_label.lua @@ -19,7 +19,8 @@ function ColourLabel:Initialise() self.IsVertical = false self.Background = self:MakeGUIItem() - self.Background:SetColor( Colour( 0, 0, 0, 0 ) ) + self.Background:SetShader( SGUI.Shaders.Invisible ) + self.Background:SetColor( Colour( 1, 1, 1, 1 ) ) end function ColourLabel:MakeVertical() @@ -60,22 +61,12 @@ function ColourLabel:SetShadow( Params ) self:ForEach( "Labels", "SetShadow", Params ) end -function ColourLabel:AlphaTo( ... ) - self:ForEach( "Labels", "AlphaTo", ... ) -end - function ColourLabel:SetText( TextContent ) local Easing if #self.Labels > 0 then for i = 1, #self.Labels do local Label = self.Labels[ i ] - local LabelAlphaEase = Label:GetEasing( "Alpha" ) - if LabelAlphaEase and not Easing then - Easing = LabelAlphaEase - end Label:Destroy() - - self.Layout.Elements[ i ] = nil self.Labels[ i ] = nil end end @@ -98,16 +89,12 @@ function ColourLabel:SetText( TextContent ) Label:SetFontScale( self.Font, self.TextScale ) Label:SetText( Text ) Label:SetColour( SGUI.CopyColour( Colour ) ) + Label:SetInheritsParentAlpha( true ) self.Layout:AddElement( Label ) Count = Count + 1 self.Labels[ Count ] = Label end - - if Easing and Easing.Elapsed < Easing.Duration then - self:AlphaTo( nil, Easing.Start, Easing.End, -Easing.Elapsed, Easing.Duration, Easing.Callback, - Easing.EaseFunc, Easing.Power ) - end end function ColourLabel:GetSize() From b0b1fcda5bda2de23898143503381ada9364cb84 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Fri, 10 Apr 2020 11:33:33 +0100 Subject: [PATCH 12/49] Add easing helpers for SGUI Converts easing functions from the built-in easing library into a form that can be used with SGUI. --- lua/shine/extensions/improvedchat/client.lua | 19 ++-------- lua/shine/lib/gui/util/easing.lua | 40 ++++++++++++++++++++ 2 files changed, 44 insertions(+), 15 deletions(-) create mode 100644 lua/shine/lib/gui/util/easing.lua diff --git a/lua/shine/extensions/improvedchat/client.lua b/lua/shine/extensions/improvedchat/client.lua index 9b2209e5b..426e9d7a7 100644 --- a/lua/shine/extensions/improvedchat/client.lua +++ b/lua/shine/extensions/improvedchat/client.lua @@ -136,25 +136,14 @@ Hook.CallAfterFileLoad( "lua/GUIChat.lua", function() MaxChatWidth = Value * 0.01 end ) - -- Ensure easing functions are loaded. - Script.Load( "lua/tweener/Tweener.lua" ) + local Easing = require "shine/lib/gui/util/easing" -- Fade fast to avoid making text hard to read. - local OutExpo = Easing.outExpo - local function FadingEase( Progress ) - return OutExpo( Progress, 0, 1, 1 ) - end - - local InExpo = Easing.inExpo - local function FadingInEase( Progress ) - return InExpo( Progress, 0, 1, 1 ) - end + local FadingEase = Easing.GetEaser( "OutExpo" ) + local FadingInEase = Easing.GetEaser( "InExpo" ) -- Move more smoothly to avoid sudden jumps. - local OutSine = Easing.outSine - local function MovementEase( Progress ) - return OutSine( Progress, 0, 1, 1 ) - end + local MovementEase = Easing.GetEaser( "OutSine" ) local AnimDuration = 0.25 local function IsAnimationEnabled( self ) diff --git a/lua/shine/lib/gui/util/easing.lua b/lua/shine/lib/gui/util/easing.lua new file mode 100644 index 000000000..f10617dfe --- /dev/null +++ b/lua/shine/lib/gui/util/easing.lua @@ -0,0 +1,40 @@ +--[[ + Easing helpers. +]] + +Script.Load( "lua/tweener/Tweener.lua" ) + +local EasingFunctions = _G.Easing + +local StringCaseFormatType = string.CaseFormatType +local StringTransformCase = string.TransformCase + +local Easing = {} +local ConvertedEasers = {} + +local function ConvertName( Name ) + return StringTransformCase( Name, StringCaseFormatType.UPPER_CAMEL, StringCaseFormatType.LOWER_CAMEL ) +end + +--[[ + Gets an easer by name (from _G.Easing) for use with SGUI. + + Input: Name of the easer to get. + Output: An easing function that can be passed to SGUI easing functions. +]] +function Easing.GetEaser( TypeName ) + local ConvertedEaser = ConvertedEasers[ TypeName ] + if not ConvertedEaser then + local Easer = EasingFunctions[ ConvertName( TypeName ) ] + Shine.AssertAtLevel( Easer, "Unkonwn easer: %s", 3, TypeName ) + + ConvertedEaser = function( Progress ) + return Easer( Progress, 0, 1, 1 ) + end + ConvertedEasers[ TypeName ] = ConvertedEaser + end + + return ConvertedEaser +end + +return Easing From 692b80b6553e4585a5950114d23320d522d70565 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Fri, 10 Apr 2020 11:40:10 +0100 Subject: [PATCH 13/49] Improve chatbox chat command auto-complete * Fix labels sometimes becoming invisible. * Fix a rare script error due to modifying the layout elements directly. * Improve fading to fade out the background segment of an entry along with the label. --- lua/shine/extensions/chatbox/client.lua | 92 ++++++++++++++++++------- lua/shine/lib/gui/base_control.lua | 4 ++ 2 files changed, 73 insertions(+), 23 deletions(-) diff --git a/lua/shine/extensions/chatbox/client.lua b/lua/shine/extensions/chatbox/client.lua index 4fc721f06..1bf39c607 100644 --- a/lua/shine/extensions/chatbox/client.lua +++ b/lua/shine/extensions/chatbox/client.lua @@ -1362,6 +1362,22 @@ do self.TextEntry:SetText( Text ) end + local BackgroundAlpha = 0.65 + local Easing = require "shine/lib/gui/util/easing" + local FadeOutTransition = { + Type = "Alpha", + EndValue = 0, + Duration = 0.2, + EasingFunction = Easing.GetEaser( "OutExpo" ) + } + local FadeInTransition = { + Type = "Alpha", + StartValue = 0, + EndValue = BackgroundAlpha, + Duration = 0.2, + EasingFunction = Easing.GetEaser( "InSine" ) + } + local function ApplyAutoCompletionResults( self, Results ) if not self.Visible then return end @@ -1378,49 +1394,77 @@ do local ResultPanel = self.AutoCompletePanel if not ResultPanel then - ResultPanel = SGUI:Create( "Panel", self.MainPanel ) + ResultPanel = SGUI:Create( "Column", self.MainPanel ) ResultPanel:SetIsSchemed( false ) + ResultPanel:SetShader( SGUI.Shaders.Invisible ) self.AutoCompletePanel = ResultPanel ResultPanel:SetAnchor( "BottomLeft" ) - - local Padding = self.MainPanel.Layout:GetPadding() - ResultPanel:SetColour( Colour( 0, 0, 0, 0.65 ) ) - ResultPanel:SetLayout( SGUI.Layout:CreateLayout( "Vertical", { - Padding = Padding - } ) ) + ResultPanel:SetColour( Colour( 0, 0, 0, 1 ) ) end local Layout = ResultPanel.Layout local Elements = Layout.Elements - local ResultPanelPadding = ResultPanel.Layout:GetComputedPadding() + local ResultPanelPadding = self.MainPanel.Layout:GetComputedPadding() local XPadding = ResultPanelPadding[ 1 ] + ResultPanelPadding[ 3 ] local YPadding = ResultPanelPadding[ 2 ] + ResultPanelPadding[ 4 ] local Size = Vector2( self.MainPanel:GetSize().x, YPadding ) for i = 1, Max( #Results, #Elements ) do - local Label = Elements[ i ] + local LabelRow = Elements[ i ] if not Results[ i ] or ( i > 1 and Results.ParameterIndex and not ResultsAreForCorrectArgument ) then - if Label then - Label:AlphaTo( nil, nil, 0, 0, 0.3, function() - if not Label then return end + if LabelRow then + FadeOutTransition.Callback = function() + if not LabelRow then return end - Label:Destroy() - Label = nil - Elements[ i ] = nil - end ) + LabelRow:Destroy() + LabelRow = nil + end + LabelRow:ApplyTransition( FadeOutTransition ) end else local ShouldFade - if not Label then + if not LabelRow then + ShouldFade = true + + local Tree = SGUI:BuildTree( { + Parent = ResultPanel, + { + ID = "Container", + Class = "Row", + Props = { + IsSchemed = false, + Padding = Spacing( + ResultPanelPadding[ 1 ], + i == 1 and ResultPanelPadding[ 2 ] or 0, + ResultPanelPadding[ 3 ], + ResultPanelPadding[ 4 ] + ), + Colour = Colour( 0, 0, 0, BackgroundAlpha ), + AutoSize = UnitVector( Percentage( 100 ), Units.Auto() ) + }, + Children = { + { + ID = "Label", + Class = "ColourLabel", + Props = { + IsSchemed = false, + Alpha = 1 / BackgroundAlpha, + InheritsParentAlpha = true + } + } + } + } + } ) + + LabelRow = Tree.Container + LabelRow.Label = Tree.Label + elseif LabelRow:GetAlpha() + 0.001 < BackgroundAlpha then ShouldFade = true - Label = SGUI:Create( "ColourLabel", ResultPanel ) - Label:SetIsSchemed( false ) - Label:SetMargin( Spacing( 0, 0, 0, Scaled( 2, self.ScalarScale ) ) ) - Elements[ i ] = Label end + local Label = LabelRow.Label local Result = Results[ i ] Result.ParameterIndex = Results.ParameterIndex @@ -1465,10 +1509,12 @@ do end Label:SetText( TextContent ) - Label:InvalidateLayout( true ) + LabelRow:InvalidateLayout( true ) if ShouldFade then - Label:AlphaTo( nil, 0, 1, 0, 0.3, nil, math.EaseIn ) + LabelRow:ApplyTransition( FadeInTransition ) + else + LabelRow:StopAlpha() end local LabelSize = Label:GetSize() diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index 34882ab3a..edf4223d3 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -830,6 +830,10 @@ function ControlMeta:SetAlpha( Alpha ) self.Background:SetColor( Colour ) end +function ControlMeta:GetAlpha() + return self.Background:GetColor().a +end + function ControlMeta:GetTextureWidth() return self.Background:GetTextureWidth() end From f50644c7140cecd9fd6896363a8cc782dbb8aca0 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Fri, 10 Apr 2020 14:05:35 +0100 Subject: [PATCH 14/49] Improve unstuck to reduce failure rate * Add a distance tolerance to account for players that are stuck but still jittering slightly. * Use the player's view position as the origin to look for spawn points from to avoid small objects at the player's feet being considered walls. --- lua/shine/extensions/unstuck/server.lua | 54 ++++++++++++++++--------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/lua/shine/extensions/unstuck/server.lua b/lua/shine/extensions/unstuck/server.lua index 3b22c36ff..3f04c1615 100644 --- a/lua/shine/extensions/unstuck/server.lua +++ b/lua/shine/extensions/unstuck/server.lua @@ -7,7 +7,7 @@ local Shine = Shine local Ceil = math.ceil local Plugin = ... -Plugin.Version = "1.1" +Plugin.Version = "1.2" Plugin.HasConfig = true Plugin.ConfigName = "Unstuck.json" @@ -15,10 +15,12 @@ Plugin.ConfigName = "Unstuck.json" Plugin.DefaultConfig = { -- The distance around the player to check for a valid location. DistanceToCheck = 6, + -- The maximum distance from the original position a player is allowed to be when unsticking occurs. + MovementToleranceDistance = 0.5, -- The time between successful unstick requests (in seconds). - TimeBetweenUse = 30, + TimeBetweenUseInSeconds = 30, -- The minimum time to wait between unstick requests (in seconds). - MinTime = 5, + MinTimeInSeconds = 5, -- How long to wait before moving a player (forces them to be stationary for this time). DelayBeforeMovingInSeconds = 0 } @@ -28,6 +30,16 @@ Plugin.CheckConfigTypes = true Plugin.PrintName = "Unstuck" +Plugin.ConfigMigrationSteps = { + { + VersionTo = "1.2", + Apply = Shine.Migrator() + :RenameField( "TimeBetweenUse", "TimeBetweenUseInSeconds" ) + :RenameField( "MinTime", "MinTimeInSeconds" ) + :AddField( "MovementToleranceDistance", Plugin.DefaultConfig.MovementToleranceDistance ) + } +} + function Plugin:Initialise() self.NextUsageTimes = {} self.ClientsBeingUnstuck = {} @@ -105,23 +117,18 @@ function Plugin:UnstickPlayer( Player, Pos ) return false end + local Filter = EntityFilterAll() local Height, Radius = GetTraceCapsuleFromExtents( Bounds ) - - local SpawnPoint - local ResourceNear - local i = 1 - local Range = self.Config.DistanceToCheck + local SpawnPoint - repeat - SpawnPoint = GetRandomSpawnForCapsule( Height, Radius, Pos, 2, Range, EntityFilterAll() ) + for i = 1, 10 do + SpawnPoint = GetRandomSpawnForCapsule( Height, Radius, Pos, 2, Range, Filter ) - if SpawnPoint then - ResourceNear = #GetEntitiesWithinRange( "ResourcePoint", SpawnPoint, 2 ) > 0 + if SpawnPoint and #GetEntitiesWithinRange( "ResourcePoint", SpawnPoint, 2 ) == 0 then + break end - - i = i + 1 - until not ResourceNear or i > 100 + end if SpawnPoint then Player:SetOrigin( SpawnPoint ) @@ -177,23 +184,30 @@ function Plugin:CreateCommands() end local CurrentOrigin = Player:GetOrigin() - if CurrentOrigin ~= InitialOrigin then + local Distance = CurrentOrigin:GetDistance( InitialOrigin ) + if Distance > self.Config.MovementToleranceDistance then self:NotifyTranslatedError( Client, "ERROR_MOVED" ) return end - local Success = self:UnstickPlayer( Player, CurrentOrigin ) + -- Use an origin that's not at the player's feet to avoid the search for a spawnpoint thinking small objects + -- on the ground are walls between the player and a valid location. + local SpawnPointOrigin = CurrentOrigin + Player:GetCoords().yAxis * Player:GetViewOffset().y + local TraceResult = Shared.TraceRay( + CurrentOrigin, SpawnPointOrigin, CollisionRep.Move, PhysicsMask.AllButPCs, EntityFilterAll() + ) + local Success = self:UnstickPlayer( Player, TraceResult.endPoint ) if Success then self:NotifyTranslated( Client, "SUCCESS" ) - self.NextUsageTimes[ Client ] = Time + self.Config.TimeBetweenUse + self.NextUsageTimes[ Client ] = Time + self.Config.TimeBetweenUseInSeconds else self:SendTranslatedError( Client, "ERROR_FAIL", { - TimeLeft = Ceil( self.Config.MinTime ) + TimeLeft = Ceil( self.Config.MinTimeInSeconds ) } ) - self.NextUsageTimes[ Client ] = Time + self.Config.MinTime + self.NextUsageTimes[ Client ] = Time + self.Config.MinTimeInSeconds end end From 15d9e0b223060cd8aa025034805d08ff1248ad07 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 11 Apr 2020 14:54:43 +0100 Subject: [PATCH 15/49] Improve mouse hover tracking in SGUI Track when the mouse enters and leaves elements, and only call the OnMouseMove event for child elements when the mouse is inside their parent or the mouse has just left the parent. These changes also simplify mouse bounds checks, removing highlight multipliers and moving custom bounds to their own method. --- lua/shine/core/client/votemenu_gui.lua | 21 +- lua/shine/lib/gui.lua | 39 +++- lua/shine/lib/gui/base_control.lua | 215 ++++++++++++++++----- lua/shine/lib/gui/objects/checkbox.lua | 30 ++- lua/shine/lib/gui/objects/colour_label.lua | 2 + lua/shine/lib/gui/objects/label.lua | 5 +- lua/shine/lib/gui/objects/list.lua | 5 +- lua/shine/lib/gui/objects/panel.lua | 39 +++- lua/shine/lib/gui/objects/richtext.lua | 5 + lua/shine/lib/gui/objects/scrollbar.lua | 2 +- lua/shine/lib/gui/objects/slider.lua | 6 +- lua/shine/lib/gui/objects/textentry.lua | 42 ++-- lua/shine/lib/gui/objects/tooltip.lua | 5 - lua/shine/lib/gui/objects/webpage.lua | 2 +- 14 files changed, 304 insertions(+), 114 deletions(-) diff --git a/lua/shine/core/client/votemenu_gui.lua b/lua/shine/core/client/votemenu_gui.lua index 9c99484f0..d5a113ddb 100644 --- a/lua/shine/core/client/votemenu_gui.lua +++ b/lua/shine/core/client/votemenu_gui.lua @@ -505,7 +505,9 @@ local function AddButton( self, Pos, Anchor, Text, DoClick ) DoClick = DoClick, IsSchemed = false } - Button:SetHighlightOnMouseOver( true, 1, true ) + -- Highlighting buttons is disabled until they are in the correct position. This avoids multiple buttons flashing + -- highlighted when the menu is opened or the page is changed. + Button.OnAnimationComplete = function() Button:SetHighlightOnMouseOver( true, true ) end Button.ClickDelay = 0 return Button @@ -520,9 +522,12 @@ local function HandleButton( Button, Text, DoClick, StartPos, EndPos ) if Shine.Config.AnimateUI then Button:SetPos( StartPos ) - Button:MoveTo( nil, nil, EndPos, 0, EasingTime ) + Button:SetHighlightOnMouseOver( false ) + Button:MoveTo( nil, nil, EndPos, 0, EasingTime, Button.OnAnimationComplete ) else Button:SetPos( EndPos ) + Button:StopMoving() + Button.OnAnimationComplete() end end @@ -549,7 +554,9 @@ function VoteMenu:AddTopButton( Text, DoClick ) Buttons.Top = TopButton if Shine.Config.AnimateUI then - TopButton:MoveTo( nil, nil, EndPos, 0, EasingTime ) + TopButton:MoveTo( nil, nil, EndPos, 0, EasingTime, TopButton.OnAnimationComplete ) + else + TopButton.OnAnimationComplete() end return TopButton @@ -578,7 +585,9 @@ function VoteMenu:AddBottomButton( Text, DoClick ) Buttons.Bottom = BottomButton if Shine.Config.AnimateUI then - BottomButton:MoveTo( nil, nil, EndPos, 0, EasingTime ) + BottomButton:MoveTo( nil, nil, EndPos, 0, EasingTime, BottomButton.OnAnimationComplete ) + else + BottomButton.OnAnimationComplete() end return BottomButton @@ -669,12 +678,14 @@ function VoteMenu:PositionButton( Button, Index, MaxIndex, Align, IgnoreAnim ) if not IgnoreAnim and Shine.Config.AnimateUI then local Size = self.Background:GetSize() + Button:SetHighlightOnMouseOver( false ) Button:SetPos( Vector( Align == GUIItem.Right and -Size.x * 0.5 or 0, Size.y * 0.5, 0 ) ) - Button:MoveTo( nil, nil, Pos, 0, EasingTime ) + Button:MoveTo( nil, nil, Pos, 0, EasingTime, Button.OnAnimationComplete ) else Button:SetPos( Pos ) Button:StopMoving() + Button.OnAnimationComplete() end end diff --git a/lua/shine/lib/gui.lua b/lua/shine/lib/gui.lua index c84cb9d84..dc8385727 100644 --- a/lua/shine/lib/gui.lua +++ b/lua/shine/lib/gui.lua @@ -66,17 +66,23 @@ local ControlMeta = {} SGUI.BaseControl = ControlMeta SGUI.PropertyModifiers = { - InvalidatesLayout = function( self, Value ) + InvalidatesLayout = function( self ) self:InvalidateLayout() end, - InvalidatesLayoutNow = function( self, Value ) + InvalidatesLayoutNow = function( self ) self:InvalidateLayout( true ) end, - InvalidatesParent = function( self, Value ) + InvalidatesParent = function( self ) self:InvalidateParent() end, - InvalidatesParentNow = function( self, Value ) + InvalidatesParentNow = function( self ) self:InvalidateParent( true ) + end, + InvalidatesMouseState = function( self ) + self:InvalidateMouseState() + end, + InvalidatesMouseStateNow = function( self ) + self:InvalidateMouseState( true ) end } do @@ -381,9 +387,14 @@ do Window:SetLayer( Window.OverrideLayer or self.BaseLayer + i ) end - if Window ~= self.FocusedWindow and self.IsValid( self.FocusedWindow ) - and self.FocusedWindow.OnLoseWindowFocus then - self.FocusedWindow:OnLoseWindowFocus( Window ) + if Window ~= self.FocusedWindow then + if self.IsValid( self.FocusedWindow ) and self.FocusedWindow.OnLoseWindowFocus then + self.FocusedWindow:OnLoseWindowFocus( Window ) + end + + if self.IsValid( Window ) and Window.OnGainWindowFocus then + Window:OnGainWindowFocus() + end end self.FocusedWindow = Window @@ -1164,6 +1175,11 @@ function SGUI.GetCursorPos() return GetCursorPosScreen() end +local GetMouseVisible = Client.GetMouseVisible +function SGUI.IsMouseVisible() + return GetMouseVisible() +end + local ScrW = Client.GetScreenWidth local ScrH = Client.GetScreenHeight @@ -1221,6 +1237,7 @@ local IsMainMenuOpen = MainMenu_GetIsOpened ]] Hook.Add( "OnMapLoad", "LoadGUIElements", function() GetCursorPosScreen = Client.GetCursorPosScreen + GetMouseVisible = Client.GetMouseVisible ScrW = Client.GetScreenWidth ScrH = Client.GetScreenHeight IsMainMenuOpen = MainMenu_GetIsOpened @@ -1237,6 +1254,14 @@ Hook.CallAfterFileLoad( "lua/menu/MouseTracker.lua", function() local Listener = { OnMouseMove = function( _, LMB ) SGUI:CallEvent( false, "OnMouseMove", LMB ) + + if + SGUI.IsValid( SGUI.MouseDownControl ) and + SGUI.MouseDownControl.__LastMouseMove ~= SGUI.FrameNumber() + then + -- Make sure the focused control still sees mouse movements until releasing the mouse button. + SGUI.MouseDownControl:OnMouseMove( LMB ) + end end, OnMouseWheel = function( _, Down ) if IsMainMenuOpen() then return end diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index edf4223d3..cebf737a5 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -59,6 +59,9 @@ function ControlMeta:Initialise() self.UseScheme = true self.PropagateSkin = true self.Stencilled = false + + self.MouseHasEntered = false + self.MouseStateIsInvalid = false end --[[ @@ -809,8 +812,13 @@ end function ControlMeta:SetSize( SizeVec ) if not self.Background then return end + local OldSize = self.Background:GetSize() + if OldSize == SizeVec then return end + self.Background:SetSize( SizeVec ) self:InvalidateLayout() + self:InvalidateMouseState() + self:OnPropertyChanged( "Size", SizeVec ) end --[[ @@ -1087,10 +1095,15 @@ end Controls may override this. ]] -function ControlMeta:SetPos( Vec ) +function ControlMeta:SetPos( Pos ) if not self.Background then return end - self.Background:SetPosition( Vec ) + local OldPos = self.Background:GetPosition() + if Pos == OldPos then return end + + self.Background:SetPosition( Pos ) + self:InvalidateMouseState() + self:OnPropertyChanged( "Pos", Pos ) end function ControlMeta:GetPos() @@ -1230,7 +1243,18 @@ do -- We call this so many times it really needs to be local, not global. local GetCursorPos = SGUI.GetCursorPos - local function IsInBox( Pos, Size, Mult, MaxX, MaxY ) + local function IsInBox( BoxW, BoxH, X, Y ) + return X >= 0 and X < BoxW and Y >= 0 and Y < BoxH + end + + local function IsInElementBox( ElementPos, ElementSize ) + local X, Y = GetCursorPos() + X = X - ElementPos.x + Y = Y - ElementPos.y + return IsInBox( ElementSize.x, ElementSize.y, X, Y ), X, Y, ElementSize, ElementPos + end + + local function ApplyMultiplier( Size, Mult ) if Mult then if IsType( Mult, "number" ) then Size = Size * Mult @@ -1239,23 +1263,26 @@ do Size.y = Size.y * Mult.y end end + return Size + end - MaxX = MaxX or Size.x - MaxY = MaxY or Size.y - - local X, Y = GetCursorPos() - - local InX = X >= Pos.x and X < Pos.x + MaxX - local InY = Y >= Pos.y and Y < Pos.y + MaxY - - local PosX = X - Pos.x - local PosY = Y - Pos.y - - if InX and InY then - return true, PosX, PosY, Size, Pos - end + --[[ + Gets whether the mouse cursor is inside the given bounds, relative to the given GUIItem. - return false, PosX, PosY + Inputs: + 1. Element to check. + 2. Width of the bounding box. + 3. Height of the bounding box. + Outputs: + 1. Boolean value to indicate whether the mouse is inside. + 2. X position of the mouse relative to the element. + 3. Y position of the mouse relative to the element. + 4. The size of the bounding box used. + 5. The element's absolute screen position. + ]] + function ControlMeta:MouseInBounds( Element, BoundsW, BoundsH ) + local Pos = Element:GetScreenPosition( SGUI.GetScreenSize() ) + return IsInElementBox( Pos, Vector2( BoundsW, BoundsH ) ) end --[[ @@ -1265,35 +1292,42 @@ do Inputs: 1. Element to check. 2. Multiplier value to increase/reduce the size of the bounding box. - 3. X value to override the width of the bounding box. - 4. Y value to override the height of the bounding box. Outputs: 1. Boolean value to indicate whether the mouse is inside. 2. X position of the mouse relative to the element. 3. Y position of the mouse relative to the element. - 4. If the mouse is inside, the size of the bounding box used. - 5. If the mouse is inside, the element's absolute screen position. + 4. The size of the bounding box used. + 5. The element's absolute screen position. ]] - function ControlMeta:MouseIn( Element, Mult, MaxX, MaxY ) + function ControlMeta:MouseIn( Element, Mult ) if not Element then return end local Pos = Element:GetScreenPosition( SGUI.GetScreenSize() ) local Size = Element:GetScaledSize() - return IsInBox( Pos, Size, Mult, MaxX, MaxY ) + return IsInElementBox( Pos, ApplyMultiplier( Size, Mult ) ) + end + + --[[ + Gets the bounds to use when checking whether the mouse is in a control. + + Override this to change how mouse enter/leave detection works. + ]] + function ControlMeta:GetMouseBounds() + return self:GetSize() end --[[ - Similar to MouseIn, but uses the control's native GetScreenPos and GetSize instead + Similar to MouseIn, but uses the control's native GetScreenPos and GetMouseBounds instead of a GUIItem's. Useful for controls whose size/position does not match a GUIItem directly. ]] - function ControlMeta:MouseInControl( Mult, MaxX, MaxY ) + function ControlMeta:MouseInControl( Mult ) local Pos = self:GetScreenPos() - local Size = self:GetSize() + local Size = self:GetMouseBounds() - return IsInBox( Pos, Size, Mult, MaxX, MaxY ) + return IsInElementBox( Pos, ApplyMultiplier( Size, Mult ) ) end function ControlMeta:MouseInCached() @@ -1307,7 +1341,7 @@ do local CachedResult = self.__LastMouseInCheck if not CachedResult then CachedResult = TableNew( 5, 0 ) - self.__LastMouseInCheckFrame = CachedResult + self.__LastMouseInCheck = CachedResult end CachedResult[ 1 ] = In @@ -1417,6 +1451,10 @@ function ControlMeta:HandleEasing( Time, DeltaTime ) EasingHandler.Setter( self, Element, EasingData.CurValue, EasingData ) else EasingHandler.Setter( self, Element, EasingData.End, EasingData ) + if EasingHandler.OnComplete then + EasingHandler.OnComplete( self, Element, EasingData ) + end + Easings:Remove( Element ) if EasingData.Callback then @@ -1477,10 +1515,12 @@ local Easers = { end, Setter = function( self, Element, Pos ) Element:SetPosition( Pos ) - self:HandleHightlighting() end, Getter = function( self, Element ) return Element:GetPosition() + end, + OnComplete = function( self, Element, EasingData ) + self:InvalidateMouseState() end }, "Move" ), Size = Easer( { @@ -1493,6 +1533,11 @@ local Easers = { end, Getter = function( self, Element ) return Element:GetSize() + end, + OnComplete = function( self, Element, EasingData ) + if Element == self.Background then + self:InvalidateMouseState() + end end }, "Size" ), Scale = Easer( { @@ -1522,7 +1567,14 @@ function ControlMeta:StopEasing( Element, EasingHandler ) local Easers = self.EasingProcesses:Get( EasingHandler ) if not Easers then return end - Easers:Remove( Element or self.Background ) + Element = Element or self.Background + + local EasingData = Easers:Get( Element ) + if EasingData and EasingHandler.OnComplete then + EasingHandler.OnComplete( self, Element, EasingData ) + end + + Easers:Remove( Element ) end local function AddEaseFunc( EasingData, EaseFunc, Power ) @@ -1718,17 +1770,10 @@ do 1. Boolean should hightlight. 2. Muliplier to the element's size when determining if the mouse is in the element. ]] - function ControlMeta:SetHighlightOnMouseOver( Bool, Mult, TextureMode ) + function ControlMeta:SetHighlightOnMouseOver( HighlightOnMouseOver, TextureMode ) local WasHighlightOnMouseOver = self.HighlightOnMouseOver - self.HighlightOnMouseOver = not not Bool - self.HighlightMult = Mult - self.TextureHighlight = TextureMode - - if not Bool and not self.ForceHighlight then - self:SetHighlighted( false, true ) - self:StopFade( self.Background ) - end + self.HighlightOnMouseOver = not not HighlightOnMouseOver if not WasHighlightOnMouseOver and self.HighlightOnMouseOver then self.HandleHightlighting = HandleHightlighting @@ -1737,6 +1782,18 @@ do self.HandleHightlighting = NoOpHighlighting self:RemovePropertyChangeListener( "IsVisible", HandleHighlightOnVisibilityChange ) end + + if not HighlightOnMouseOver then + if not self.ForceHighlight then + self:SetHighlighted( false, true ) + self:StopFade( self.Background ) + end + + self.TextureHighlight = TextureMode + else + self.TextureHighlight = TextureMode + self:HandleHightlighting() + end end end @@ -1798,10 +1855,7 @@ do function ControlMeta:HandleHovering( Time ) if not self.OnHover then return end - local MouseIn, X, Y - if self:GetIsVisible() then - MouseIn, X, Y = self:MouseInCached() - end + local MouseIn = self:HasMouseEntered() and self:GetIsVisible() -- If the mouse is in this object, and our window is in focus (i.e. not obstructed by a higher window) -- then consider the object hovered. @@ -1811,6 +1865,8 @@ do else if Time - self.MouseHoverStart > ( self.HoverTime or DEFAULT_HOVER_TIME ) and not self.MouseHovered then self.MouseHovered = true + + local _, X, Y = self:MouseInCached() self:OnHover( X, Y ) end end @@ -1850,6 +1906,7 @@ function ControlMeta:Think( DeltaTime ) self:HandleEasing( Time, DeltaTime ) self:HandleHovering( Time ) self:HandleLayout( DeltaTime ) + self:HandleMouseState() end function ControlMeta:ThinkWithChildren( DeltaTime ) @@ -1951,7 +2008,7 @@ function ControlMeta:SetHighlighted( Highlighted, SkipAnim ) end function ControlMeta:ShouldHighlight() - return self:GetIsVisible() and self:MouseIn( self.Background, self.HighlightMult ) + return self:GetIsVisible() and self:MouseInCached() end function ControlMeta:SetForceHighlight( ForceHighlight, SkipAnim ) @@ -1995,11 +2052,75 @@ function ControlMeta:OnMouseWheel( Down ) if Result ~= nil then return true end end +function ControlMeta:HasMouseEntered() + return self.MouseHasEntered +end + +--[[ + Called when the mouse cursor has entered the control. + + The result of the MouseInControl method determines when this occurs. +]] +function ControlMeta:OnMouseEnter() + +end + +--[[ + Called when the mouse cursor has left the control. + + The result of the MouseInControl method determines when this occurs. +]] +function ControlMeta:OnMouseLeave() + +end + +function ControlMeta:InvalidateMouseState( Now ) + self.MouseStateIsInvalid = true + if Now then + self:HandleMouseState() + end +end + +function ControlMeta:HandleMouseState() + if not self.MouseStateIsInvalid or not SGUI.IsMouseVisible() then return end + + self:EvaluateMouseState() + self:CallOnChildren( "OnMouseMove", false ) +end + +function ControlMeta:EvaluateMouseState() + local IsMouseIn = self:MouseInCached() + local StateChanged = false + + if IsMouseIn and not self.MouseHasEntered then + StateChanged = true + + self.MouseHasEntered = true + self:HandleHightlighting() + self:OnMouseEnter() + elseif not IsMouseIn and self.MouseHasEntered then + -- Need to let children see the mouse exit themselves too. + StateChanged = true + + self.MouseHasEntered = false + self:HandleHightlighting() + self:OnMouseLeave() + end + + self.MouseStateIsInvalid = false + + return IsMouseIn, StateChanged +end + function ControlMeta:OnMouseMove( Down ) if not self:GetIsVisible() then return end - self:CallOnChildren( "OnMouseMove", Down ) - self:HandleHightlighting() + self.__LastMouseMove = SGUI.FrameNumber() + + local IsMouseIn, StateChanged = self:EvaluateMouseState() + if IsMouseIn or StateChanged then + self:CallOnChildren( "OnMouseMove", Down ) + end end --[[ diff --git a/lua/shine/lib/gui/objects/checkbox.lua b/lua/shine/lib/gui/objects/checkbox.lua index 8a9115dd7..77dca4153 100644 --- a/lua/shine/lib/gui/objects/checkbox.lua +++ b/lua/shine/lib/gui/objects/checkbox.lua @@ -10,6 +10,8 @@ local RoundTo = math.RoundTo local CheckBox = {} +SGUI.AddProperty( CheckBox, "LabelPadding", 10, { "InvalidatesLayout" } ) + function CheckBox:Initialise() self.BaseClass.Initialise( self ) @@ -53,7 +55,7 @@ function CheckBox:SetSize( Vec ) Vec.x = RoundTo( Vec.x, 2 ) Vec.y = RoundTo( Vec.y, 2 ) - self.Background:SetSize( Vec ) + self.BaseClass.SetSize( self, Vec ) local BoxSize = Vec * 0.75 BoxSize.x = RoundTo( BoxSize.x, 2 ) @@ -61,8 +63,6 @@ function CheckBox:SetSize( Vec ) self.Box:SetSize( BoxSize ) self.Box:SetPosition( -BoxSize * 0.5 ) - - self:InvalidateLayout() end function CheckBox:GetChecked() @@ -103,6 +103,15 @@ function CheckBox:SetChecked( Value, DontFade ) self:OnPropertyChanged( "Checked", false ) end +-- Include the attached label when checking for mouse entry. +function CheckBox:GetMouseBounds() + local Size = self:GetSize() + if SGUI.IsValid( self.Label ) then + Size = Size + Vector2( Size.x + self:GetLabelPadding() + self.Label:GetSize().x, 0 ) + end + return Size +end + function CheckBox:OnMouseDown( Key, DoubleClick ) if not self:GetIsVisible() then return end if Key ~= InputKey.MouseButton0 then return end @@ -127,7 +136,7 @@ end function CheckBox:PerformLayout() if self.Label then local Size = self:GetSize().x - self.Label:SetPos( Vector( Size + 10, 0, 0 ) ) + self.Label:SetPos( Vector( Size + self:GetLabelPadding(), 0, 0 ) ) end end @@ -144,7 +153,7 @@ function CheckBox:AddLabel( Text ) Label:SetAnchor( GUIItem.Left, GUIItem.Center ) Label:SetTextAlignmentY( GUIItem.Align_Center ) Label:SetText( Text ) - Label:SetPos( Vector( self:GetSize().x + 10, 0, 0 ) ) + Label:SetPos( Vector( self:GetSize().x + self:GetLabelPadding(), 0, 0 ) ) if self.Font then Label:SetFont( self.Font ) @@ -162,20 +171,9 @@ function CheckBox:AddLabel( Text ) Label.Label:SetInheritsParentStencilSettings( true ) end - if self.TooltipText then - Label:SetTooltip( self.TooltipText ) - end - self.Label = Label end -function CheckBox:SetTooltip( Tooltip ) - self.BaseClass.SetTooltip( self, Tooltip ) - if SGUI.IsValid( self.Label ) then - self.Label:SetTooltip( Tooltip ) - end -end - function CheckBox:OnChecked( Checked ) end diff --git a/lua/shine/lib/gui/objects/colour_label.lua b/lua/shine/lib/gui/objects/colour_label.lua index a0a2c06d3..dc5d199c6 100644 --- a/lua/shine/lib/gui/objects/colour_label.lua +++ b/lua/shine/lib/gui/objects/colour_label.lua @@ -95,6 +95,8 @@ function ColourLabel:SetText( TextContent ) Count = Count + 1 self.Labels[ Count ] = Label end + + self:InvalidateMouseState() end function ColourLabel:GetSize() diff --git a/lua/shine/lib/gui/objects/label.lua b/lua/shine/lib/gui/objects/label.lua index 4e76383ac..3f898cd38 100644 --- a/lua/shine/lib/gui/objects/label.lua +++ b/lua/shine/lib/gui/objects/label.lua @@ -26,6 +26,7 @@ do self.CachedTextWidth = nil self.CachedTextHeight = nil self.NeedsTextSizeRefresh = true + self:InvalidateMouseState() end local function SetupElementForFontName( self, Font ) @@ -86,8 +87,8 @@ do end end -function Label:MouseIn( Element, Mult, MaxX, MaxY ) - return self:MouseInControl( Mult, MaxX, MaxY ) +function Label:MouseIn( Element, Mult ) + return self:MouseInControl( Mult ) end local AlignmentMultipliers = { diff --git a/lua/shine/lib/gui/objects/list.lua b/lua/shine/lib/gui/objects/list.lua index c18cd87f5..6e3df029c 100644 --- a/lua/shine/lib/gui/objects/list.lua +++ b/lua/shine/lib/gui/objects/list.lua @@ -216,10 +216,9 @@ end Sets the list's size, just a simple vector input. ]] function List:SetSize( Size ) - self.Background:SetSize( Size ) + self.BaseClass.SetSize( self, Size ) self.CroppingBox:SetSize( Size ) self.Size = Size - self:InvalidateLayout() end function List:PerformLayout() @@ -721,7 +720,7 @@ function List:OnMouseMove( Down ) self.Scrollbar:OnMouseMove( Down ) end - self:CallOnChildren( "OnMouseMove", Down ) + self.BaseClass.OnMouseMove( self, Down ) end function List:Think( DeltaTime ) diff --git a/lua/shine/lib/gui/objects/panel.lua b/lua/shine/lib/gui/objects/panel.lua index dcc0b270b..d8a58b5d8 100644 --- a/lua/shine/lib/gui/objects/panel.lua +++ b/lua/shine/lib/gui/objects/panel.lua @@ -37,6 +37,7 @@ function Panel:Initialise() self.OverflowY = false self.HorizontalScrollingEnabled = true self.BlockEventsIfFocusedWindow = true + self.AlwaysInMouseFocus = false end function Panel:SkinColour() @@ -462,6 +463,10 @@ function Panel:SetMaxWidth( MaxWidth ) self.ScrollParentPos = self.ScrollParentPos or Vector2( 0, 0 ) + local function OnScrollChanged() + self:OnMouseMove( false ) + end + function self:OnScrollChangeX( Pos, MaxPos, Smoothed ) local SetWidth = self:GetSize().x @@ -471,9 +476,10 @@ function Panel:SetMaxWidth( MaxWidth ) self.ScrollParentPos.x = -Diff * Fraction if Smoothed and self.AllowSmoothScroll then - self:MoveTo( self.ScrollParent, nil, self.ScrollParentPos, 0, 0.2, nil, math.EaseOut, 3 ) + self:MoveTo( self.ScrollParent, nil, self.ScrollParentPos, 0, 0.2, OnScrollChanged, math.EaseOut, 3 ) else self.ScrollParent:SetPosition( self.ScrollParentPos ) + OnScrollChanged() end end @@ -543,6 +549,10 @@ function Panel:SetMaxHeight( MaxHeight, ForceInstantScroll ) self.ScrollParentPos = self.ScrollParentPos or Vector2( 0, 0 ) + local function OnScrollChanged() + self:OnMouseMove( false ) + end + function self:OnScrollChange( Pos, MaxPos, Smoothed ) local SetHeight = self:GetSize().y local MaxHeight = self.MaxHeight @@ -553,9 +563,10 @@ function Panel:SetMaxHeight( MaxHeight, ForceInstantScroll ) self.ScrollParentPos.y = -Diff * Fraction if Smoothed and self.AllowSmoothScroll then - self:MoveTo( self.ScrollParent, nil, self.ScrollParentPos, 0, 0.2, nil, math.EaseOut, 3 ) + self:MoveTo( self.ScrollParent, nil, self.ScrollParentPos, 0, 0.2, OnScrollChanged, math.EaseOut, 3 ) else self.ScrollParent:SetPosition( self.ScrollParentPos ) + OnScrollChanged() end end @@ -625,7 +636,9 @@ function Panel:DragClick( Key, DoubleClick ) if not self.Draggable then return end if Key ~= InputKey.MouseButton0 then return end - if not self:MouseIn( self.Background, nil, nil, self:GetSize().y * 0.05 ) then return end + + local Size = self:GetSize() + if not self:MouseInBounds( self.Background, Size.x, Size.y * 0.05 ) then return end if Clock() - LastInput < 0.2 then DoubleClick = true @@ -718,6 +731,8 @@ end function Panel:OnMouseMove( Down ) if not self:GetIsVisible() then return end + self.__LastMouseMove = SGUI.FrameNumber() + if SGUI.IsValid( self.Scrollbar ) then self.Scrollbar:OnMouseMove( Down ) end @@ -726,13 +741,14 @@ function Panel:OnMouseMove( Down ) self.HorizontalScrollbar:OnMouseMove( Down ) end - self:CallOnChildren( "OnMouseMove", Down ) + local MouseIn, StateChanged = self:EvaluateMouseState() + if MouseIn or StateChanged or self.AlwaysInMouseFocus or SGUI:IsWindow( self ) then + self:CallOnChildren( "OnMouseMove", Down ) + end + self:DragMove( Down ) - local MouseIn if self.AutoHideScrollbar and ( SGUI.IsValid( self.Scrollbar ) or SGUI.IsValid( self.HorizontalScrollbar ) ) then - MouseIn = self:MouseIn( self.Background ) - if not MouseIn and self.ScrollbarIsVisible then local ScrollbarHasFocus = self.Scrollbar and self.Scrollbar:HasMouseFocus() local HorizontalScrollbarHasFocus = self.HorizontalScrollbar and self.HorizontalScrollbar:HasMouseFocus() @@ -761,14 +777,17 @@ function Panel:OnMouseMove( Down ) -- Block mouse movement for lower windows. if ( SGUI:IsWindow( self ) and self.BlockEventsIfFocusedWindow ) or self.BlockOnMouseDown then - if MouseIn == nil then - MouseIn = self:MouseIn( self.Background ) - end if MouseIn then return true end end end +function Panel:OnGainWindowFocus() + self:InvalidateMouseState( true ) +end + function Panel:Think( DeltaTime ) + if not self:GetIsVisible() then return end + self.BaseClass.Think( self, DeltaTime ) if SGUI.IsValid( self.Scrollbar ) then diff --git a/lua/shine/lib/gui/objects/richtext.lua b/lua/shine/lib/gui/objects/richtext.lua index 93a811298..c16dcd59b 100644 --- a/lua/shine/lib/gui/objects/richtext.lua +++ b/lua/shine/lib/gui/objects/richtext.lua @@ -31,6 +31,7 @@ function RichText:SetFont( Font ) self.ComputedWrapping = false self:InvalidateLayout() + self:InvalidateMouseState() end function RichText:SetTextScale( Scale ) @@ -40,6 +41,7 @@ function RichText:SetTextScale( Scale ) self.ComputedWrapping = false self:InvalidateLayout() + self:InvalidateMouseState() end function RichText:PerformLayout() @@ -56,6 +58,7 @@ function RichText:SetSize( Size ) self.MaxWidth = MaxWidth self.ComputedWrapping = false self:InvalidateLayout() + self:InvalidateMouseState() end function RichText:GetComputedSize( Index, ParentSize ) @@ -120,12 +123,14 @@ function RichText:SetContent( Contents ) self.Lines = self:ParseContents( Contents ) self.ComputedWrapping = false self:InvalidateLayout() + self:InvalidateMouseState() end function RichText:RestoreFromLines( Lines ) self.Lines = Lines self.ComputedWrapping = false self:InvalidateLayout() + self:InvalidateMouseState() end function RichText:HasVisibleElements() diff --git a/lua/shine/lib/gui/objects/scrollbar.lua b/lua/shine/lib/gui/objects/scrollbar.lua index 4f263f7a8..fef19ba03 100644 --- a/lua/shine/lib/gui/objects/scrollbar.lua +++ b/lua/shine/lib/gui/objects/scrollbar.lua @@ -78,7 +78,7 @@ end function Scrollbar:SetSize( Size ) self.Size = Size - self.Background:SetSize( Size ) + self.BaseClass.SetSize( self, Size ) self:UpdateScrollBarSize() end diff --git a/lua/shine/lib/gui/objects/slider.lua b/lua/shine/lib/gui/objects/slider.lua index d99fd1e14..685151f65 100644 --- a/lua/shine/lib/gui/objects/slider.lua +++ b/lua/shine/lib/gui/objects/slider.lua @@ -382,7 +382,11 @@ function Slider:OnMouseUp( Key ) end function Slider:OnMouseMove( Down ) - self:CallOnChildren( "OnMouseMove", Down ) + self.BaseClass.OnMouseMove( self, Down ) + + if SGUI.IsValid( self.TextEntry ) then + self.TextEntry:OnMouseMove( Down ) + end if not Down then return end if not self.Dragging then return end diff --git a/lua/shine/lib/gui/objects/textentry.lua b/lua/shine/lib/gui/objects/textentry.lua index 8082f29c4..6cf548c6d 100644 --- a/lua/shine/lib/gui/objects/textentry.lua +++ b/lua/shine/lib/gui/objects/textentry.lua @@ -128,7 +128,7 @@ function TextEntry:GetContentSizeForAxis( Axis ) end function TextEntry:SetSize( SizeVec ) - self.Background:SetSize( SizeVec ) + self.BaseClass.SetSize( self, SizeVec ) local InnerBoxSize = SizeVec - self.BorderSize * 2 self.InnerBox:SetSize( InnerBoxSize ) @@ -786,6 +786,24 @@ function TextEntry:OnMouseUp() return true end +function TextEntry:OnMouseEnter() + self.BaseClass.OnMouseEnter( self ) + + if self.Enabled or self.Highlighted then return end + + self:FadeTo( self.InnerBox, self.DarkCol, self.FocusColour, 0, 0.1 ) + self.Highlighted = true +end + +function TextEntry:OnMouseLeave() + self.BaseClass.OnMouseLeave( self ) + + if self.Enabled or not self.Highlighted then return end + + self:FadeTo( self.InnerBox, self.FocusColour, self.DarkCol, 0, 0.1 ) + self.Highlighted = false +end + function TextEntry:OnMouseMove( Down ) if not self:GetIsVisible() then return end @@ -795,19 +813,7 @@ function TextEntry:OnMouseMove( Down ) return end - if not self:MouseIn( self.Background ) then - if not self.Enabled and self.Highlighted then - self:FadeTo( self.InnerBox, self.FocusColour, self.DarkCol, 0, 0.1 ) - self.Highlighted = false - end - - return - end - - if self.Highlighted or self.Enabled then return end - - self:FadeTo( self.InnerBox, self.DarkCol, self.FocusColour, 0, 0.1 ) - self.Highlighted = true + self.BaseClass.OnMouseMove( self, Down ) end function TextEntry:GetColumnFromMouse( X ) @@ -1181,8 +1187,12 @@ function TextEntry:OnFocusChange( NewFocus, ClickingOtherElement ) if self.Enabled then self.Enabled = false - self.Highlighted = false - self:FadeTo( self.InnerBox, self.FocusColour, self.DarkCol, 0, 0.1 ) + + if not self:HasMouseEntered() then + self.Highlighted = false + self:FadeTo( self.InnerBox, self.FocusColour, self.DarkCol, 0, 0.1 ) + end + self:RemoveStylingState( "Focus" ) end diff --git a/lua/shine/lib/gui/objects/tooltip.lua b/lua/shine/lib/gui/objects/tooltip.lua index 71445c724..2881541d8 100644 --- a/lua/shine/lib/gui/objects/tooltip.lua +++ b/lua/shine/lib/gui/objects/tooltip.lua @@ -23,11 +23,6 @@ function Tooltip:Initialise() self.TextPadding = 16 end -function Tooltip:SetSize( Vec ) - self.Size = Vec - self.Background:SetSize( Vec ) -end - function Tooltip:SetTextColour( Col ) self.TextCol = Col diff --git a/lua/shine/lib/gui/objects/webpage.lua b/lua/shine/lib/gui/objects/webpage.lua index bfd60126b..dfb8828f3 100644 --- a/lua/shine/lib/gui/objects/webpage.lua +++ b/lua/shine/lib/gui/objects/webpage.lua @@ -56,7 +56,7 @@ function Webpage:LoadURL( URL, W, H ) self.WebView = Client.CreateWebView( W, H ) self.WebView:SetTargetTexture( TextureName ) - self.Background:SetSize( Vector( W, H, 0 ) ) + self.BaseClass.SetSize( self, Vector2( W, H ) ) self.Background:SetTexture( TextureName ) self.WebView:HookJSAlert( function( WebView, AlertText ) From afbd8c26aacdb77ee51e32bad564490099ba30b3 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 11 Apr 2020 15:35:21 +0100 Subject: [PATCH 16/49] Improve handling of SGUI:Destroy() in events Automatically queue the destruction to run after the event has been called to avoid destroying GUIItems that may be used in later parts of the event. --- lua/shine/lib/gui.lua | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/lua/shine/lib/gui.lua b/lua/shine/lib/gui.lua index dc8385727..dedcbc2c0 100644 --- a/lua/shine/lib/gui.lua +++ b/lua/shine/lib/gui.lua @@ -477,22 +477,19 @@ end local OnError = Shine.BuildErrorHandler( "SGUI Error" ) +SGUI.PostEventActions = {} + function SGUI:PostCallEvent( Result, Control ) - local PostEventActions = self.PostEventActions - if not PostEventActions then return end + self.CallingEvent = nil + local PostEventActions = self.PostEventActions for i = 1, #PostEventActions do xpcall( PostEventActions[ i ], OnError, Result, Control ) + PostEventActions[ i ] = nil end - - self.PostEventActions = nil end function SGUI:AddPostEventAction( Action ) - if not self.PostEventActions then - self.PostEventActions = {} - end - self.PostEventActions[ #self.PostEventActions + 1 ] = Action end @@ -506,6 +503,8 @@ do local Windows = self.Windows local WindowCount = #Windows + self.CallingEvent = Name + -- The focused window is the last in the list, so we call backwards. for i = WindowCount, 1, -1 do local Window = Windows[ i ] @@ -1058,6 +1057,17 @@ do end local OnCallOnRemoveError = Shine.BuildErrorHandler( "SGUI CallOnRemove callback error" ) + local DestructionAction = Shine.TypeDef() + function DestructionAction:Init( Control ) + self.Control = Control + return self + end + + function DestructionAction:__call() + if SGUI.IsValid( self.Control ) then + self.Control:Destroy() + end + end --[[ Destroys an SGUI control. @@ -1067,6 +1077,13 @@ do Input: SGUI control object. ]] function SGUI:Destroy( Control ) + if self.CallingEvent then + -- Wait until after the running event to destroy the control. This avoids needing loads of validity checks + -- in event code paths. + self:AddPostEventAction( DestructionAction( Control ) ) + return + end + if Control.Parent then Control:SetParent( nil ) end From 4dadce6c6f6ed544e433f85b88b1fafa1eb122ff Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 11 Apr 2020 15:35:54 +0100 Subject: [PATCH 17/49] Use InvalidateMouseState in Panel scroll callback --- lua/shine/lib/gui/objects/panel.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lua/shine/lib/gui/objects/panel.lua b/lua/shine/lib/gui/objects/panel.lua index d8a58b5d8..6afcff66f 100644 --- a/lua/shine/lib/gui/objects/panel.lua +++ b/lua/shine/lib/gui/objects/panel.lua @@ -464,7 +464,7 @@ function Panel:SetMaxWidth( MaxWidth ) self.ScrollParentPos = self.ScrollParentPos or Vector2( 0, 0 ) local function OnScrollChanged() - self:OnMouseMove( false ) + self:InvalidateMouseState( true ) end function self:OnScrollChangeX( Pos, MaxPos, Smoothed ) @@ -550,7 +550,7 @@ function Panel:SetMaxHeight( MaxHeight, ForceInstantScroll ) self.ScrollParentPos = self.ScrollParentPos or Vector2( 0, 0 ) local function OnScrollChanged() - self:OnMouseMove( false ) + self:InvalidateMouseState( true ) end function self:OnScrollChange( Pos, MaxPos, Smoothed ) From f4619066e999ac3c9a78d575f23523aff7a7d15a Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sat, 11 Apr 2020 18:34:17 +0100 Subject: [PATCH 18/49] Fix more trace aborts in client code paths * Make easing callbacks pass the SGUI control as the first argument. This makes avoiding closures much easier. * Allow passing data with timers to avoid needing closures for callbacks. * Refactor various places that create functions at runtime to use either callable tables or pre-defined functions instead. * Remove some tail calls that fail to compile. --- lua/shine/extensions/chatbox/client.lua | 49 +++---- lua/shine/extensions/improvedchat/client.lua | 124 +++++++++++------- .../extensions/mapvote/ui/map_vote_menu.lua | 21 +-- .../mapvote/ui/map_vote_notification.lua | 19 +-- lua/shine/extensions/voterandom/client.lua | 10 +- lua/shine/lib/gui.lua | 8 +- lua/shine/lib/gui/base_control.lua | 16 ++- lua/shine/lib/gui/notification_manager.lua | 24 ++-- lua/shine/lib/gui/objects/chatline.lua | 32 +++-- lua/shine/lib/gui/objects/checkbox.lua | 8 +- lua/shine/lib/gui/objects/label.lua | 24 ++-- lua/shine/lib/gui/objects/notification.lua | 37 +++--- lua/shine/lib/gui/objects/panel.lua | 119 +++++++---------- lua/shine/lib/gui/objects/progressbar.lua | 2 +- lua/shine/lib/gui/objects/richtext.lua | 6 + lua/shine/lib/gui/objects/tooltip.lua | 18 ++- lua/shine/lib/gui/richtext/elements/base.lua | 15 ++- lua/shine/lib/timer.lua | 11 +- test/lib/gui/richtext/elements/base.lua | 56 ++++++++ 19 files changed, 359 insertions(+), 240 deletions(-) create mode 100644 test/lib/gui/richtext/elements/base.lua diff --git a/lua/shine/extensions/chatbox/client.lua b/lua/shine/extensions/chatbox/client.lua index 1bf39c607..09eeb12ed 100644 --- a/lua/shine/extensions/chatbox/client.lua +++ b/lua/shine/extensions/chatbox/client.lua @@ -1122,7 +1122,7 @@ function Plugin:OpenSettings( MainPanel, UIScale, ScalarScale ) Expanded = false end - SettingsPanel:SizeTo( SettingsPanel.Background, Start, End, 0, 0.25, function( Panel ) + SettingsPanel:SizeTo( SettingsPanel.Background, Start, End, 0, 0.25, function() SettingsButton.Expanded = Expanded if Expanded then @@ -1188,7 +1188,7 @@ function Plugin:OnResolutionChanged( OldX, OldY, NewX, NewY ) end end -function Plugin:AddMessageFromPopulator( Populator, ... ) +function Plugin:AddMessageFromPopulator( Populator, Context ) if not SGUI.IsValid( self.MainPanel ) then self:CreateChatbox() @@ -1222,7 +1222,7 @@ function Plugin:AddMessageFromPopulator( Populator, ... ) ChatLine:SetFontScale( Font, Scale ) ChatLine:SetLineSpacing( LineMargin ) - Populator( ChatLine, ... ) + Populator( ChatLine, Context ) self.ChatBox.Layout:AddElement( ChatLine ) @@ -1232,14 +1232,12 @@ function Plugin:AddMessageFromPopulator( Populator, ... ) end do - local function AddMessageFromRichText( ChatLine, Contents, ShowTimestamps ) - ChatLine:SetContent( Contents, ShowTimestamps ) + local function AddMessageFromRichText( ChatLine, Contents ) + ChatLine:SetContent( Contents, Plugin.Config.ShowTimestamps ) end function Plugin:AddMessageFromRichText( MessageData ) - return self:AddMessageFromPopulator( - AddMessageFromRichText, MessageData.Message, self.Config.ShowTimestamps - ) + return self:AddMessageFromPopulator( AddMessageFromRichText, MessageData.Message ) end end @@ -1255,9 +1253,16 @@ end do local IntToColour - - local function AddSimpleChatMessage( ChatLine, ... ) - ChatLine:SetMessage( ... ) + local BasicMessageContext = {} + local function AddSimpleChatMessage( ChatLine, Context ) + ChatLine:SetMessage( + Context.TagData, + Context.PlayerColour, + Context.PlayerName, + Context.MessageColour, + Context.MessageText, + Plugin.Config.ShowTimestamps + ) end --[[ @@ -1267,11 +1272,11 @@ do Messages with multiple colours can be added through the chat API. ]] - function Plugin:AddMessage( PlayerColour, PlayerName, MessageColour, MessageName, TagData ) + function Plugin:AddMessage( PlayerColour, PlayerName, MessageColour, MessageText, TagData ) -- Don't add anything if one of the elements is the wrong type. Default chat will error instead. if not ( IsType( PlayerColour, "number" ) or IsType( PlayerColour, "cdata" ) ) or not IsType( PlayerName, "string" ) or not IsType( MessageColour, "cdata" ) - or not IsType( MessageName, "string" ) then + or not IsType( MessageText, "string" ) then return end @@ -1282,10 +1287,13 @@ do PlayerColour = IntToColour( PlayerColour ) end - self:AddMessageFromPopulator( - AddSimpleChatMessage, TagData, PlayerColour, PlayerName, MessageColour, MessageName, - self.Config.ShowTimestamps - ) + BasicMessageContext.PlayerColour = PlayerColour + BasicMessageContext.PlayerName = PlayerName + BasicMessageContext.MessageColour = MessageColour + BasicMessageContext.MessageText = MessageText + BasicMessageContext.TagData = TagData + + self:AddMessageFromPopulator( AddSimpleChatMessage, BasicMessageContext ) end end @@ -1415,12 +1423,7 @@ do local LabelRow = Elements[ i ] if not Results[ i ] or ( i > 1 and Results.ParameterIndex and not ResultsAreForCorrectArgument ) then if LabelRow then - FadeOutTransition.Callback = function() - if not LabelRow then return end - - LabelRow:Destroy() - LabelRow = nil - end + FadeOutTransition.Callback = LabelRow.Destroy LabelRow:ApplyTransition( FadeOutTransition ) end else diff --git a/lua/shine/extensions/improvedchat/client.lua b/lua/shine/extensions/improvedchat/client.lua index 426e9d7a7..2af70bec3 100644 --- a/lua/shine/extensions/improvedchat/client.lua +++ b/lua/shine/extensions/improvedchat/client.lua @@ -156,45 +156,59 @@ Hook.CallAfterFileLoad( "lua/GUIChat.lua", function() ChatLine:Reset() end - local function MakeFadeOutCallback( self, ChatLine, PaddingAmount ) - return function() - if not TableRemoveByValue( self.ChatLines, ChatLine ) then - return - end + local FadeOutCallback = Shine.TypeDef() + function FadeOutCallback:Init( GUIChat, ChatLine, PaddingAmount ) + self.GUIChat = GUIChat + self.ChatLine = ChatLine + self.PaddingAmount = PaddingAmount + return self + end - ResetChatLine( ChatLine ) + function FadeOutCallback:__call() + local ChatLine = self.ChatLine + local GUIChat = self.GUIChat + local PaddingAmount = self.PaddingAmount - if Plugin.Config.MessageDisplayType == Plugin.MessageDisplayType.DOWNWARDS then - -- Move remaining messages upwards to fill in the gap. - local YOffset = 0 - local ShouldAnimate = IsAnimationEnabled( self ) - - for i = 1, #self.ChatLines do - local ChatLine = self.ChatLines[ i ] - local Pos = ChatLine:GetPos() - Pos.y = YOffset - - if ShouldAnimate then - ChatLine:ApplyTransition( { - Type = "Move", - EndValue = Pos, - Duration = AnimDuration, - EasingFunction = MovementEase - } ) - else - ChatLine:SetPos( Pos ) - end + if not TableRemoveByValue( GUIChat.ChatLines, ChatLine ) then + return + end - YOffset = YOffset + ChatLine:GetSize().y + PaddingAmount + ResetChatLine( ChatLine ) + + if Plugin.Config.MessageDisplayType == Plugin.MessageDisplayType.DOWNWARDS then + -- Move remaining messages upwards to fill in the gap. + local YOffset = 0 + local ShouldAnimate = IsAnimationEnabled( GUIChat ) + + for i = 1, #GUIChat.ChatLines do + local ChatLine = GUIChat.ChatLines[ i ] + local Pos = ChatLine:GetPos() + Pos.y = YOffset + + if ShouldAnimate then + ChatLine:ApplyTransition( { + Type = "Move", + EndValue = Pos, + Duration = AnimDuration, + EasingFunction = MovementEase + } ) + else + ChatLine:SetPos( Pos ) end - else - -- Update local message positions and re-position the container panel downward to account for the - -- lost message. - UpdateUpwardsMessagePositions( self, PaddingAmount ) - end - self.ChatLinePool[ #self.ChatLinePool + 1 ] = ChatLine + YOffset = YOffset + ChatLine:GetSize().y + PaddingAmount + end + else + -- Update local message positions and re-position the container panel downward to account for the + -- lost message. + UpdateUpwardsMessagePositions( GUIChat, PaddingAmount ) end + + GUIChat.ChatLinePool[ #GUIChat.ChatLinePool + 1 ] = ChatLine + end + + local function MakeFadeOutCallback( self, ChatLine, PaddingAmount ) + return FadeOutCallback( self, ChatLine, PaddingAmount ) end local function RemoveLineIfOffScreen( Line, Index, self ) @@ -221,6 +235,15 @@ Hook.CallAfterFileLoad( "lua/GUIChat.lua", function() end end + local RemoveOffscreenLinesCallback = Shine.TypeDef() + function RemoveOffscreenLinesCallback:Init( GUIChat ) + self.GUIChat = GUIChat + return self + end + function RemoveOffscreenLinesCallback:__call() + RemoveOffscreenLines( self.GUIChat ) + end + local function AddChatLineMovingUpwards( self, ChatLine, PaddingAmount, ShouldAnimate ) local NewLineHeight = ChatLine:GetSize().y local YOffset = 0 @@ -244,7 +267,7 @@ Hook.CallAfterFileLoad( "lua/GUIChat.lua", function() EndValue = MessagePanelPos, Duration = AnimDuration, EasingFunction = MovementEase, - Callback = function() RemoveOffscreenLines( self ) end + Callback = RemoveOffscreenLinesCallback( self ) } ) else self.MessagePanel:SetPos( MessagePanelPos ) @@ -289,7 +312,7 @@ Hook.CallAfterFileLoad( "lua/GUIChat.lua", function() local BackgroundTexture = PrecacheAsset "ui/shine/chat_bg.dds" - function ChatElement:AddChatLine( Populator, ... ) + function ChatElement:AddChatLine( Populator, Context ) local ChatLine = TableRemove( self.ChatLinePool ) or SGUI:Create( "ChatLine", self.MessagePanel ) self.ChatLines[ #self.ChatLines + 1 ] = ChatLine @@ -302,7 +325,7 @@ Hook.CallAfterFileLoad( "lua/GUIChat.lua", function() ChatLine:SetTextScale( Scale ) ChatLine:SetLineSpacing( LineMargin ) - Populator( ChatLine, ... ) + Populator( ChatLine, Context ) if not ChatLine:HasVisibleElements() then -- Avoid displaying empty messages. @@ -569,12 +592,14 @@ function Plugin:ReceiveResetChatTag( Message ) self.ChatTags[ Message.SteamID ] = nil end -local function PopulateFromBasicMessage( ChatLine, PlayerColour, PlayerName, MessageColour, MessageText, TagData ) +local BasicMessageContext = {} +local function PopulateFromBasicMessage( ChatLine, Context ) + local PlayerColour = Context.PlayerColour if IsType( PlayerColour, "number" ) then PlayerColour = IntToColour( PlayerColour ) end - ChatLine:SetMessage( TagData, PlayerColour, PlayerName, MessageColour, MessageText ) + ChatLine:SetMessage( Context.TagData, PlayerColour, Context.PlayerName, Context.MessageColour, Context.MessageText ) end function Plugin:SetupGUIChat( ChatElement ) @@ -633,14 +658,13 @@ function Plugin:SetupGUIChat( ChatElement ) MessageText = StringFormat( "%s\n%s", MessageText, Message.Message2:GetText() ) end - ChatElement:AddChatLine( - PopulateFromBasicMessage, - Message.Player:GetColor(), - Message.Player:GetText(), - Message.Message:GetColor(), - MessageText, - TagData - ) + BasicMessageContext.PlayerColour = Message.Player:GetColor() + BasicMessageContext.PlayerName = Message.Player:GetText() + BasicMessageContext.MessageColour = Message.Message:GetColor() + BasicMessageContext.MessageText = MessageText + BasicMessageContext.TagData = TagData + + ChatElement:AddChatLine( PopulateFromBasicMessage, BasicMessageContext ) end end @@ -676,7 +700,13 @@ function Plugin:OnChatAddMessage( GUIChat, PlayerColour, PlayerName, MessageColo } end - GUIChat:AddChatLine( PopulateFromBasicMessage, PlayerColour, PlayerName, MessageColour, MessageText, TagData ) + BasicMessageContext.PlayerColour = PlayerColour + BasicMessageContext.PlayerName = PlayerName + BasicMessageContext.MessageColour = MessageColour + BasicMessageContext.MessageText = MessageText + BasicMessageContext.TagData = TagData + + GUIChat:AddChatLine( PopulateFromBasicMessage, BasicMessageContext ) Hook.Call( "OnChatMessageDisplayed", PlayerColour, PlayerName, MessageColour, MessageText, TagData ) diff --git a/lua/shine/extensions/mapvote/ui/map_vote_menu.lua b/lua/shine/extensions/mapvote/ui/map_vote_menu.lua index 7dd812c29..f12e637d6 100644 --- a/lua/shine/extensions/mapvote/ui/map_vote_menu.lua +++ b/lua/shine/extensions/mapvote/ui/map_vote_menu.lua @@ -487,22 +487,25 @@ function MapVoteMenu:FadeIn() end end +local function OnFadeOutComplete( self ) + self:SetIsVisible( false ) + self.FadingOut = false + self:OnClose() + if self.FadeOutCallback then + -- Call after Think exits to avoid destroying GUIItems that are in use. + SGUI:AddPostEventAction( self.FadeOutCallback ) + end +end + function MapVoteMenu:FadeOut( Callback ) self.FadingOut = true + self.FadeOutCallback = Callback self:ApplyTransition( { Type = "Alpha", EndValue = 0, Duration = 0.3, - Callback = function() - self:SetIsVisible( false ) - self.FadingOut = false - self:OnClose() - if Callback then - -- Call after Think exits to avoid destroying GUIItems that are in use. - SGUI:AddPostEventAction( Callback ) - end - end + Callback = OnFadeOutComplete } ) end diff --git a/lua/shine/extensions/mapvote/ui/map_vote_notification.lua b/lua/shine/extensions/mapvote/ui/map_vote_notification.lua index 1eb80b40b..dd27ee896 100644 --- a/lua/shine/extensions/mapvote/ui/map_vote_notification.lua +++ b/lua/shine/extensions/mapvote/ui/map_vote_notification.lua @@ -283,21 +283,24 @@ function MapVoteNotification:FadeIn() self:UpdateTeamVariation() end +local function OnFadeOutComplete( self ) + self:SetIsVisible( false ) + self.FadingOut = false + if self.FadeOutCallback then + -- Call after Think exits to avoid destroying GUIItems that are in use. + SGUI:AddPostEventAction( self.FadeOutCallback ) + end +end + function MapVoteNotification:FadeOut( Callback ) self.FadingOut = true + self.FadeOutCallback = Callback self:ApplyTransition( { Type = "Alpha", EndValue = 0, Duration = 0.3, - Callback = function() - self:SetIsVisible( false ) - self.FadingOut = false - if Callback then - -- Call after Think exits to avoid destroying GUIItems that are in use. - SGUI:AddPostEventAction( Callback ) - end - end + Callback = OnFadeOutComplete } ) end diff --git a/lua/shine/extensions/voterandom/client.lua b/lua/shine/extensions/voterandom/client.lua index c137d2f1d..2b24e6c94 100644 --- a/lua/shine/extensions/voterandom/client.lua +++ b/lua/shine/extensions/voterandom/client.lua @@ -465,11 +465,11 @@ local IsPlayingTeam = Shine.IsPlayingTeam local pairs = pairs function Plugin:UpdateTeamMemoryEntry( ClientIndex, TeamNumber, CurTime ) - local MemoryEntry = self.TeamTracking[ ClientIndex ] + local MemoryEntry = self.TeamTracking:Get( ClientIndex ) if not MemoryEntry then -- Start with the team they're currently on to avoid everyone flashing on first join. MemoryEntry = { TeamNumber = TeamNumber } - self.TeamTracking[ ClientIndex ] = MemoryEntry + self.TeamTracking:Add( ClientIndex, MemoryEntry ) end -- For some reason, spectators are constantly swapped between team 0 and 3. @@ -485,7 +485,7 @@ function Plugin:UpdateTeamMemoryEntry( ClientIndex, TeamNumber, CurTime ) end function Plugin:Initialise() - self.TeamTracking = {} + self.TeamTracking = Shine.Map() self.FriendGroup = {} self.InFriendGroup = false @@ -508,9 +508,9 @@ function Plugin:Initialise() self:UpdateTeamMemoryEntry( ClientIndex, Entry.EntityTeamNumber, CurTime ) end - for ClientIndex in pairs( self.TeamTracking ) do + for ClientIndex in self.TeamTracking:Iterate() do if not Clients[ ClientIndex ] then - self.TeamTracking[ ClientIndex ] = nil + self.TeamTracking:Remove( ClientIndex ) end end end ) diff --git a/lua/shine/lib/gui.lua b/lua/shine/lib/gui.lua index dedcbc2c0..8b7e58bb6 100644 --- a/lua/shine/lib/gui.lua +++ b/lua/shine/lib/gui.lua @@ -32,8 +32,9 @@ do local Vector = Vector -- A little easier than having to always include that 0 z value. + -- The return is wrapped to avoid a tail-call which doesn't compile. function Vector2( X, Y ) - return Vector( X, Y, 0 ) + return ( Vector( X, Y, 0 ) ) end end @@ -1189,12 +1190,13 @@ SGUI.NotifyFocusChange = NotifyFocusChange local GetCursorPosScreen = Client.GetCursorPosScreen function SGUI.GetCursorPos() - return GetCursorPosScreen() + local X, Y = GetCursorPosScreen() + return X, Y end local GetMouseVisible = Client.GetMouseVisible function SGUI.IsMouseVisible() - return GetMouseVisible() + return ( GetMouseVisible() ) end local ScrW = Client.GetScreenWidth diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index cebf737a5..cdefd6024 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -1458,7 +1458,7 @@ function ControlMeta:HandleEasing( Time, DeltaTime ) Easings:Remove( Element ) if EasingData.Callback then - EasingData.Callback( Element ) + EasingData.Callback( self, Element ) end end end @@ -1961,12 +1961,16 @@ function ControlMeta:ShowTooltip( MouseX, MouseY ) self.Tooltip = Tooltip end -function ControlMeta:HideTooltip() - if not SGUI.IsValid( self.Tooltip ) then return end - - self.Tooltip:FadeOut( function() +do + local function OnTooltipHidden( self ) self.Tooltip = nil - end ) + end + + function ControlMeta:HideTooltip() + if not SGUI.IsValid( self.Tooltip ) then return end + + self.Tooltip:FadeOut( OnTooltipHidden, self ) + end end function ControlMeta:SetHighlighted( Highlighted, SkipAnim ) diff --git a/lua/shine/lib/gui/notification_manager.lua b/lua/shine/lib/gui/notification_manager.lua index b11cc40c7..dbd175e31 100644 --- a/lua/shine/lib/gui/notification_manager.lua +++ b/lua/shine/lib/gui/notification_manager.lua @@ -48,6 +48,18 @@ local MARGIN = HighResScaled( 16 ) local PADDING = HighResScaled( 16 ) local FLAIR_WIDTH = HighResScaled( 48 ) +local function OnNotificationFadeOut( Notification ) + for i = #Notifications, 1, -1 do + if Notifications[ i ] == Notification then + Notification:StopMoving() + TableRemove( Notifications, i ) + -- Move any notifications above this one down to compensate for the gap. + OffsetAllNotifications( -Notification:GetSize().y - MARGIN:GetValue(), 1, i - 1 ) + break + end + end +end + --[[ Adds a notification to the screen. @@ -85,17 +97,7 @@ function NotificationManager.AddNotification( Type, Message, Duration, Options ) Notification.TargetPos = TargetPos Notification:MoveTo( nil, nil, TargetPos, 0, 0.3 ) Notification:FadeIn() - Notification:FadeOutAfter( Duration, function() - for i = #Notifications, 1, -1 do - if Notifications[ i ] == Notification then - Notification:StopMoving() - TableRemove( Notifications, i ) - -- Move any notifications above this one down to compensate for the gap. - OffsetAllNotifications( -Notification:GetSize().y - MARGIN:GetValue(), 1, i - 1 ) - break - end - end - end ) + Notification:FadeOutAfter( Duration, OnNotificationFadeOut ) -- Move all existing notifications up by the notification's size + margin. OffsetAllNotifications( Notification:GetSize().y + MARGIN:GetValue(), 1, #Notifications ) diff --git a/lua/shine/lib/gui/objects/chatline.lua b/lua/shine/lib/gui/objects/chatline.lua index 6b5fccf6e..30d8281a1 100644 --- a/lua/shine/lib/gui/objects/chatline.lua +++ b/lua/shine/lib/gui/objects/chatline.lua @@ -158,21 +158,33 @@ function ChatLine:MakeVisible() end end +local function OnFadeOutDelayPassed( Timer ) + local Data = Timer.Data + + local ChatLineInstance = Data.ChatLine + if not SGUI.IsValid( ChatLineInstance ) then return end + + ChatLineInstance.FadeOutTimer = nil + + if not ChatLineInstance.Parent:GetIsVisible() then + -- Skip fading if currently invisible. + return Data.OnComplete() + end + + ChatLineInstance:FadeOut( Data.Duration, Data.OnComplete, Data.Easer ) +end + function ChatLine:FadeOutIn( Delay, Duration, OnComplete, Easer ) if self.FadeOutTimer then self.FadeOutTimer:Destroy() end - self.FadeOutTimer = Shine.Timer.Simple( Delay, function() - self.FadeOutTimer = nil - - if not self.Parent:GetIsVisible() then - -- Skip fading if currently invisible. - return OnComplete() - end - - self:FadeOut( Duration, OnComplete, Easer ) - end ) + self.FadeOutTimer = Shine.Timer.Simple( Delay, OnFadeOutDelayPassed, { + ChatLine = self, + Duration = Duration, + OnComplete = OnComplete, + Easer = Easer + } ) end function ChatLine:FadeOut( Duration, OnComplete, Easer ) diff --git a/lua/shine/lib/gui/objects/checkbox.lua b/lua/shine/lib/gui/objects/checkbox.lua index 77dca4153..8df600d38 100644 --- a/lua/shine/lib/gui/objects/checkbox.lua +++ b/lua/shine/lib/gui/objects/checkbox.lua @@ -78,9 +78,7 @@ function CheckBox:SetChecked( Value, DontFade ) if DontFade then self.Box:SetColor( self.BoxCol ) else - self:FadeTo( self.Box, self.BoxHideCol, self.BoxCol, 0, 0.1, function( Box ) - Box:SetColor( self.BoxCol ) - end ) + self:FadeTo( self.Box, self.BoxHideCol, self.BoxCol, 0, 0.1 ) end self:OnChecked( true ) @@ -94,9 +92,7 @@ function CheckBox:SetChecked( Value, DontFade ) if DontFade then self.Box:SetColor( self.BoxHideCol ) else - self:FadeTo( self.Box, self.BoxCol, self.BoxHideCol, 0, 0.1, function( Box ) - Box:SetColor( self.BoxHideCol ) - end ) + self:FadeTo( self.Box, self.BoxCol, self.BoxHideCol, 0, 0.1 ) end self:OnChecked( false ) diff --git a/lua/shine/lib/gui/objects/label.lua b/lua/shine/lib/gui/objects/label.lua index 3f898cd38..6321409d0 100644 --- a/lua/shine/lib/gui/objects/label.lua +++ b/lua/shine/lib/gui/objects/label.lua @@ -115,24 +115,26 @@ function Label:SetupStencil() self.Label:SetStencilFunc( GUIItem.NotEqual ) end +local WordWrapDummy = { + GetTextWidth = function( self, Text ) + -- Need to account for scale here. + return self.Element:GetTextWidth( Text ) + end, + SetText = function( self, Text ) + self.Element.Label:SetText( Text ) + self.Element:EvaluateOptionFlags( Text ) + end +} + -- Apply word wrapping before the height is computed (assuming height = Units.Auto()). function Label:PreComputeHeight( Width ) if not self.AutoWrap then return end local CurrentText = self.Label:GetText() + -- Pass in a dummy to avoid mutating the actual text value assigned to this label, -- and instead only update the displayed text on the GUIItem. - local WordWrapDummy = { - GetTextWidth = function( _, Text ) - -- Need to account for scale here. - return self:GetTextWidth( Text ) - end, - SetText = function( _, Text ) - self.Label:SetText( Text ) - self:EvaluateOptionFlags( Text ) - end - } - + WordWrapDummy.Element = self SGUI.WordWrap( WordWrapDummy, self.Text, 0, Width ) if CurrentText ~= self.Label:GetText() then diff --git a/lua/shine/lib/gui/objects/notification.lua b/lua/shine/lib/gui/objects/notification.lua index c98f4831f..766c8584b 100644 --- a/lua/shine/lib/gui/objects/notification.lua +++ b/lua/shine/lib/gui/objects/notification.lua @@ -216,11 +216,20 @@ function Notification:Think( DeltaTime ) end end +local function OnFadeInComplete( self ) + self.FadingIn = false +end + function Notification:FadeIn() self.FadingIn = true - self:AlphaTo( self.Background, 0, self:GetColour().a, 0, 0.3, function() - self.FadingIn = false - end ) + self:AlphaTo( self.Background, 0, self:GetColour().a, 0, 0.3, OnFadeInComplete ) +end + +local function OnFadeOutComplete( self ) + if self.FadeOutCallback then + self.FadeOutCallback( self ) + end + self:Destroy() end function Notification:FadeOut() @@ -232,22 +241,20 @@ function Notification:FadeOut() end self.FadingOut = true - self:AlphaTo( self.Background, self.Background:GetColor().a, 0, 0, 0.3, function() - if self.FadeOutCallback then - self.FadeOutCallback( self ) - end - self:Destroy() - end ) + self:AlphaTo( self.Background, self.Background:GetColor().a, 0, 0, 0.3, OnFadeOutComplete ) +end + +local function OnFadeOutStart( Timer ) + local NotificationInstance = Timer.Data + if not SGUI.IsValid( NotificationInstance ) then return end + + NotificationInstance.FadeOutTimer = nil + NotificationInstance:FadeOut() end function Notification:FadeOutAfter( Duration, Callback ) self.FadeOutCallback = Callback - self.FadeOutTimer = Shine.Timer.Simple( Duration, function() - if not SGUI.IsValid( self ) then return end - - self.FadeOutTimer = nil - self:FadeOut() - end ) + self.FadeOutTimer = Shine.Timer.Simple( Duration, OnFadeOutStart, self ) end SGUI:Register( "Notification", Notification ) diff --git a/lua/shine/lib/gui/objects/panel.lua b/lua/shine/lib/gui/objects/panel.lua index 6afcff66f..21408e140 100644 --- a/lua/shine/lib/gui/objects/panel.lua +++ b/lua/shine/lib/gui/objects/panel.lua @@ -15,7 +15,7 @@ local Panel = {} Panel.IsWindow = true local DefaultBuffer = 20 -local ScrollPos = Vector( -20, 10, 0 ) +local ScrollPos = Vector( -10, 0, 0 ) local ZeroColour = Colour( 0, 0, 0, 0 ) SGUI.AddProperty( Panel, "AutoHideScrollbar" ) @@ -221,6 +221,22 @@ function Panel:RecomputeMaxHeight() self:SetMaxHeight( MaxHeight ) end +local function UpdateMaxSize( Child ) + local Parent = Child.Parent + if not Parent.ScrollParent or not Child:GetIsVisible() then return end + + local Size = Parent:GetSize() + local NewMaxWidth = ComputeMaxWidth( Child, Size.x ) + if NewMaxWidth > Parent:GetMaxWidth() then + Parent:SetMaxWidth( NewMaxWidth ) + end + + local NewMaxHeight = ComputeMaxHeight( Child, Size.y ) + if NewMaxHeight > Parent:GetMaxHeight() then + Parent:SetMaxHeight( NewMaxHeight + Parent.BufferAmount ) + end +end + function Panel:Add( Class, Created ) local Element = Created or SGUI:Create( Class, self, self.ScrollParent ) Element:SetParent( self, self.ScrollParent ) @@ -230,32 +246,8 @@ function Panel:Add( Class, Created ) Element:SetStencilled( true ) end - local function UpdateMaxSize( Child ) - if not self.ScrollParent or not Child:GetIsVisible() then return end - - local Size = self:GetSize() - local NewMaxWidth = ComputeMaxWidth( Child, Size.x ) - if NewMaxWidth > self:GetMaxWidth() then - self:SetMaxWidth( NewMaxWidth ) - end - - local NewMaxHeight = ComputeMaxHeight( Child, Size.y ) - if NewMaxHeight > self:GetMaxHeight() then - self:SetMaxHeight( NewMaxHeight + self.BufferAmount ) - end - end - - local OldSetPos = Element.SetPos - function Element:SetPos( Pos ) - OldSetPos( self, Pos ) - UpdateMaxSize( self ) - end - - local OldSetSize = Element.SetSize - function Element:SetSize( OurSize ) - OldSetSize( self, OurSize ) - UpdateMaxSize( self ) - end + Element:AddPropertyChangeListener( "Pos", UpdateMaxSize ) + Element:AddPropertyChangeListener( "Size", UpdateMaxSize ) return Element end @@ -424,6 +416,24 @@ function Panel:OnRemoveScrollbar() self.Layout:SetMargin( nil ) end +local function OnScrollChanged( self ) + self:InvalidateMouseState( true ) +end + +function Panel:OnScrollChangeX( Pos, MaxPos, Smoothed ) + local Fraction = MaxPos == 0 and 0 or Pos / MaxPos + local Diff = self.MaxWidth - self:GetSize().x + + self.ScrollParentPos.x = -Diff * Fraction + + if Smoothed and self.AllowSmoothScroll then + self:MoveTo( self.ScrollParent, nil, self.ScrollParentPos, 0, 0.2, OnScrollChanged, math.EaseOut, 3 ) + else + self.ScrollParent:SetPosition( self.ScrollParentPos ) + OnScrollChanged( self ) + end +end + function Panel:SetMaxWidth( MaxWidth ) self.MaxWidth = MaxWidth @@ -463,26 +473,6 @@ function Panel:SetMaxWidth( MaxWidth ) self.ScrollParentPos = self.ScrollParentPos or Vector2( 0, 0 ) - local function OnScrollChanged() - self:InvalidateMouseState( true ) - end - - function self:OnScrollChangeX( Pos, MaxPos, Smoothed ) - local SetWidth = self:GetSize().x - - local Fraction = MaxPos == 0 and 0 or Pos / MaxPos - local Diff = self.MaxWidth - SetWidth - - self.ScrollParentPos.x = -Diff * Fraction - - if Smoothed and self.AllowSmoothScroll then - self:MoveTo( self.ScrollParent, nil, self.ScrollParentPos, 0, 0.2, OnScrollChanged, math.EaseOut, 3 ) - else - self.ScrollParent:SetPosition( self.ScrollParentPos ) - OnScrollChanged() - end - end - Scrollbar._CallEventsManually = true if self.HideHorizontalScrollbar then @@ -507,6 +497,20 @@ function Panel:SetMaxWidth( MaxWidth ) self.HorizontalScrollbar:SetScrollSize( ElementWidth / MaxWidth ) end +function Panel:OnScrollChange( Pos, MaxPos, Smoothed ) + local Fraction = MaxPos == 0 and 0 or Pos / MaxPos + local Diff = self.MaxHeight - self:GetSize().y + + self.ScrollParentPos.y = -Diff * Fraction + + if Smoothed and self.AllowSmoothScroll then + self:MoveTo( self.ScrollParent, nil, self.ScrollParentPos, 0, 0.2, OnScrollChanged, math.EaseOut, 3 ) + else + self.ScrollParent:SetPosition( self.ScrollParentPos ) + OnScrollChanged( self ) + end +end + function Panel:SetMaxHeight( MaxHeight, ForceInstantScroll ) local OldMaxHeight = self.MaxHeight @@ -549,27 +553,6 @@ function Panel:SetMaxHeight( MaxHeight, ForceInstantScroll ) self.ScrollParentPos = self.ScrollParentPos or Vector2( 0, 0 ) - local function OnScrollChanged() - self:InvalidateMouseState( true ) - end - - function self:OnScrollChange( Pos, MaxPos, Smoothed ) - local SetHeight = self:GetSize().y - local MaxHeight = self.MaxHeight - - local Fraction = MaxPos == 0 and 0 or Pos / MaxPos - local Diff = MaxHeight - SetHeight - - self.ScrollParentPos.y = -Diff * Fraction - - if Smoothed and self.AllowSmoothScroll then - self:MoveTo( self.ScrollParent, nil, self.ScrollParentPos, 0, 0.2, OnScrollChanged, math.EaseOut, 3 ) - else - self.ScrollParent:SetPosition( self.ScrollParentPos ) - OnScrollChanged() - end - end - Scrollbar._CallEventsManually = true if self.StickyScroll then diff --git a/lua/shine/lib/gui/objects/progressbar.lua b/lua/shine/lib/gui/objects/progressbar.lua index f0cc02ba3..e4b79e1fc 100644 --- a/lua/shine/lib/gui/objects/progressbar.lua +++ b/lua/shine/lib/gui/objects/progressbar.lua @@ -49,7 +49,7 @@ function ProgressBar:SetBorderSize( BorderSize ) end function ProgressBar:SetSize( Size ) - self.Background:SetSize( Size ) + self.BaseClass.SetSize( self, Size ) local BoxSize = Size - self.BorderSize * 2 RefreshInnerSizes( self, BoxSize ) diff --git a/lua/shine/lib/gui/objects/richtext.lua b/lua/shine/lib/gui/objects/richtext.lua index c16dcd59b..29b2e2e94 100644 --- a/lua/shine/lib/gui/objects/richtext.lua +++ b/lua/shine/lib/gui/objects/richtext.lua @@ -151,8 +151,14 @@ function RichText:PerformWrapping() TextScale = self.TextScale } ) + local OldW, OldH = self.WrappedWidth, self.WrappedHeight + self:ApplyLines( WrappedLines ) self.ComputedWrapping = true + + if OldW ~= self.WrappedWidth or OldH ~= self.WrappedHeight then + self:OnPropertyChanged( "Size", Vector2( self.WrappedWidth, self.WrappedHeight ) ) + end end local function MakeElementFromPool( self, Class ) diff --git a/lua/shine/lib/gui/objects/tooltip.lua b/lua/shine/lib/gui/objects/tooltip.lua index 2881541d8..8ace0a2e4 100644 --- a/lua/shine/lib/gui/objects/tooltip.lua +++ b/lua/shine/lib/gui/objects/tooltip.lua @@ -101,20 +101,24 @@ function Tooltip:FadeIn() self:FadeTo( self.Background, Start, End, 0, 0.2 ) end -function Tooltip:FadeOut( Callback ) +local function OnFadeOutComplete( self ) + if self.FadeOutCallback then + self.FadeOutCallback( self.FadeOutCallbackContext ) + end + self:Destroy() +end + +function Tooltip:FadeOut( Callback, Context ) if self.FadingOut then return end self.FadingOut = true + self.FadeOutCallback = Callback + self.FadeOutCallbackContext = Context local Start = self.Background:GetColor() local End = SGUI.ColourWithAlpha( Start, 0 ) - self:FadeTo( self.Background, Start, End, 0, 0.2, function() - if Callback then - Callback() - end - self:Destroy() - end ) + self:FadeTo( self.Background, Start, End, 0, 0.2, OnFadeOutComplete ) end function Tooltip:OnLoseWindowFocus() diff --git a/lua/shine/lib/gui/richtext/elements/base.lua b/lua/shine/lib/gui/richtext/elements/base.lua index a71e29b05..c3083ce5c 100644 --- a/lua/shine/lib/gui/richtext/elements/base.lua +++ b/lua/shine/lib/gui/richtext/elements/base.lua @@ -9,17 +9,22 @@ function Base:IsVisibleElement() return false end +local function ThinkWithExtra( self, DeltaTime ) + self:__ExtraThink( DeltaTime ) + return self:__OldThink( DeltaTime ) +end + function Base.AddThinkFunction( Element, ExtraThink ) -- Remove any old override (so Think comes from the metatable). Element.Think = nil + Element.__ExtraThink = nil + Element.__OldThink = nil if not ExtraThink then return end - local OldThink = Element.Think - function Element:Think( DeltaTime ) - ExtraThink( self, DeltaTime ) - return OldThink( self, DeltaTime ) - end + Element.__OldThink = Element.Think + Element.__ExtraThink = ExtraThink + Element.Think = ThinkWithExtra end function Base:Setup( Element ) diff --git a/lua/shine/lib/timer.lua b/lua/shine/lib/timer.lua index 2565c09bf..017f75490 100644 --- a/lua/shine/lib/timer.lua +++ b/lua/shine/lib/timer.lua @@ -77,10 +77,10 @@ do --[[ Creates a timer. - Inputs: Name, delay in seconds, number of times to repeat, function to run. + Inputs: Name, delay in seconds, number of times to repeat, function to run, optional data to attach. Pass a negative number to reps to have it repeat indefinitely. ]] - local function Create( Name, Delay, Reps, Func ) + local function Create( Name, Delay, Reps, Func, Data ) -- Edit it so it's not destroyed if it's created again inside its old function. local TimerObject = Timers:Get( Name ) or PausedTimers[ Name ] if not TimerObject then @@ -94,6 +94,7 @@ do TimerObject.Func = Func TimerObject.LastRun = 0 TimerObject.NextRun = SharedTime() + Delay + TimerObject.Data = Data return TimerObject end @@ -103,14 +104,14 @@ do --[[ Creates a simple timer. - Inputs: Delay in seconds, function to run. + Inputs: Delay in seconds, function to run, optional data to attach. Unlike a standard timer, this will only run once. ]] - function Timer.Simple( Delay, Func ) + function Timer.Simple( Delay, Func, Data ) local Index = "Simple"..SimpleCount SimpleCount = SimpleCount + 1 - return Create( Index, Delay, 1, Func ) + return Create( Index, Delay, 1, Func, Data ) end end diff --git a/test/lib/gui/richtext/elements/base.lua b/test/lib/gui/richtext/elements/base.lua new file mode 100644 index 000000000..2b6774f46 --- /dev/null +++ b/test/lib/gui/richtext/elements/base.lua @@ -0,0 +1,56 @@ +--[[ + Tests for base rich text element. +]] + +local UnitTest = Shine.UnitTest +local Base = require "shine/lib/gui/richtext/elements/base" + +UnitTest:Test( "AddThinkFunction - Adds the given callback", function( Assert ) + local MetaTable = { + __index = { + Think = UnitTest.MockFunction() + } + } + local Element = setmetatable( {}, MetaTable ) + + local Think = UnitTest.MockFunction() + Base.AddThinkFunction( Element, Think ) + + Assert.IsType( "Should have set a function value to Think", Element.Think, "function" ) + + Element:Think( 0 ) + + Assert.DeepEquals( "Should have invoked the added think function", { + { + ArgCount = 2, + Element, + 0 + } + }, Think.Invocations ) + Assert.DeepEquals( "Should have invoked the original think function", { + { + ArgCount = 2, + Element, + 0 + } + }, MetaTable.__index.Think.Invocations ) +end ) + +UnitTest:Test( "AddThinkFunction - Removes any existing override if passed nil", function( Assert ) + local MetaTable = { + __index = { + Think = function() end + } + } + local Element = setmetatable( { + Think = function() end, + __ExtraThink = UnitTest.MockFunction(), + __OldThink = UnitTest.MockFunction() + }, MetaTable ) + + Base.AddThinkFunction( Element, nil ) + + Assert.Nil( "Should have removed the extra think field", Element.__ExtraThink ) + Assert.Nil( "Should have removed the old think field", Element.__OldThink ) + Assert.Equals( "Should have restored the original Think function", MetaTable.__index.Think, Element.Think ) +end ) From 8e9b70bbeeebe4d532f12ae6599c869f643011a0 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 11:03:05 +0100 Subject: [PATCH 19/49] Use MouseTracker to get cursor position This avoids using Client.GetCursorPosScreen which doens't compile. --- lua/shine/lib/gui.lua | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lua/shine/lib/gui.lua b/lua/shine/lib/gui.lua index 8b7e58bb6..5eb60992d 100644 --- a/lua/shine/lib/gui.lua +++ b/lua/shine/lib/gui.lua @@ -1188,15 +1188,15 @@ local function NotifyFocusChange( Element, ClickingOtherElement ) end SGUI.NotifyFocusChange = NotifyFocusChange -local GetCursorPosScreen = Client.GetCursorPosScreen +local GetCursorPos = MouseTracker_GetCursorPos function SGUI.GetCursorPos() - local X, Y = GetCursorPosScreen() - return X, Y + local Pos = GetCursorPos() + return Pos.x, Pos.y end -local GetMouseVisible = Client.GetMouseVisible +local GetMouseVisible = MouseTracker_GetIsVisible function SGUI.IsMouseVisible() - return ( GetMouseVisible() ) + return GetMouseVisible() end local ScrW = Client.GetScreenWidth @@ -1255,8 +1255,6 @@ local IsMainMenuOpen = MainMenu_GetIsOpened If we don't load after everything, things aren't registered properly. ]] Hook.Add( "OnMapLoad", "LoadGUIElements", function() - GetCursorPosScreen = Client.GetCursorPosScreen - GetMouseVisible = Client.GetMouseVisible ScrW = Client.GetScreenWidth ScrH = Client.GetScreenHeight IsMainMenuOpen = MainMenu_GetIsOpened @@ -1270,6 +1268,9 @@ Hook.Add( "OnMapLoad", "LoadGUIElements", function() end ) Hook.CallAfterFileLoad( "lua/menu/MouseTracker.lua", function() + GetCursorPos = MouseTracker_GetCursorPos + GetMouseVisible = MouseTracker_GetIsVisible + local Listener = { OnMouseMove = function( _, LMB ) SGUI:CallEvent( false, "OnMouseMove", LMB ) From 01bfdf1ce78a62ee000ffa63fd676f5aad1bf64a Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 11:06:11 +0100 Subject: [PATCH 20/49] Fix highlighting state not updating properly --- lua/shine/lib/gui/base_control.lua | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index cdefd6024..8349dc498 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -2100,17 +2100,16 @@ function ControlMeta:EvaluateMouseState() StateChanged = true self.MouseHasEntered = true - self:HandleHightlighting() self:OnMouseEnter() elseif not IsMouseIn and self.MouseHasEntered then -- Need to let children see the mouse exit themselves too. StateChanged = true self.MouseHasEntered = false - self:HandleHightlighting() self:OnMouseLeave() end + self:HandleHightlighting() self.MouseStateIsInvalid = false return IsMouseIn, StateChanged From 273086bc20749e8dd902dd18ef8d98f13a175e49 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 11:08:43 +0100 Subject: [PATCH 21/49] Remove controls from their parent inside events When destroying inside an event, detach the control to avoid it showing up in iterations. --- lua/shine/lib/gui.lua | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lua/shine/lib/gui.lua b/lua/shine/lib/gui.lua index 5eb60992d..e02232175 100644 --- a/lua/shine/lib/gui.lua +++ b/lua/shine/lib/gui.lua @@ -1078,13 +1078,8 @@ do Input: SGUI control object. ]] function SGUI:Destroy( Control ) - if self.CallingEvent then - -- Wait until after the running event to destroy the control. This avoids needing loads of validity checks - -- in event code paths. - self:AddPostEventAction( DestructionAction( Control ) ) - return - end - + -- Remove the control from its parent immediately regardless of the running event. This avoids it showing + -- up in child iterations. if Control.Parent then Control:SetParent( nil ) end @@ -1093,6 +1088,13 @@ do Control.LayoutParent:RemoveElement( Control ) end + if self.CallingEvent then + -- Wait until after the running event to destroy the control. This avoids needing loads of validity checks + -- in event code paths. + self:AddPostEventAction( DestructionAction( Control ) ) + return + end + self.ActiveControls:Remove( Control ) self.KeyboardFocusControls:Remove( Control ) From dcceaaa688011011a4771696e336dec7079ea76f Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 11:09:43 +0100 Subject: [PATCH 22/49] Defer max height/width changes for panel children --- lua/shine/lib/gui/objects/panel.lua | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/lua/shine/lib/gui/objects/panel.lua b/lua/shine/lib/gui/objects/panel.lua index 21408e140..c0da60405 100644 --- a/lua/shine/lib/gui/objects/panel.lua +++ b/lua/shine/lib/gui/objects/panel.lua @@ -225,16 +225,7 @@ local function UpdateMaxSize( Child ) local Parent = Child.Parent if not Parent.ScrollParent or not Child:GetIsVisible() then return end - local Size = Parent:GetSize() - local NewMaxWidth = ComputeMaxWidth( Child, Size.x ) - if NewMaxWidth > Parent:GetMaxWidth() then - Parent:SetMaxWidth( NewMaxWidth ) - end - - local NewMaxHeight = ComputeMaxHeight( Child, Size.y ) - if NewMaxHeight > Parent:GetMaxHeight() then - Parent:SetMaxHeight( NewMaxHeight + Parent.BufferAmount ) - end + Parent:InvalidateLayout() end function Panel:Add( Class, Created ) From b46491741f502cd752fc2825e5e344d3473797cf Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 11:51:06 +0100 Subject: [PATCH 23/49] Populate SGUI setter name lookup at startup Use it for calls to AddProperty/AddBoundProperty to pre-populate it with most setter names. --- lua/shine/lib/gui.lua | 16 ++++++++++++++-- lua/shine/lib/gui/base_control.lua | 11 +---------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lua/shine/lib/gui.lua b/lua/shine/lib/gui.lua index e02232175..e5475a8f9 100644 --- a/lua/shine/lib/gui.lua +++ b/lua/shine/lib/gui.lua @@ -87,6 +87,18 @@ SGUI.PropertyModifiers = { end } do + -- This exists to avoid constant concatenation every time properties are set dynamically. + local SetterKeys = setmetatable( require "table.new"( 0, 100 ), { + __index = function( self, Key ) + local Setter = "Set"..Key + + self[ Key ] = Setter + + return Setter + end + } ) + SGUI.SetterKeys = SetterKeys + local function GetModifiers( Modifiers ) local RealModifiers = {} @@ -101,7 +113,7 @@ do Adds Get and Set functions for a property name, with an optional default value. ]] function SGUI.AddProperty( Table, Name, Default, Modifiers ) - local TableSetter = "Set"..Name + local TableSetter = SetterKeys[ Name ] local TableGetter = "Get"..Name Table[ TableSetter ] = function( self, Value ) @@ -181,7 +193,7 @@ do return self[ Name ] end - local TableSetter = "Set"..Name + local TableSetter = SetterKeys[ Name ] Table[ TableSetter ] = function( self, Value ) local OldValue = self[ Name ] diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index 8349dc498..0616ee78f 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -24,16 +24,7 @@ local Map = Shine.Map local Multimap = Shine.Multimap local Source = require "shine/lib/gui/binding/source" --- This exists to avoid constant concatenation every time properties are set dynamically. -local SetterKeys = setmetatable( TableNew( 0, 100 ), { - __index = function( self, Key ) - local Setter = "Set"..Key - - self[ Key ] = Setter - - return Setter - end -} ) +local SetterKeys = SGUI.SetterKeys SGUI.AddBoundProperty( ControlMeta, "BlendTechnique", "Background" ) SGUI.AddBoundProperty( ControlMeta, "InheritsParentAlpha", "Background" ) From 58d2c8a3a8c99a7bb64122fdaeb7f6608f7928bd Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 12:40:16 +0100 Subject: [PATCH 24/49] Make binding source ignore first call argument This avoids needing a closure when creating binding sources. --- lua/shine/lib/gui/base_control.lua | 4 +--- lua/shine/lib/gui/binding/source.lua | 2 +- test/lib/gui/binding/source.lua | 5 +++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index 0616ee78f..a8e7cb228 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -162,9 +162,7 @@ function ControlMeta:GetPropertySource( Name ) SourceInstance.Element = self self.PropertySources[ Name ] = SourceInstance - self:AddPropertyChangeListener( Name, function( self, Value ) - return SourceInstance( Value ) - end ) + self:AddPropertyChangeListener( Name, SourceInstance ) return SourceInstance end diff --git a/lua/shine/lib/gui/binding/source.lua b/lua/shine/lib/gui/binding/source.lua index 199fa84d6..68fa36200 100644 --- a/lua/shine/lib/gui/binding/source.lua +++ b/lua/shine/lib/gui/binding/source.lua @@ -29,7 +29,7 @@ end -- Called when a change occurs, internally tracks whether a value has changed -- and then calls downstream listeners if it has. -function Source:__call( Value ) +function Source:__call( _, Value ) if self.Value == Value then return end -- Update the value before calling listeners to allow them to iterate over diff --git a/test/lib/gui/binding/source.lua b/test/lib/gui/binding/source.lua index 9eab9e619..c2ce596bc 100644 --- a/test/lib/gui/binding/source.lua +++ b/test/lib/gui/binding/source.lua @@ -11,15 +11,16 @@ UnitTest:Test( "It should add/remove/call listeners as expected", function( Asse end ) local Listener2 = UnitTest.MockFunction() + local Host = {} local SourceInstance = Source() SourceInstance:AddListener( Listener1 ) SourceInstance:AddListener( Listener2 ) - SourceInstance( "test" ) + SourceInstance( Host, "test" ) SourceInstance:RemoveListener( Listener2 ) - SourceInstance( "test2" ) + SourceInstance( Host, "test2" ) Assert.DeepEquals( "Should have called the first listener twice", { { From 3703f6dbf4d6a9c935529aedeaeadd054bc2c20c Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 14:48:24 +0100 Subject: [PATCH 25/49] Use code generation for SGUI property generators This generates more optimal code that avoids loops and upvalues. --- lua/shine/lib/codegen.lua | 40 ++++- lua/shine/lib/gui.lua | 225 ++++++++++++++++---------- lua/shine/lib/gui/objects/tooltip.lua | 1 - 3 files changed, 176 insertions(+), 90 deletions(-) diff --git a/lua/shine/lib/codegen.lua b/lua/shine/lib/codegen.lua index ef260c0c8..c7330c830 100644 --- a/lua/shine/lib/codegen.lua +++ b/lua/shine/lib/codegen.lua @@ -13,6 +13,39 @@ local unpack = unpack local CodeGen = {} +--[[ + Applies template values to the given string. + Inputs: + 1. The function code to be used as a template. This should be a valid Lua string with placeholders expressed + as {Placeholder}. Each placeholder will have the corresponding value in the template values substituted. + 2. The template values to replace placeholders with. + Output: + The generated string with all variables substituted. +]] +local function ApplyTemplateValues( FunctionCode, TemplateValues ) + return ( StringGSub( FunctionCode, "{([^%s]+)}", TemplateValues ) ) +end +CodeGen.ApplyTemplateValues = ApplyTemplateValues + +--[[ + Generates a function from a template, where template values are replaced with the values in the given template + values table. + + Inputs: + 1. The function code to be used as a template. This should be a valid Lua string with placeholders expressed + as {Placeholder}. Each placeholder will have the corresponding value in the template values substituted. + 2. The source to give the loaded function (shown in error tracebacks). + 3. The template values to replace placeholders with. + ... Any values to pass to the chunk (e.g. to provide upvalues). + Output: + The generated function. +]] +local function GenerateTemplatedFunction( FunctionCode, ChunkName, TemplateValues, ... ) + local GeneratedFunctionCode = ApplyTemplateValues( FunctionCode, TemplateValues ) + return load( GeneratedFunctionCode, ChunkName )( ... ) +end +CodeGen.GenerateTemplatedFunction = GenerateTemplatedFunction + --[[ Generates a function from a template with the given number of arguments. @@ -49,12 +82,11 @@ local function GenerateFunctionWithArguments( FunctionCode, NumArguments, ChunkN local ArgumentsList = TableConcat( Arguments, ", " ) local ArgumentsWithoutPrefix = TableConcat( Arguments, ", ", 2 ) - local GeneratedFunctionCode = StringGSub( FunctionCode, "{([^}]+)}", { + + return GenerateTemplatedFunction( FunctionCode, ChunkName, { Arguments = ArgumentsList, FunctionArguments = ArgumentsWithoutPrefix - } ) - - return load( GeneratedFunctionCode, ChunkName )( ... ) + }, ... ) end CodeGen.GenerateFunctionWithArguments = GenerateFunctionWithArguments diff --git a/lua/shine/lib/gui.lua b/lua/shine/lib/gui.lua index e5475a8f9..c54d215ec 100644 --- a/lua/shine/lib/gui.lua +++ b/lua/shine/lib/gui.lua @@ -6,6 +6,8 @@ Shine.GUI = Shine.GUI or {} +local CodeGen = require "shine/lib/codegen" + local SGUI = Shine.GUI local Hook = Shine.Hook local IsType = Shine.IsType @@ -67,26 +69,18 @@ local ControlMeta = {} SGUI.BaseControl = ControlMeta SGUI.PropertyModifiers = { - InvalidatesLayout = function( self ) - self:InvalidateLayout() - end, - InvalidatesLayoutNow = function( self ) - self:InvalidateLayout( true ) - end, - InvalidatesParent = function( self ) - self:InvalidateParent() - end, - InvalidatesParentNow = function( self ) - self:InvalidateParent( true ) - end, - InvalidatesMouseState = function( self ) - self:InvalidateMouseState() - end, - InvalidatesMouseStateNow = function( self ) - self:InvalidateMouseState( true ) - end + InvalidatesLayout = [[self:InvalidateLayout()]], + InvalidatesLayoutNow = [[self:InvalidateLayout( true )]], + InvalidatesParent = [[self:InvalidateParent()]], + InvalidatesParentNow = [[self:InvalidateParent( true )]], + InvalidatesMouseState = [[self:InvalidateMouseState()]], + InvalidatesMouseStateNow = [[self:InvalidateMouseState( true )]] } do + local DebugGetInfo = debug.getinfo + local StringMatch = string.match + local TableConcat = table.concat + -- This exists to avoid constant concatenation every time properties are set dynamically. local SetterKeys = setmetatable( require "table.new"( 0, 100 ), { __index = function( self, Key ) @@ -106,50 +100,79 @@ do RealModifiers[ #RealModifiers + 1 ] = SGUI.PropertyModifiers[ Modifiers[ i ] ] end - return RealModifiers, #RealModifiers + return TableConcat( RealModifiers, "\n" ) end - --[[ - Adds Get and Set functions for a property name, with an optional default value. - ]] - function SGUI.AddProperty( Table, Name, Default, Modifiers ) - local TableSetter = SetterKeys[ Name ] - local TableGetter = "Get"..Name + local SetterTemplate = [[return function( self, Value ) + local OldValue = self:Get{Name}() + + self.{Name} = Value - Table[ TableSetter ] = function( self, Value ) - local OldValue = self[ TableGetter ]( self ) + if OldValue == Value then return false end - self[ Name ] = Value + self:OnPropertyChanged( "{Name}", Value ) + {Modifiers} - if OldValue == Value then return false end + return true + end]] - self:OnPropertyChanged( Name, Value ) + local GetterWithoutDefaultTemplate = [[return function( self ) + return self.{Name} + end]] - return true + local GetterWithDefaultTemplate = [[local Default = ... + return function( self ) + local Value = self.{Name} + if Value == nil then + Value = Default end + return Value + end]] - Table[ TableGetter ] = function( self ) - local Value = self[ Name ] - if Value == nil then - Value = Default - end - return Value - end + local BaseSource = "@lua/shine/lib/gui.lua" + local function GetCallerName() + local Caller = DebugGetInfo( 4, "S" ).source + return Caller and StringMatch( Caller, "/([^/]+)%.lua$" ) or "?" + end + + local function GetSetterSource( Name ) + return StringFormat( "%s/Property/%s/Set%s", BaseSource, GetCallerName(), Name ) + end + local function GetGetterSource( Name ) + return StringFormat( "%s/Property/%s/Get%s", BaseSource, GetCallerName(), Name ) + end - if not Modifiers then return end + --[[ + Adds Get and Set functions for a property name, with an optional default value. + ]] + function SGUI.AddProperty( Table, Name, Default, Modifiers ) + local TableSetter = SetterKeys[ Name ] + local TableGetter = "Get"..Name - local RealModifiers, NumModifiers = GetModifiers( Modifiers ) + local ModifierLines = Modifiers and GetModifiers( Modifiers ) or "" + local SetterSource = GetSetterSource( Name ) + Table[ TableSetter ] = CodeGen.GenerateTemplatedFunction( SetterTemplate, SetterSource, { + Name = Name, + Modifiers = ModifierLines + } ) - local Old = Table[ TableSetter ] - Table[ TableSetter ] = function( self, Value ) - if Old( self, Value ) then - for i = 1, NumModifiers do - RealModifiers[ i ]( self, Value ) - end - end + local GetterSource = GetGetterSource( Name ) + if Default ~= nil then + Table[ TableGetter ] = CodeGen.GenerateTemplatedFunction( GetterWithDefaultTemplate, GetterSource, { + Name = Name + }, Default ) + else + Table[ TableGetter ] = CodeGen.GenerateTemplatedFunction( GetterWithoutDefaultTemplate, GetterSource, { + Name = Name + } ) end end + local BoundCallTemplate = [[do + local Object = self.{FieldName} + if Object then Object:{Setter}( Value ) end + end]] + local StringExplode = string.Explode local unpack = unpack @@ -159,6 +182,7 @@ do end local BoundFields = {} + local Callbacks = {} for i = 1, #BoundObject do local Entry = BoundObject[ i ] @@ -166,61 +190,93 @@ do local FieldName, Setter = unpack( StringExplode( Entry, ":", true ) ) Setter = Setter or "Set"..PropertyName - BoundFields[ i ] = function( self, Value ) - local Object = self[ FieldName ] - if Object then - Object[ Setter ]( Object, Value ) - end - end + BoundFields[ #BoundFields + 1 ] = CodeGen.ApplyTemplateValues( BoundCallTemplate, { + Setter = Setter, + FieldName = FieldName + } ) else - BoundFields[ i ] = Entry + Callbacks[ #Callbacks + 1 ] = Entry end end - return BoundFields + return TableConcat( BoundFields, "\n" ), Callbacks end - --[[ - Adds Get/Set property methods that pass through the value to a field - on the table as well as storing it. + local BoundWithoutCallbacksTemplate = [[return function( self, Value ) + local OldValue = self:Get{Name}() - Used to perform actions on GUIItems without boilerplate code. - ]] - function SGUI.AddBoundProperty( Table, Name, BoundObject, Modifiers ) - local BoundFields = GetBindingInfo( BoundObject, Name ) + self.{Name} = Value - Table[ "Get"..Name ] = function( self ) - return self[ Name ] - end + {BoundFields} - local TableSetter = SetterKeys[ Name ] - Table[ TableSetter ] = function( self, Value ) - local OldValue = self[ Name ] + if OldValue == Value then return false end - self[ Name ] = Value + self:OnPropertyChanged( "{Name}", Value ) + {Modifiers} - for i = 1, #BoundFields do - BoundFields[ i ]( self, Value ) - end + return true + end]] - if OldValue == Value then return false end + local CallbacksCallTemplate = [[Callback%d( self, Value )]] - self:OnPropertyChanged( Name, Value ) + local BoundWithCallbacksTemplate = [[local {CallbackVariables} = ... + return function( self, Value ) + local OldValue = self:Get{Name}() - return true - end + self.{Name} = Value - if not Modifiers then return end + {BoundFields} - local RealModifiers, NumModifiers = GetModifiers( Modifiers ) + {Callbacks} - local Old = Table[ TableSetter ] - Table[ TableSetter ] = function( self, Value ) - if Old( self, Value ) then - for i = 1, NumModifiers do - RealModifiers[ i ]( self, Value ) - end + if OldValue == Value then return false end + + self:OnPropertyChanged( "{Name}", Value ) + {Modifiers} + + return true + end]] + + --[[ + Adds Get/Set property methods that pass through the value to a field + on the table as well as storing it. + + Used to perform actions on GUIItems without boilerplate code. + ]] + function SGUI.AddBoundProperty( Table, Name, BoundObject, Modifiers ) + local BoundFields, Callbacks = GetBindingInfo( BoundObject, Name ) + + local GetterSource = GetGetterSource( Name ) + Table[ "Get"..Name ] = CodeGen.GenerateTemplatedFunction( GetterWithoutDefaultTemplate, GetterSource, { + Name = Name + } ) + + local SetterSource = GetSetterSource( Name ) + local TableSetter = SetterKeys[ Name ] + local ModifierLines = Modifiers and GetModifiers( Modifiers ) or "" + + if #Callbacks > 0 then + -- Unroll the loop upfront. + local CallbackVariables = {} + local CallbackLines = {} + for i = 1, #Callbacks do + CallbackVariables[ i ] = StringFormat( "Callback%d", i ) + CallbackLines[ i ] = StringFormat( CallbacksCallTemplate, i ) end + + Table[ TableSetter ] = CodeGen.GenerateTemplatedFunction( BoundWithCallbacksTemplate, SetterSource, { + Name = Name, + BoundFields = BoundFields, + Modifiers = ModifierLines, + CallbackVariables = TableConcat( CallbackVariables, ", " ), + Callbacks = TableConcat( CallbackLines, "\n" ) + }, unpack( Callbacks ) ) + else + Table[ TableSetter ] = CodeGen.GenerateTemplatedFunction( BoundWithoutCallbacksTemplate, SetterSource, { + Name = Name, + BoundFields = BoundFields, + Modifiers = ModifierLines + } ) end end end @@ -507,7 +563,6 @@ function SGUI:AddPostEventAction( Action ) end do - local CodeGen = require "shine/lib/codegen" local select = select local Callers = CodeGen.MakeFunctionGenerator( { diff --git a/lua/shine/lib/gui/objects/tooltip.lua b/lua/shine/lib/gui/objects/tooltip.lua index 8ace0a2e4..7e4b78dca 100644 --- a/lua/shine/lib/gui/objects/tooltip.lua +++ b/lua/shine/lib/gui/objects/tooltip.lua @@ -13,7 +13,6 @@ Tooltip.IgnoreMouseFocus = true SGUI.AddBoundProperty( Tooltip, "Colour", "Background:SetColor" ) SGUI.AddBoundProperty( Tooltip, "Texture", "Background:SetTexture" ) -SGUI.AddBoundProperty( Tooltip, "TexturePixelCoordinates", "Background:SetTexturePixelCoordinates" ) function Tooltip:Initialise() self.BaseClass.Initialise( self ) From d9b520f75c6abde9194c1b4eafc97c605d45b259 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 14:56:19 +0100 Subject: [PATCH 26/49] Extract inline callbacks from dropdown/radio --- lua/shine/lib/gui/objects/dropdown.lua | 66 +++++++++++++------------ lua/shine/lib/gui/objects/radio.lua | 67 ++++++++++++++------------ 2 files changed, 70 insertions(+), 63 deletions(-) diff --git a/lua/shine/lib/gui/objects/dropdown.lua b/lua/shine/lib/gui/objects/dropdown.lua index 6838d9ec4..f79751bf5 100644 --- a/lua/shine/lib/gui/objects/dropdown.lua +++ b/lua/shine/lib/gui/objects/dropdown.lua @@ -27,38 +27,7 @@ function Dropdown:Initialise() self.Options = {} - self:SetOpenMenuOnClick( function( self ) - return { - MenuPos = self.MenuPos.BOTTOM, - Populate = function( Menu ) - Menu:SetMaxVisibleButtons( Max( self:GetMaxVisibleOptions(), 1 ) ) - Menu:SetFontScale( self:GetFont(), self:GetTextScale() ) - - for i = 1, #self.Options do - local Option = self.Options[ i ] - - local function DoClick( Button ) - local Result = true - if Option.DoClick then - Result = Option.DoClick( Button ) - end - - self:SetSelectedOption( Option ) - - Menu:Destroy() - - return Result - end - - local Button = Menu:AddButton( Option.Text, DoClick, Option.Tooltip ) - if Option.Icon then - Button:SetIcon( Option.Icon, Option.IconFont, Option.IconScale ) - end - Button:SetStyleName( "DropdownButton" ) - end - end - } - end ) + self:SetOpenMenuOnClick( self.BuildMenu ) Binder():FromElement( self, "SelectedOption" ) :ToElement( self, "Text", { @@ -88,6 +57,39 @@ function Dropdown:Initialise() :BindProperty() end +function Dropdown:BuildMenu() + return { + MenuPos = self.MenuPos.BOTTOM, + Populate = function( Menu ) + Menu:SetMaxVisibleButtons( Max( self:GetMaxVisibleOptions(), 1 ) ) + Menu:SetFontScale( self:GetFont(), self:GetTextScale() ) + + for i = 1, #self.Options do + local Option = self.Options[ i ] + + local function DoClick( Button ) + local Result = true + if Option.DoClick then + Result = Option.DoClick( Button ) + end + + self:SetSelectedOption( Option ) + + Menu:Destroy() + + return Result + end + + local Button = Menu:AddButton( Option.Text, DoClick, Option.Tooltip ) + if Option.Icon then + Button:SetIcon( Option.Icon, Option.IconFont, Option.IconScale ) + end + Button:SetStyleName( "DropdownButton" ) + end + end + } +end + function Dropdown:SelectOption( Value ) for i = 1, #self.Options do local Option = self.Options[ i ] diff --git a/lua/shine/lib/gui/objects/radio.lua b/lua/shine/lib/gui/objects/radio.lua index 77098a379..e0d78b414 100644 --- a/lua/shine/lib/gui/objects/radio.lua +++ b/lua/shine/lib/gui/objects/radio.lua @@ -73,55 +73,60 @@ function Radio:SetOptions( Options ) end end -function Radio:AddOption( Option ) - TypeCheck( Option, "table", 1, "AddOption" ) - TypeCheckField( Option, "Description", "string", "Option" ) - - local Index = #self.CheckBoxes + 1 - - local CheckBox = SGUI:Create( "CheckBox", self ) - CheckBox:SetFontScale( self.Font, self.TextScale ) - CheckBox:AddLabel( Option.Description ) - CheckBox:SetRadio( not self.MultipleChoice ) - CheckBox:SetAutoSize( self.CheckBoxAutoSize ) - CheckBox:SetStyleName( self.CheckBoxStyleName ) - CheckBox:SetTooltip( Option.Tooltip ) - CheckBox.RadioOption = Option - - if Index > 1 then - CheckBox:SetMargin( self.CheckBoxMargin ) - end - - CheckBox:AddPropertyChangeListener( "Checked", function( CheckBox, IsChecked ) - if not self.MultipleChoice then +do + local function OnCheckBoxStateChanged( CheckBox, IsChecked ) + local Parent = CheckBox.Parent + if not Parent.MultipleChoice then if not IsChecked then return end - for i = 1, #self.CheckBoxes do - local Box = self.CheckBoxes[ i ] + for i = 1, #Parent.CheckBoxes do + local Box = Parent.CheckBoxes[ i ] if Box ~= CheckBox then Box:SetChecked( false ) end end - self:OnPropertyChanged( "SelectedOption", Option ) + Parent:OnPropertyChanged( "SelectedOption", CheckBox.RadioOption ) else local Options = {} - for i = 1, #self.CheckBoxes do - local Box = self.CheckBoxes[ i ] + for i = 1, #Parent.CheckBoxes do + local Box = Parent.CheckBoxes[ i ] if Box:GetChecked() then Options[ #Options + 1 ] = Box.RadioOption end end - self:OnPropertyChanged( "SelectedOptions", Options ) + Parent:OnPropertyChanged( "SelectedOptions", Options ) + end + end + + function Radio:AddOption( Option ) + TypeCheck( Option, "table", 1, "AddOption" ) + TypeCheckField( Option, "Description", "string", "Option" ) + + local Index = #self.CheckBoxes + 1 + + local CheckBox = SGUI:Create( "CheckBox", self ) + CheckBox:SetFontScale( self.Font, self.TextScale ) + CheckBox:AddLabel( Option.Description ) + CheckBox:SetRadio( not self.MultipleChoice ) + CheckBox:SetAutoSize( self.CheckBoxAutoSize ) + CheckBox:SetStyleName( self.CheckBoxStyleName ) + CheckBox:SetTooltip( Option.Tooltip ) + CheckBox.RadioOption = Option + + if Index > 1 then + CheckBox:SetMargin( self.CheckBoxMargin ) end - end ) - self.CheckBoxes[ Index ] = CheckBox - self.CheckBoxes[ Option ] = CheckBox + CheckBox:AddPropertyChangeListener( "Checked", OnCheckBoxStateChanged ) - self.Layout:AddElement( CheckBox ) + self.CheckBoxes[ Index ] = CheckBox + self.CheckBoxes[ Option ] = CheckBox + + self.Layout:AddElement( CheckBox ) + end end function Radio:RemoveOption( Option ) From 7ee86f6ba4d1a62e0719ed32cb99fe4faccf4032 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 14:58:34 +0100 Subject: [PATCH 27/49] Fix a few more trace aborts in SGUI --- lua/shine/lib/gui/base_control.lua | 69 ++++++++++++++----------- lua/shine/lib/gui/objects/textentry.lua | 15 +++--- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index a8e7cb228..901157cb9 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -1372,6 +1372,10 @@ do return Vector2( Value.x, Value.y ) end + local function LinearEase( Progress ) + return Progress + end + local Max = math.max function ControlMeta:EaseValue( Element, Start, End, Delay, Duration, Callback, EasingHandlers ) @@ -1398,6 +1402,7 @@ do EasingData.Diff = SubtractValues( End, Start ) EasingData.CurValue = CopyValue( Start ) EasingData.Easer = EasingHandlers.Easer + EasingData.EaseFunc = LinearEase EasingData.StartTime = Clock() + Delay EasingData.Duration = Duration @@ -1418,45 +1423,47 @@ do end end -function ControlMeta:HandleEasing( Time, DeltaTime ) - if not self.EasingProcesses or self.EasingProcesses:IsEmpty() then return end - - for EasingHandler, Easings in self.EasingProcesses:Iterate() do - for Element, EasingData in Easings:Iterate() do - local Start = EasingData.StartTime +do + local function UpdateEasing( self, Time, DeltaTime, EasingHandler, Easings, Element, EasingData ) + EasingData.Elapsed = EasingData.Elapsed + Max( DeltaTime, Time - EasingData.LastUpdate ) + + local Duration = EasingData.Duration + local Elapsed = EasingData.Elapsed + if Elapsed <= Duration then + local Progress = EasingData.EaseFunc( Elapsed / Duration, EasingData.Power ) + EasingData.Easer( self, Element, EasingData, Progress ) + EasingHandler.Setter( self, Element, EasingData.CurValue, EasingData ) + else + EasingHandler.Setter( self, Element, EasingData.End, EasingData ) + if EasingHandler.OnComplete then + EasingHandler.OnComplete( self, Element, EasingData ) + end - if Start <= Time then - EasingData.Elapsed = EasingData.Elapsed + Max( DeltaTime, Time - EasingData.LastUpdate ) + Easings:Remove( Element ) - local Duration = EasingData.Duration - local Elapsed = EasingData.Elapsed - if Elapsed <= Duration then - local Progress = Elapsed / Duration - if EasingData.EaseFunc then - Progress = EasingData.EaseFunc( Progress, EasingData.Power ) - end + if EasingData.Callback then + EasingData.Callback( self, Element ) + end + end + end - EasingData.Easer( self, Element, EasingData, Progress ) - EasingHandler.Setter( self, Element, EasingData.CurValue, EasingData ) - else - EasingHandler.Setter( self, Element, EasingData.End, EasingData ) - if EasingHandler.OnComplete then - EasingHandler.OnComplete( self, Element, EasingData ) - end + function ControlMeta:HandleEasing( Time, DeltaTime ) + if not self.EasingProcesses or self.EasingProcesses:IsEmpty() then return end - Easings:Remove( Element ) + for EasingHandler, Easings in self.EasingProcesses:Iterate() do + for Element, EasingData in Easings:Iterate() do + local Start = EasingData.StartTime - if EasingData.Callback then - EasingData.Callback( self, Element ) - end + if Start <= Time then + UpdateEasing( self, Time, DeltaTime, EasingHandler, Easings, Element, EasingData ) end - end - EasingData.LastUpdate = Time - end + EasingData.LastUpdate = Time + end - if Easings:IsEmpty() then - self.EasingProcesses:Remove( EasingHandler ) + if Easings:IsEmpty() then + self.EasingProcesses:Remove( EasingHandler ) + end end end end diff --git a/lua/shine/lib/gui/objects/textentry.lua b/lua/shine/lib/gui/objects/textentry.lua index 6cf548c6d..d82db5b38 100644 --- a/lua/shine/lib/gui/objects/textentry.lua +++ b/lua/shine/lib/gui/objects/textentry.lua @@ -6,12 +6,11 @@ local SGUI = Shine.GUI local Timer = Shine.Timer local Clamp = math.Clamp -local Clock = os.clock +local Clock = Shared.GetSystemTimeReal local Max = math.max local Min = math.min local StringFind = string.find local StringFormat = string.format -local StringLength = string.len local StringLower = string.lower local StringSub = string.sub local StringUTF8Encode = string.UTF8Encode @@ -488,10 +487,10 @@ function TextEntry:SelectAll() end local function FindFurthestSpace( Text ) - local PreviousSpace = StringFind( Text, " " ) - --Find the furthest along space before the caret. + local PreviousSpace = StringFind( Text, " ", 1, true ) + -- Find the furthest along space before the caret. while PreviousSpace do - local NextSpace = StringFind( Text, " ", PreviousSpace + 1 ) + local NextSpace = StringFind( Text, " ", PreviousSpace + 1, true ) if NextSpace then PreviousSpace = NextSpace @@ -520,7 +519,7 @@ function TextEntry:FindWordBounds( CharPos ) end local After = StringUTF8Sub( Text, CharPos ) - local NextSpace = StringFind( After, " " ) or ( #After + 1 ) + local NextSpace = StringFind( After, " ", 1, true ) or ( #After + 1 ) NextSpace = StringUTF8Length( Before ) + StringUTF8Length( StringSub( After, 1, NextSpace - 1 ) ) return PreSpace, NextSpace @@ -666,9 +665,9 @@ function TextEntry:RemoveWord( Forward ) After = StringUTF8Sub( self.Text, self.Column + 1 ) - local NextSpace = StringFind( After, " " ) + local NextSpace = StringFind( After, " ", 1, true ) if not NextSpace then - NextSpace = StringLength( self.Text ) + NextSpace = #self.Text end Before = StringUTF8Sub( self.Text, 1, self.Column ) From 276399e6461f0c97c5096aefe6d8ab6b61647369 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 15:12:36 +0100 Subject: [PATCH 28/49] Add missing type checks to CodeGen functions --- lua/shine/lib/codegen.lua | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lua/shine/lib/codegen.lua b/lua/shine/lib/codegen.lua index c7330c830..18cdb4cb6 100644 --- a/lua/shine/lib/codegen.lua +++ b/lua/shine/lib/codegen.lua @@ -23,6 +23,9 @@ local CodeGen = {} The generated string with all variables substituted. ]] local function ApplyTemplateValues( FunctionCode, TemplateValues ) + Shine.TypeCheck( FunctionCode, "string", 1, "ApplyTemplateValues" ) + Shine.TypeCheck( TemplateValues, "table", 2, "ApplyTemplateValues" ) + return ( StringGSub( FunctionCode, "{([^%s]+)}", TemplateValues ) ) end CodeGen.ApplyTemplateValues = ApplyTemplateValues @@ -41,6 +44,10 @@ CodeGen.ApplyTemplateValues = ApplyTemplateValues The generated function. ]] local function GenerateTemplatedFunction( FunctionCode, ChunkName, TemplateValues, ... ) + Shine.TypeCheck( FunctionCode, "string", 1, "GenerateTemplatedFunction" ) + Shine.TypeCheck( ChunkName, { "string", "nil" }, 2, "GenerateTemplatedFunction" ) + Shine.TypeCheck( TemplateValues, "table", 3, "GenerateTemplatedFunction" ) + local GeneratedFunctionCode = ApplyTemplateValues( FunctionCode, TemplateValues ) return load( GeneratedFunctionCode, ChunkName )( ... ) end From bc762d3dd0fb574e4e80d4b2b7c71742e64565aa Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 15:29:50 +0100 Subject: [PATCH 29/49] Fix auto-hide scrollbars using wrong focus colour If the bar is clicked during fade-in, make sure the fade is cancelled. --- lua/shine/lib/gui/objects/scrollbar.lua | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lua/shine/lib/gui/objects/scrollbar.lua b/lua/shine/lib/gui/objects/scrollbar.lua index fef19ba03..4ed30feed 100644 --- a/lua/shine/lib/gui/objects/scrollbar.lua +++ b/lua/shine/lib/gui/objects/scrollbar.lua @@ -48,6 +48,19 @@ function Scrollbar:FadeOut( Duration, Callback, EaseFunc ) self:AlphaTo( self.Bar, nil, 0, 0, Duration, nil, EaseFunc ) end +function Scrollbar:StopFading() + if not self:GetEasing( "Alpha", self.Bar ) then return end + + self:StopAlpha( self.Background ) + self:StopAlpha( self.Bar ) + + self:SetAlpha( self:GetNormalAlpha( self.Background ) ) + + local BarColour = self.Bar:GetColor() + BarColour.a = self:GetNormalAlpha( self.Bar ) + self.Bar:SetColor( BarColour ) +end + function Scrollbar:SetHidden( Hidden ) if Hidden then self:HideAndDisableInput() @@ -150,6 +163,7 @@ function Scrollbar:OnMouseDown( Key, DoubleClick ) self.StartingX = X self.StartingY = Y + self:StopFading() self.Bar:SetColor( self.ActiveCol ) return true, self From d6f94b6c682b2c49fde170bc89286f3efaaaf791 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 15:39:13 +0100 Subject: [PATCH 30/49] Fix chatbox settings scrollbar ending up invisible --- lua/shine/lib/gui/objects/panel.lua | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lua/shine/lib/gui/objects/panel.lua b/lua/shine/lib/gui/objects/panel.lua index c0da60405..af02cf6a9 100644 --- a/lua/shine/lib/gui/objects/panel.lua +++ b/lua/shine/lib/gui/objects/panel.lua @@ -28,6 +28,21 @@ SGUI.AddProperty( Panel, "StickyScroll" ) SGUI.AddBoundProperty( Panel, "Colour", "Background:SetColor" ) SGUI.AddBoundProperty( Panel, "HideHorizontalScrollbar", "HorizontalScrollbar:SetHidden" ) +local function OnAutoHideScrollbarChanged( self, AutoHideScrollbar ) + if AutoHideScrollbar then return end + + -- When the scrollbar is set to no longer auto-hide, make sure it's visible. + if SGUI.IsValid( self.Scrollbar ) then + self.Scrollbar:SetIsVisible( true ) + self.Scrollbar:StopFading() + end + + if SGUI.IsValid( self.HorizontalScrollbar ) then + self.HorizontalScrollbar:SetIsVisible( true ) + self.HorizontalScrollbar:StopFading() + end +end + function Panel:Initialise() self.BaseClass.Initialise( self ) @@ -38,6 +53,8 @@ function Panel:Initialise() self.HorizontalScrollingEnabled = true self.BlockEventsIfFocusedWindow = true self.AlwaysInMouseFocus = false + + self:AddPropertyChangeListener( "AutoHideScrollbar", OnAutoHideScrollbarChanged ) end function Panel:SkinColour() From 03d46c0a4fdf272aca9b763d39f022a51d2c2cc1 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Sun, 12 Apr 2020 16:06:39 +0100 Subject: [PATCH 31/49] Add data argument to plugin timer methods --- lua/shine/core/shared/base_plugin/timers.lua | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lua/shine/core/shared/base_plugin/timers.lua b/lua/shine/core/shared/base_plugin/timers.lua index 2e36350e7..2984edd41 100644 --- a/lua/shine/core/shared/base_plugin/timers.lua +++ b/lua/shine/core/shared/base_plugin/timers.lua @@ -15,7 +15,7 @@ local TimerModule = {} Inputs: Same as Shine.Timer.Create. ]] -function TimerModule:CreateTimer( Name, Delay, Reps, Func ) +function TimerModule:CreateTimer( Name, Delay, Reps, Func, Data ) Shine.TypeCheck( Delay, "number", 2, "CreateTimer" ) Shine.TypeCheck( Reps, "number", 3, "CreateTimer" ) Shine.TypeCheck( Func, "function", 4, "CreateTimer" ) @@ -23,7 +23,7 @@ function TimerModule:CreateTimer( Name, Delay, Reps, Func ) self.Timers = rawget( self, "Timers" ) or setmetatable( {}, { __mode = "v" } ) local RealName = StringFormat( "%s_%s", self.__Name, Name ) - local Timer = Shine.Timer.Create( RealName, Delay, Reps, Func ) + local Timer = Shine.Timer.Create( RealName, Delay, Reps, Func, Data ) self.Timers[ Name ] = Timer @@ -34,13 +34,13 @@ end Creates a simple timer and adds it to the list of timers associated to the plugin. Inputs: Same as Shine.Timer.Simple. ]] -function TimerModule:SimpleTimer( Delay, Func ) +function TimerModule:SimpleTimer( Delay, Func, Data ) Shine.TypeCheck( Delay, "number", 1, "SimpleTimer" ) Shine.TypeCheck( Func, "function", 2, "SimpleTimer" ) self.Timers = rawget( self, "Timers" ) or setmetatable( {}, { __mode = "v" } ) - local Timer = Shine.Timer.Simple( Delay, Func ) + local Timer = Shine.Timer.Simple( Delay, Func, Data ) self.Timers[ Timer.Name ] = Timer From 06a800eeb1d95ca49389757abf8815d595212057 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Mon, 13 Apr 2020 10:43:14 +0100 Subject: [PATCH 32/49] Fix map vote menu not showing multiple choices If a map vote starts while the player has yet to trigger the ClientConfirmConnect event, they would not have the datatable values that indicate the type of vote. Datatables will now be sent at client connection time to ensure they are immediately in sync. --- lua/shine/extensions/mapvote/client.lua | 3 +++ lua/shine/extensions/mapvote/voting.lua | 36 +++++++++++++++++++++++-- lua/shine/lib/datatables.lua | 2 +- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/lua/shine/extensions/mapvote/client.lua b/lua/shine/extensions/mapvote/client.lua index 095c2c7e4..46ed6e22d 100644 --- a/lua/shine/extensions/mapvote/client.lua +++ b/lua/shine/extensions/mapvote/client.lua @@ -879,6 +879,9 @@ function Plugin:CreateMapVoteNotification( VoteButton ) end function Plugin:ReceiveVoteOptions( Message ) + -- Clear out any previous vote. + self:EndVote() + Shine.CheckVoteMenuBind() local Duration = Message.Duration diff --git a/lua/shine/extensions/mapvote/voting.lua b/lua/shine/extensions/mapvote/voting.lua index a07d505de..93e622fe5 100644 --- a/lua/shine/extensions/mapvote/voting.lua +++ b/lua/shine/extensions/mapvote/voting.lua @@ -132,11 +132,31 @@ end Send the map vote text and map options when a new player connects and a map vote is in progress. ]] function Plugin:ClientConfirmConnect( Client ) - if not self:VoteStarted() then return end + if not self:VoteStarted() or ( self.Vote.NotifiedClients and self.Vote.NotifiedClients[ Client ] ) then + if self.Logger:IsDebugEnabled() then + self.Logger:Debug( + "%s does not need to be notified of an ongoing map vote.", + Shine.GetClientInfo( Client ) + ) + end + return + end local Time = SharedTime() local Duration = Floor( self.Vote.EndTime - Time ) - if Duration < 5 then return end + if Duration < 5 then + if self.Logger:IsDebugEnabled() then + self.Logger:Debug( + "Skipping sending map vote to %s as the vote will end soon.", + Shine.GetClientInfo( Client ) + ) + end + return + end + + if self.Logger:IsDebugEnabled() then + self.Logger:Debug( "Sending map vote to %s who has just connected.", Shine.GetClientInfo( Client ) ) + end -- Send any mods for maps in the current vote (so the map vote menu shows the right preview image). self:SendMapMods( Client ) @@ -150,11 +170,17 @@ function Plugin:ClientConfirmConnect( Client ) for Map, Votes in pairs( self.Vote.VoteList ) do self:SendMapVoteCount( Client, Map, Votes ) end + + self.Vote.NotifiedClients[ Client ] = true end function Plugin:ClientDisconnect( Client ) self.StartingVote:ClientDisconnect( Client ) self:UpdateVoteCounters( self.StartingVote ) + + if self.Vote.NotifiedClients then + self.Vote.NotifiedClients[ Client ] = nil + end end function Plugin:SetGameState( Gamerules, State, OldState ) @@ -654,6 +680,7 @@ local TableRandom = table.ChooseRandom function Plugin:ProcessResults( NextMap ) self:EndVote() + self.Vote.NotifiedClients = nil local Cycle = self.MapCycle @@ -1031,6 +1058,7 @@ function Plugin:StartVote( NextMap, Force ) self.Vote.TotalVotes = 0 self.Vote.Voted = Shine.Multimap() self.Vote.NominationTracker = {} + self.Vote.NotifiedClients = {} local MapList = self:BuildMapChoices() self.Vote.MapList = MapList @@ -1069,6 +1097,10 @@ function Plugin:StartVote( NextMap, Force ) self:ProcessResults( NextMap ) end ) + for Client in Shine.IterateClients() do + self.Vote.NotifiedClients[ Client ] = true + end + -- Stop the game from starting if the current vote is player-initiated. self.CheckGameStart = self.CheckGameStartDuringVote self.UpdatePregame = self.UpdatePregameDuringVote diff --git a/lua/shine/lib/datatables.lua b/lua/shine/lib/datatables.lua index 1f74e9e02..2f98b2975 100644 --- a/lua/shine/lib/datatables.lua +++ b/lua/shine/lib/datatables.lua @@ -189,7 +189,7 @@ if Server then end -- Obey permissions... - Shine.Hook.Add( "ClientConfirmConnect", "DataTablesUpdate", function( Client ) + Shine.Hook.Add( "ClientConnect", "DataTablesUpdate", function( Client ) for Table, Data in pairs( RealData ) do if not Table.__Access then Shine.SendNetworkMessage( Client, Table.__Name, Data, true ) From cec49de48eaf71b9bc2e107822e06f35564585ea Mon Sep 17 00:00:00 2001 From: Person8880 Date: Mon, 13 Apr 2020 10:46:16 +0100 Subject: [PATCH 33/49] Allow text to be removed from buttons Also fire the property change event. --- lua/shine/lib/gui/objects/button.lua | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lua/shine/lib/gui/objects/button.lua b/lua/shine/lib/gui/objects/button.lua index 7344a4363..7aa5b96ab 100644 --- a/lua/shine/lib/gui/objects/button.lua +++ b/lua/shine/lib/gui/objects/button.lua @@ -70,14 +70,27 @@ local function UpdateIconMargin( self ) end function Button:SetText( Text ) - self:InvalidateParent() + if SGUI.IsValid( self.Label ) then + if not Text then + self.Label:Destroy() + self.Label = nil + self:InvalidateParent() + self:OnPropertyChanged( "Text", nil ) + return + end + + if self.Label:GetText() == Text then return end - if self.Label then self.Label:SetText( Text ) + self:InvalidateParent() self:InvalidateLayout() + self:OnPropertyChanged( "Text", Text ) + return end + if not Text then return end + local Description = SGUI:Create( "Label", self ) Description:SetIsSchemed( false ) Description:SetAlignment( self.TextAlignment or SGUI.LayoutAlignment.CENTRE ) @@ -125,8 +138,11 @@ function Button:SetText( Text ) self.Layout:AddElement( Description ) self.Label = Description + self:InvalidateParent() UpdateIconMargin( self ) + + self:OnPropertyChanged( "Text", Text ) end function Button:GetText() From 245d36fc2d072de28cae288c8982c9f2a0267fec Mon Sep 17 00:00:00 2001 From: Person8880 Date: Mon, 13 Apr 2020 11:07:29 +0100 Subject: [PATCH 34/49] Fix minor issues with map vote menu * Fix map selection state not being set properly when opening the menu. * Fix error when opening the menu due to highlighting changing before the tile is setup. --- lua/shine/extensions/mapvote/client.lua | 2 +- lua/shine/extensions/mapvote/ui/map_vote_menu.lua | 6 +++--- lua/shine/extensions/mapvote/ui/map_vote_menu_tile.lua | 8 ++++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lua/shine/extensions/mapvote/client.lua b/lua/shine/extensions/mapvote/client.lua index 46ed6e22d..07538598d 100644 --- a/lua/shine/extensions/mapvote/client.lua +++ b/lua/shine/extensions/mapvote/client.lua @@ -400,7 +400,7 @@ do MapName = MapName, NiceName = self:GetNiceMapName( MapName ), ModID = self.MapMods and self.MapMods[ MapName ] and tostring( self.MapMods[ MapName ] ), - IsSelected = MapName == self.ChosenMap, + IsSelected = self:IsMapSelected( MapName ), NumVotes = self.MapVoteCounts[ MapName ] } end ):AsTable() diff --git a/lua/shine/extensions/mapvote/ui/map_vote_menu.lua b/lua/shine/extensions/mapvote/ui/map_vote_menu.lua index f12e637d6..328db2149 100644 --- a/lua/shine/extensions/mapvote/ui/map_vote_menu.lua +++ b/lua/shine/extensions/mapvote/ui/map_vote_menu.lua @@ -606,24 +606,24 @@ function MapVoteMenu:SetMaps( Maps ) for i = 1, #Maps do local Entry = Maps[ i ] local Tile = SGUI:CreateFromDefinition( MapTile, self.Elements.MapTileGrid ) + Tile:SetMapVoteMenu( self ) Tile:SetSkin( Skin ) Tile:SetMap( Entry.ModID, Entry.MapName ) if Entry.MapName == Shared.GetMapName() then - Tile:SetText( + Tile:SetMapNameText( Locale:GetInterpolatedPhrase( "mapvote", "MAP_VOTE_MENU_EXTEND_MAP", { MapName = Entry.NiceName } ) ) else - Tile:SetText( Entry.NiceName ) + Tile:SetMapNameText( Entry.NiceName ) end Tile:SetSelected( Entry.IsSelected ) Tile:SetNumVotes( Entry.NumVotes ) Tile:SetInheritsParentAlpha( true ) Tile:SetTeamVariation( self:GetTeamVariation() ) - Tile:SetMapVoteMenu( self ) if #Maps > 9 then Tile:SetStyleName( "SmallerFonts" ) diff --git a/lua/shine/extensions/mapvote/ui/map_vote_menu_tile.lua b/lua/shine/extensions/mapvote/ui/map_vote_menu_tile.lua index f1da97a45..95f008b4f 100644 --- a/lua/shine/extensions/mapvote/ui/map_vote_menu_tile.lua +++ b/lua/shine/extensions/mapvote/ui/map_vote_menu_tile.lua @@ -30,9 +30,11 @@ SGUI.AddProperty( MapTile, "Selected", false ) SGUI.AddProperty( MapTile, "TeamVariation", "Marine" ) SGUI.AddProperty( MapTile, "WinnerType" ) -SGUI.AddBoundProperty( MapTile, "Text", "MapNameLabel:SetText" ) -SGUI.AddBoundProperty( MapTile, "TextColour", { "MapNameLabel:SetColour", "VoteCounterLabel:SetColour" } ) +SGUI.AddBoundProperty( MapTile, "MapNameText", "MapNameLabel:SetText" ) SGUI.AddBoundProperty( MapTile, "MapNameAutoFont", "MapNameLabel:SetAutoFont" ) +SGUI.AddBoundProperty( MapTile, "TextColour", { + "Label:SetColour", "MapNameLabel:SetColour", "VoteCounterLabel:SetColour" +} ) SGUI.AddBoundProperty( MapTile, "VoteCounterAutoFont", "VoteCounterLabel:SetAutoFont" ) MapTile.WinnerTypeName = table.AsEnum{ @@ -321,6 +323,8 @@ function MapTile:SetHighlighted( Highlighted, SkipAnim ) self.Highlighted = Highlighted self:OnPropertyChanged( "Highlighted", Highlighted ) + if not SGUI.IsValid( self.PreviewImage ) then return end + local Colour = Highlighted and self.PreviewImage.ActiveCol or self.PreviewImage.InactiveCol if SkipAnim then self.PreviewImage:SetColour( Colour ) From ab20111c259e79a5f527a9cd5658ef45af77d8f6 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Mon, 13 Apr 2020 11:42:26 +0100 Subject: [PATCH 35/49] Ensure the correct map vote elements are destroyed --- lua/shine/extensions/mapvote/client.lua | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/lua/shine/extensions/mapvote/client.lua b/lua/shine/extensions/mapvote/client.lua index 07538598d..ee682a570 100644 --- a/lua/shine/extensions/mapvote/client.lua +++ b/lua/shine/extensions/mapvote/client.lua @@ -780,19 +780,27 @@ function Plugin:EndVote() TableEmpty( self.MapButtons ) Shine.ScreenText.End( "MapVote" ) - if SGUI.IsValid( self.FullVoteMenu ) then - self.FullVoteMenu:Close( function() - if SGUI.IsValid( self.FullVoteMenu ) then - self.FullVoteMenu:Destroy() + local FullVoteMenu = self.FullVoteMenu + if SGUI.IsValid( FullVoteMenu ) then + FullVoteMenu:Close( function() + if not SGUI.IsValid( FullVoteMenu ) then return end + + FullVoteMenu:Destroy() + + if FullVoteMenu == self.FullVoteMenu then self.FullVoteMenu = nil end end ) end - if SGUI.IsValid( self.MapVoteNotification ) then - self.MapVoteNotification:Hide( function() - if SGUI.IsValid( self.MapVoteNotification ) then - self.MapVoteNotification:Destroy() + local MapVoteNotification = self.MapVoteNotification + if SGUI.IsValid( MapVoteNotification ) then + MapVoteNotification:Hide( function() + if not SGUI.IsValid( MapVoteNotification ) then return end + + MapVoteNotification:Destroy() + + if MapVoteNotification == self.MapVoteNotification then self.MapVoteNotification = nil end end ) From 71fb9274926ec3ee69a17f91dd611da8e937a0b9 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Mon, 13 Apr 2020 12:00:34 +0100 Subject: [PATCH 36/49] Fix stale mouse state in certain cases When becoming visible again, or changing parent, the mouse state should be invalidated. --- lua/shine/lib/gui/base_control.lua | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lua/shine/lib/gui/base_control.lua b/lua/shine/lib/gui/base_control.lua index 901157cb9..4eab82c45 100644 --- a/lua/shine/lib/gui/base_control.lua +++ b/lua/shine/lib/gui/base_control.lua @@ -371,6 +371,8 @@ function ControlMeta:SetParent( Control, Element ) self.ParentElement:RemoveChild( self.Background ) end + self:InvalidateMouseState() + if not Control then self.Parent = nil self.ParentElement = nil @@ -642,6 +644,8 @@ function ControlMeta:SetIsVisible( IsVisible ) if not IsVisible then self:HideTooltip() + else + self:InvalidateMouseState() end if not SGUI:IsWindow( self ) then return end From a7907ed66ad12b5791baf835b38b2dd17924b315 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Mon, 13 Apr 2020 14:40:28 +0100 Subject: [PATCH 37/49] Add test cases for every hooking mode --- test/core/shared/hook.lua | 198 +++++++++++++++++++++++++++++++++++--- test/test_init.lua | 35 +++++-- 2 files changed, 212 insertions(+), 21 deletions(-) diff --git a/test/core/shared/hook.lua b/test/core/shared/hook.lua index a8ec11ac8..968195f41 100644 --- a/test/core/shared/hook.lua +++ b/test/core/shared/hook.lua @@ -5,6 +5,13 @@ local UnitTest = Shine.UnitTest local Hook = Shine.Hook +local StringFormat = string.format + +UnitTest:After( function() + Hook.Clear( "Test" ) + Hook.Clear( "TestClassTestMethod" ) +end ) + local function TestHooks( Assert, MethodName ) local Called = {} Hook.Add( "Test", "Normal1", function() @@ -72,8 +79,6 @@ UnitTest:Test( "Hook integration test", function( Assert ) TestHooks( Assert, "Broadcast" ) end ) -Hook.Clear( "Test" ) - UnitTest:Test( "Broadcast - Ignores return values", function( Assert ) local Called = {} Hook.Add( "Test", "ReturnsValue", function() @@ -89,8 +94,6 @@ UnitTest:Test( "Broadcast - Ignores return values", function( Assert ) Assert.ArrayEquals( "Should have called both listeners", { "ReturnsValue", "DoesNotReturnValue" }, Called ) end ) -Hook.Clear( "Test" ) - local FunctionName = "TestFunction"..os.time() UnitTest:Test( "SetupGlobalHook - Replaces functions only once", function( Assert ) @@ -121,8 +124,6 @@ UnitTest:Test( "SetupGlobalHook - Replaces functions only once", function( Asser }, Callback.Invocations ) end ) -Hook.Clear( "Test" ) - local NestedName = "TestHolder"..os.time() UnitTest:Test( "SetupGlobalHook - Handles nested global values", function( Assert ) @@ -153,16 +154,13 @@ UnitTest:Test( "SetupGlobalHook - Handles nested global values", function( Asser }, Callback.Invocations ) end ) -Hook.Clear( "Test" ) - _G[ NestedName ] = nil _G[ FunctionName ] = nil local ClassName = "HookTestClass"..os.time() +class( ClassName ) UnitTest:Test( "SetupClassHook - Replaces functions only once", function( Assert ) - class( ClassName ) - local TestFunction = function( self ) end _G[ ClassName ].TestMethod = TestFunction @@ -195,6 +193,184 @@ UnitTest:Test( "SetupClassHook - Replaces functions only once", function( Assert }, Callback.Invocations ) end ) -Hook.Clear( "TestClassTestMethod" ) +local HOOK_RETURN_VALUE = "HOOK_RETURN_VALUE" +local ORIGINAL_RETURN_VALUE = "ORIGINAL_RETURN_VALUE" + +local HookModeTests = { + { + Mode = "Replace", + + ShouldCallOriginalOnHookReturn = false, + ExpectedOnHookReturn = HOOK_RETURN_VALUE, + + ShouldCallOriginalOnHookNil = false, + ExpectedOnHookNil = nil + }, + { + Mode = "PassivePre", + + ShouldCallOriginalOnHookReturn = true, + ExpectedOnHookReturn = ORIGINAL_RETURN_VALUE, + + ShouldCallOriginalOnHookNil = true, + ExpectedOnHookNil = ORIGINAL_RETURN_VALUE + }, + { + Mode = "PassivePost", + + ShouldCallOriginalOnHookReturn = true, + ExpectedOnHookReturn = ORIGINAL_RETURN_VALUE, + + ShouldCallOriginalOnHookNil = true, + ExpectedOnHookNil = ORIGINAL_RETURN_VALUE + }, + { + Mode = "ActivePre", + + ShouldCallOriginalOnHookReturn = false, + ExpectedOnHookReturn = HOOK_RETURN_VALUE, + + ShouldCallOriginalOnHookNil = true, + ExpectedOnHookNil = ORIGINAL_RETURN_VALUE + }, + { + Mode = "ActivePost", + + ShouldCallOriginalOnHookReturn = true, + ExpectedOnHookReturn = HOOK_RETURN_VALUE, + + ShouldCallOriginalOnHookNil = true, + ExpectedOnHookNil = ORIGINAL_RETURN_VALUE + }, + { + Mode = "Halt", + + ShouldCallOriginalOnHookReturn = false, + ExpectedOnHookReturn = nil, + + ShouldCallOriginalOnHookNil = true, + ExpectedOnHookNil = ORIGINAL_RETURN_VALUE + } +} + +local OriginalFunction = UnitTest.MockFunction( function() return ORIGINAL_RETURN_VALUE end ) +UnitTest:Before( function() + OriginalFunction:Reset() +end ) + +for i = 1, #HookModeTests do + local TestCase = HookModeTests[ i ] + local GlobalKey = FunctionName..TestCase.Mode + local GlobalHookName = "TestGlobalHook"..TestCase.Mode + + _G[ GlobalKey ] = OriginalFunction + Hook.SetupGlobalHook( GlobalKey, GlobalHookName, TestCase.Mode ) + + UnitTest:Test( + StringFormat( + "SetupGlobalHook - Applies a %s hook mode as expected when the hook returns a value", + TestCase.Mode + ), + function( Assert ) + Assert.NotEquals( "Should have replaced the global value", OriginalFunction, _G[ GlobalKey ] ) + + Hook.Add( GlobalHookName, "TestCase", function() return HOOK_RETURN_VALUE end ) + Assert.Equals( + "Should return the expected value when the hook returns a value", + TestCase.ExpectedOnHookReturn, + _G[ GlobalKey ]() + ) + + if TestCase.ShouldCallOriginalOnHookReturn then + Assert.Equals( "Should have called the original function", 1, OriginalFunction:GetInvocationCount() ) + else + Assert.Equals( + "Should not have called the original function", 0, OriginalFunction:GetInvocationCount() + ) + end + end + ) + + UnitTest:Test( + StringFormat( + "SetupGlobalHook - Applies a %s hook mode as expected when the hook returns nil", + TestCase.Mode + ), + function( Assert ) + Hook.Add( GlobalHookName, "TestCase", function() return nil end ) + Assert.Equals( + "Should return the expected value when the hook returns nil", + TestCase.ExpectedOnHookNil, + _G[ GlobalKey ]() + ) + if TestCase.ShouldCallOriginalOnHookNil then + Assert.Equals( "Should have called the original function", 1, OriginalFunction:GetInvocationCount() ) + else + Assert.Equals( + "Should not have called the original function", 0, OriginalFunction:GetInvocationCount() + ) + end + end + ) + + Hook.Clear( GlobalHookName ) + + _G[ GlobalKey ] = nil + + local ClassKey = TestCase.Mode + local ClassHookName = "TestClassHook"..TestCase.Mode + + _G[ ClassName ][ ClassKey ] = OriginalFunction + Hook.SetupClassHook( ClassName, ClassKey, ClassHookName, TestCase.Mode ) + + UnitTest:Test( + StringFormat( + "SetupClassHook - Applies a %s hook mode as expected when the hook returns a value", + TestCase.Mode + ), + function( Assert ) + Assert.NotEquals( "Should have replaced the class method", OriginalFunction, _G[ ClassName ][ ClassKey ] ) + + Hook.Add( ClassHookName, "TestCase", function() return HOOK_RETURN_VALUE end ) + Assert.Equals( + "Should return the expected value when the hook returns a value", + TestCase.ExpectedOnHookReturn, + _G[ ClassName ][ ClassKey ]() + ) + + if TestCase.ShouldCallOriginalOnHookReturn then + Assert.Equals( "Should have called the original function", 1, OriginalFunction:GetInvocationCount() ) + else + Assert.Equals( + "Should not have called the original function", 0, OriginalFunction:GetInvocationCount() + ) + end + end + ) + + UnitTest:Test( + StringFormat( + "SetupClassHook - Applies a %s hook mode as expected when the hook returns nil", + TestCase.Mode + ), + function( Assert ) + Hook.Add( ClassHookName, "TestCase", function() return nil end ) + Assert.Equals( + "Should return the expected value when the hook returns nil", + TestCase.ExpectedOnHookNil, + _G[ ClassName ][ ClassKey ]() + ) + if TestCase.ShouldCallOriginalOnHookNil then + Assert.Equals( "Should have called the original function", 1, OriginalFunction:GetInvocationCount() ) + else + Assert.Equals( + "Should not have called the original function", 0, OriginalFunction:GetInvocationCount() + ) + end + end + ) + + Hook.Clear( ClassHookName ) +end _G[ ClassName ] = nil diff --git a/test/test_init.lua b/test/test_init.lua index 23f52a7fe..2ea29cb30 100644 --- a/test/test_init.lua +++ b/test/test_init.lua @@ -29,6 +29,7 @@ local select = select local setmetatable = setmetatable local StringExplode = string.Explode local StringFormat = string.format +local TableClear = require "table.clear" local TableConcat = table.concat local TableCopy = table.Copy local type = type @@ -98,17 +99,31 @@ function UnitTest.MakeMockClient( SteamID ) } end -function UnitTest.MockFunction( Impl ) - return setmetatable( { - Invocations = {} - }, { - __call = function( self, ... ) - self.Invocations[ #self.Invocations + 1 ] = { ArgCount = select( "#", ... ), ... } - if Impl then - return Impl( ... ) - end +do + local MockFunction = {} + MockFunction.__index = MockFunction + + function MockFunction:Reset() + TableClear( self.Invocations ) + end + + function MockFunction:GetInvocationCount() + return #self.Invocations + end + + function MockFunction:__call( ... ) + self.Invocations[ #self.Invocations + 1 ] = { ArgCount = select( "#", ... ), ... } + if self.Impl then + return self.Impl( ... ) end - } ) + end + + function UnitTest.MockFunction( Impl ) + return setmetatable( { + Invocations = {}, + Impl = Impl + }, MockFunction ) + end end local MockedGlobals = {} From a7d2efaad9c55477eed2efe109d50e8c06140227 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 14 Apr 2020 10:14:49 +0100 Subject: [PATCH 38/49] Avoid vararg in recycle/consume hooks --- lua/shine/core/shared/hook.lua | 60 +++++++++++++++------------------- 1 file changed, 26 insertions(+), 34 deletions(-) diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index af80180a9..acc3f069e 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -833,8 +833,7 @@ if Client then SetupClassHook( "HelpScreen", "Display", "OnHelpScreenDisplay", "PassivePost" ) SetupClassHook( "HelpScreen", "Hide", "OnHelpScreenHide", "PassivePost" ) - Shine.Hook.SetupGlobalHook( "ClientUI.EvaluateUIVisibility", - "EvaluateUIVisibility", "PassivePost" ) + SetupGlobalHook( "ClientUI.EvaluateUIVisibility", "EvaluateUIVisibility", "PassivePost" ) local OptionsFunctions = { "Client.SetOptionInteger", @@ -843,7 +842,7 @@ if Client then "Client.SetOptionBoolean" } for i = 1, #OptionsFunctions do - Shine.Hook.SetupGlobalHook( OptionsFunctions[ i ], "OnClientOptionChanged", "PassivePost", { + SetupGlobalHook( OptionsFunctions[ i ], "OnClientOptionChanged", "PassivePost", { OverrideWithoutWarning = true } ) end @@ -971,43 +970,36 @@ Add( "Think", "ReplaceMethods", function() -- Need to double check for kTechId and the appropriate enums in case a different gamemode is running that doesn't -- use the NS2 code that creates this enum. if kTechId then - local function CallIfResearchIDMatches( HookName, TechID ) - return function( OldFunc, Building, ResearchID, ... ) - if ResearchID == TechID then - Broadcast( HookName, Building, ResearchID, ... ) - end - return OldFunc( Building, ResearchID, ... ) - end + local function SetupResearchHook( ClassName, MethodName, HookName, TechID ) + local Method = Shine.GetClassMethod( ClassName, MethodName ) + local NumArguments = GetNumArguments( Method ) + + SetupClassHook( ClassName, MethodName, HookName, CodeGen.GenerateFunctionWithArguments( + [[local Broadcast, HookName, TechID = ... + return function( OldFunc, Building, ResearchID{Arguments} ) + if ResearchID == TechID then + Broadcast( HookName, Building, ResearchID{Arguments} ) + end + return OldFunc( Building, ResearchID{Arguments} ) + end]], + NumArguments - 2, + StringFormat( "@lua/shine/core/shared/hook.lua/%s:%s", ClassName, MethodName ), + Broadcast, + HookName, + TechID + ) ) end if rawget( kTechId, "Recycle" ) then - SetupClassHook( - "RecycleMixin", "OnResearch", "OnRecycle", - CallIfResearchIDMatches( "OnRecycle", kTechId.Recycle ) - ) - SetupClassHook( - "RecycleMixin", "OnResearchComplete", "OnBuildingRecycled", - CallIfResearchIDMatches( "OnBuildingRecycled", kTechId.Recycle ) - ) - SetupClassHook( - "RecycleMixin", "OnResearchCancel", "OnRecycleCancelled", - CallIfResearchIDMatches( "OnRecycleCancelled", kTechId.Recycle ) - ) + SetupResearchHook( "RecycleMixin", "OnResearch", "OnRecycle", kTechId.Recycle ) + SetupResearchHook( "RecycleMixin", "OnResearchComplete", "OnBuildingRecycled", kTechId.Recycle ) + SetupResearchHook( "RecycleMixin", "OnResearchCancel", "OnRecycleCancelled", kTechId.Recycle ) end if rawget( kTechId, "Consume" ) then - SetupClassHook( - "ConsumeMixin", "OnResearch", "OnConsume", - CallIfResearchIDMatches( "OnConsume", kTechId.Consume ) - ) - SetupClassHook( - "ConsumeMixin", "OnResearchComplete", "OnBuildingConsumed", - CallIfResearchIDMatches( "OnBuildingConsumed", kTechId.Consume ) - ) - SetupClassHook( - "ConsumeMixin", "OnResearchCancel", "OnConsumeCancelled", - CallIfResearchIDMatches( "OnConsumeCancelled", kTechId.Consume ) - ) + SetupResearchHook( "ConsumeMixin", "OnResearch", "OnConsume", kTechId.Consume ) + SetupResearchHook( "ConsumeMixin", "OnResearchComplete", "OnBuildingConsumed", kTechId.Consume ) + SetupResearchHook( "ConsumeMixin", "OnResearchCancel", "OnConsumeCancelled", kTechId.Consume ) end end From e94c3aa6269004ed29054390d556f8d39762bdc0 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 14 Apr 2020 10:16:37 +0100 Subject: [PATCH 39/49] Add class/method/global name to hook sources --- lua/shine/core/shared/hook.lua | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index acc3f069e..d9ca5624b 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -349,13 +349,14 @@ local function AddClassHook( ReplacementFuncTemplate, HookName, Caller, Class, M return ReplacementFunc( OldFunc{Arguments} ) end]], NumArguments, - "@lua/shine/core/shared/hook.lua/CustomClassHook", + StringFormat( "@lua/shine/core/shared/hook.lua/CustomClassHook/%s:%s", Class, Method ), OldFunc, ReplacementFuncTemplate ) else ReplacementFunc = CodeGen.GenerateFunctionWithArguments( - ReplacementFuncTemplate, NumArguments, "@lua/shine/core/shared/hook.lua/ClassHook", + ReplacementFuncTemplate, NumArguments, + StringFormat( "@lua/shine/core/shared/hook.lua/ClassHook/%s:%s", Class, Method ), HookName, Caller, OldFunc ) end @@ -407,13 +408,13 @@ local function AddGlobalHook( ReplacementFunc, HookName, Caller, FuncName ) return ReplacementFunc( OldFunc{Arguments} ) end]], NumArguments, - "@lua/shine/core/shared/hook.lua/CustomGlobalHook", + StringFormat( "@lua/shine/core/shared/hook.lua/CustomGlobalHook/%s", FuncName ), Func, ReplacementFunc ) else Prev[ Path[ NumSegments ] ] = CodeGen.GenerateFunctionWithArguments( - ReplacementFunc, NumArguments, "@lua/shine/core/shared/hook.lua/GlobalHook", + ReplacementFunc, NumArguments, StringFormat( "@lua/shine/core/shared/hook.lua/GlobalHook/%s", FuncName ), HookName, Caller, Func ) end From 90f65446c74abeb97759b6b4d7ce1ee61121dd27 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 14 Apr 2020 10:52:23 +0100 Subject: [PATCH 40/49] Replace Server.GetOwner with Player:GetClient() Server.GetOwner is not compiled. --- lua/shine/extensions/afkkick/server.lua | 18 +++--- lua/shine/extensions/basecommands/server.lua | 7 ++- lua/shine/extensions/commbans/server.lua | 4 +- lua/shine/extensions/logging.lua | 20 +++---- lua/shine/extensions/mapvote/server.lua | 4 +- lua/shine/extensions/readyroom.lua | 8 +-- .../extensions/tournamentmode/server.lua | 3 +- .../extensions/voterandom/local_stats.lua | 14 ++--- lua/shine/extensions/voterandom/server.lua | 4 +- .../extensions/voterandom/team_balance.lua | 56 +++++++++++-------- lua/shine/extensions/votesurrender/server.lua | 7 +-- .../extensions/welcomemessages/server.lua | 4 +- lua/shine/lib/player.lua | 11 +++- 13 files changed, 88 insertions(+), 72 deletions(-) diff --git a/lua/shine/extensions/afkkick/server.lua b/lua/shine/extensions/afkkick/server.lua index 058e6a500..76160b49f 100644 --- a/lua/shine/extensions/afkkick/server.lua +++ b/lua/shine/extensions/afkkick/server.lua @@ -7,11 +7,11 @@ local Shine = Shine local assert = assert local Clamp = math.Clamp local Floor = math.floor +local GetClientForPlayer = Shine.GetClientForPlayer local GetHumanPlayerCount = Shine.GetHumanPlayerCount local GetMaxPlayers = Server.GetMaxPlayers local GetMaxSpectators = Server.GetMaxSpectators local GetNumClientsTotal = Server.GetNumClientsTotal -local GetOwner = Server.GetOwner local Max = math.max local Random = math.random local StringContainsNonUTF8Whitespace = string.ContainsNonUTF8Whitespace @@ -407,7 +407,7 @@ function Plugin:PrePlayerInfoUpdate( PlayerInfo, Player ) return end - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) local Data = self.Users:Get( Client ) -- Network the AFK state of the player. @@ -673,7 +673,7 @@ end and means that as soon as actions should be applied, they will be. ]] function Plugin:OnProcessMove( Player, Input ) - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client or Client:GetIsVirtual() then return end local DataTable = self.Users:Get( Client ) @@ -782,7 +782,7 @@ function Plugin:PlayerSay( Client, MessageTable ) end function Plugin:CanPlayerHearPlayer( Gamerules, Listener, Speaker ) - local Client = GetOwner( Speaker ) + local Client = GetClientForPlayer( Speaker ) if Client then self:SubtractAFKTime( Client, 0.1 ) end @@ -796,7 +796,7 @@ function Plugin:OnConstructInit( Building ) Owner = Owner or Team:GetCommander() if not Owner then return end - local Client = GetOwner( Owner ) + local Client = GetClientForPlayer( Owner ) if not Client then return end self:ResetAFKTime( Client ) @@ -809,7 +809,7 @@ function Plugin:OnRecycle( Building, ResearchID ) local Commander = Team:GetCommander() if not Commander then return end - local Client = GetOwner( Commander ) + local Client = GetClientForPlayer( Commander ) if not Client then return end self:ResetAFKTime( Client ) @@ -818,7 +818,7 @@ end do local function ResetForCommander() return function( self, Commander ) - local Client = GetOwner( Commander ) + local Client = GetClientForPlayer( Commander ) if not Client then return end self:ResetAFKTime( Client ) @@ -918,7 +918,7 @@ function Plugin:OnFirstThink() do local function MoveIfNotAFK( Player ) - local Client = Player and GetOwner( Player ) + local Client = Player and GetClientForPlayer( Player ) if not Client or self:IsAFKFor( Client, 60 ) then return end JoinRandomTeam( Player ) @@ -933,7 +933,7 @@ function Plugin:OnFirstThink() local function FilterPlayers( Player ) local ShouldKeep = true - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client or self:IsAFKFor( Client, 60 ) then ShouldKeep = false diff --git a/lua/shine/extensions/basecommands/server.lua b/lua/shine/extensions/basecommands/server.lua index aa4b4f27b..7737bab8c 100644 --- a/lua/shine/extensions/basecommands/server.lua +++ b/lua/shine/extensions/basecommands/server.lua @@ -6,7 +6,6 @@ local Shine = Shine local Hook = Shine.Hook local Call = Hook.Call -local GetOwner = Server.GetOwner local IsType = Shine.IsType local Min = math.min local Notify = Shared.Message @@ -365,6 +364,8 @@ function Plugin:TakeDamage( Ent, Damage, Attacker, Inflictor, Point, Direction, end do + local GetClientForPlayer = Shine.GetClientForPlayer + function Plugin:IsPregameAllTalk( Gamerules ) return self.Config.AllTalkPreGame and Gamerules:GetGameState() < kGameState.PreGame end @@ -403,7 +404,7 @@ do end function Plugin:CanPlayerHearLocalVoice( Gamerules, Listener, Speaker, SpeakerClient ) - local ListenerClient = GetOwner( Listener ) + local ListenerClient = GetClientForPlayer( Listener ) -- Default behaviour for those that have chosen to disable it. if self:IsLocalAllTalkDisabled( ListenerClient ) @@ -429,7 +430,7 @@ do Override voice chat to allow everyone to hear each other with alltalk on. ]] function Plugin:CanPlayerHearPlayer( Gamerules, Listener, Speaker, ChannelType ) - local SpeakerClient = GetOwner( Speaker ) + local SpeakerClient = GetClientForPlayer( Speaker ) if SpeakerClient and self:IsClientGagged( SpeakerClient ) then return false end if Listener:GetClientMuted( Speaker:GetClientIndex() ) then return false end diff --git a/lua/shine/extensions/commbans/server.lua b/lua/shine/extensions/commbans/server.lua index 92beba181..5b9a417b8 100644 --- a/lua/shine/extensions/commbans/server.lua +++ b/lua/shine/extensions/commbans/server.lua @@ -6,7 +6,7 @@ local Plugin = ... local Shine = Shine -local GetOwner = Server.GetOwner +local GetClientForPlayer = Shine.GetClientForPlayer local Max = math.max local StringFormat = string.format local Time = os.time @@ -52,7 +52,7 @@ end Deny commanding if they're banned. ]] function Plugin:ValidateCommanderLogin( Gamerules, CommandStation, Player ) - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client then self.Logger:Error( "Unable to get client for player %s! Cannot check for commander ban.", Player ) return diff --git a/lua/shine/extensions/logging.lua b/lua/shine/extensions/logging.lua index 9e678bc58..247200b02 100644 --- a/lua/shine/extensions/logging.lua +++ b/lua/shine/extensions/logging.lua @@ -92,7 +92,7 @@ function Plugin:PlayerNameChange( Player, Name, OldName ) if Name == kDefaultPlayerName then return end if OldName == kDefaultPlayerName then return end - local Client = Server.GetOwner( Player ) + local Client = Shine.GetClientForPlayer( Player ) if Client and Client:GetIsVirtual() then return end Shine:LogString( StringFormat( "%s changed their name from '%s' to '%s'.", @@ -103,7 +103,7 @@ function Plugin:PostJoinTeam( Gamerules, Player, OldTeam, NewTeam, Force ) if not self.Config.LogTeamJoins then return end if not Player then return end - local Client = Server.GetOwner( Player ) + local Client = Shine.GetClientForPlayer( Player ) if not Client then return end Shine:LogString( StringFormat( "Player %s joined team %s.", @@ -176,8 +176,8 @@ function Plugin:OnEntityKilled( Gamerules, Victim, Attacker, Inflictor, Point, D local AttackerPos = Attacker:GetOrigin() local VictimPos = Victim:GetOrigin() - local AttackerClient = Server.GetOwner( Attacker ) - local VictimClient = Server.GetOwner( Victim ) + local AttackerClient = Shine.GetClientForPlayer( Attacker ) + local VictimClient = Shine.GetClientForPlayer( Victim ) Shine:LogString( StringFormat( "%s killed %s with %s. Attacker location: %s. Victim location: %s.", AttackerClient and self:GetClientInfo( AttackerClient ) or Attacker:GetClassName(), @@ -198,8 +198,8 @@ function Plugin:CastVoteByPlayer( Gamerules, VoteTechID, Player ) if not CommPlayer then return end - local Target = Server.GetOwner( CommPlayer ) - local Client = Server.GetOwner( Player ) + local Target = Shine.GetClientForPlayer( CommPlayer ) + local Client = Shine.GetClientForPlayer( Player ) if Target and Client then Shine:LogString( StringFormat( "%s voted to eject %s.", @@ -212,7 +212,7 @@ function Plugin:CommLoginPlayer( Chair, Player ) if not Player then return end Shine:LogString( StringFormat( "%s became the commander of the %s.", - self:GetClientInfo( Server.GetOwner( Player ) ), + self:GetClientInfo( Shine.GetClientForPlayer( Player ) ), Shine:GetTeamName( Player:GetTeamNumber(), nil, true ) ) ) end @@ -224,7 +224,7 @@ function Plugin:CommLogout( Chair ) if not Commander then return end Shine:LogString( StringFormat( "%s stopped commanding the %s.", - self:GetClientInfo( Server.GetOwner( Commander ) ), + self:GetClientInfo( Shine.GetClientForPlayer( Commander ) ), Shine:GetTeamName( Commander:GetTeamNumber(), nil, true ) ) ) end @@ -239,7 +239,7 @@ do local Commander = Team:GetCommander() if not Commander then return end - local Client = Server.GetOwner( Commander ) + local Client = Shine.GetClientForPlayer( Commander ) Shine:LogString( StringFormat( "%s %s %s %s[%s].", self:GetClientInfo( Client ), State, RecycleAction, Name, ID ) ) end @@ -307,7 +307,7 @@ function Plugin:OnConstructInit( Building ) if not Owner then return end - local Client = Server.GetOwner( Owner ) + local Client = Shine.GetClientForPlayer( Owner ) Shine:LogString( StringFormat( "%s began construction of %s[%s].", self:GetClientInfo( Client ), Name, ID ) ) end diff --git a/lua/shine/extensions/mapvote/server.lua b/lua/shine/extensions/mapvote/server.lua index 1d0c177a7..77cbc9126 100644 --- a/lua/shine/extensions/mapvote/server.lua +++ b/lua/shine/extensions/mapvote/server.lua @@ -7,7 +7,7 @@ local Shine = Shine local Ceil = math.ceil local Clamp = math.Clamp local Floor = math.floor -local GetOwner = Server.GetOwner +local GetClientForPlayer = Shine.GetClientForPlayer local IsType = Shine.IsType local Max = math.max local Notify = Shared.Message @@ -668,7 +668,7 @@ function Plugin:JoinTeam( Gamerules, Player, NewTeam, Force, ShineForce ) if ShineForce then return end if NewTeam == 0 then return end - if Shine:CanNotify( GetOwner( Player ) ) then + if Shine:CanNotify( GetClientForPlayer( Player ) ) then self:SendTranslatedNotify( Player, "TeamSwitchFail", { IsEndVote = IsEndVote or false } ) diff --git a/lua/shine/extensions/readyroom.lua b/lua/shine/extensions/readyroom.lua index f61d70f36..3b6983b45 100644 --- a/lua/shine/extensions/readyroom.lua +++ b/lua/shine/extensions/readyroom.lua @@ -6,7 +6,7 @@ local Shine = Shine -local GetOwner = Server.GetOwner +local GetClientForPlayer = Shine.GetClientForPlayer local pairs = pairs local Random = math.random local SharedTime = Shared.GetTime @@ -65,7 +65,7 @@ function Plugin:JoinTeam( Gamerules, Player, NewTeam, Force, ShineForce ) return end - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client then return end if Client.JoinTeamRRPlugin then return end @@ -124,7 +124,7 @@ function Plugin:EndGame() local Player = Players[ i ] if Player then - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if Client then self.TeamMemory[ Client ] = Player:GetTeamNumber() @@ -150,7 +150,7 @@ function Plugin:JoinRandomTeam( Player ) local Team1 = Gamerules:GetTeam( kTeam1Index ):GetNumPlayers() local Team2 = Gamerules:GetTeam( kTeam2Index ):GetNumPlayers() - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client then return end diff --git a/lua/shine/extensions/tournamentmode/server.lua b/lua/shine/extensions/tournamentmode/server.lua index 0071a993f..d9d921bb4 100644 --- a/lua/shine/extensions/tournamentmode/server.lua +++ b/lua/shine/extensions/tournamentmode/server.lua @@ -281,8 +281,7 @@ end function Plugin:PostJoinTeam( Gamerules, Player, OldTeam, NewTeam, Force ) if NewTeam == 0 or NewTeam == 3 then return end - local Client = Server.GetOwner( Player ) - + local Client = Shine.GetClientForPlayer( Player ) if not Client then return end local ID = Client:GetUserId() diff --git a/lua/shine/extensions/voterandom/local_stats.lua b/lua/shine/extensions/voterandom/local_stats.lua index 70c466d5d..ef7efb8e7 100644 --- a/lua/shine/extensions/voterandom/local_stats.lua +++ b/lua/shine/extensions/voterandom/local_stats.lua @@ -4,7 +4,7 @@ local Shine = Shine -local GetOwner = Server.GetOwner +local GetClientForPlayer = Shine.GetClientForPlayer local Min = math.min local setmetatable = setmetatable local tostring = tostring @@ -64,7 +64,7 @@ do if not self.Config.UseLocalFileStats then return end if not self.StatsStorage:IsInTransaction() then return end - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client or Client:GetIsVirtual() then return end self:IncrementStatValue( GetClientUID( Client ), Player, StatValue, Value or 1 ) @@ -127,7 +127,7 @@ function StatsModule:IsRookie( ClientID, Player ) end function StatsModule:IsPlayerRookie( Player ) - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client then return false end return self:IsRookie( GetClientUID( Client ), Player ) @@ -162,7 +162,7 @@ end function StatsModule:GetPlayerKDR( Player ) if not self.Config.UseLocalFileStats then return end - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client then return 0 end return self:GetKDRStat( GetClientUID( Client ), Player ) @@ -178,7 +178,7 @@ end function StatsModule:GetPlayerScorePerMinute( Player ) if not self.Config.UseLocalFileStats then return end - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client then return 0 end return self:GetScorePerMinuteStat( GetClientUID( Client ), Player ) @@ -222,8 +222,8 @@ function StatsModule:EndGame( Gamerules, WinningTeam, Players ) for i = 1, #Players do local Player = Players[ i ] - local Client = GetOwner( Player ) - if not Client:GetIsVirtual() and Player.client and Player.GetMarinePlayTime then + local Client = GetClientForPlayer( Player ) + if Client and not Client:GetIsVirtual() and Player.client and Player.GetMarinePlayTime then self:StoreRoundEndData( GetClientUID( Client ), Player, WinningTeamNumber, RoundLength ) end end diff --git a/lua/shine/extensions/voterandom/server.lua b/lua/shine/extensions/voterandom/server.lua index b44189ecd..40d1af0d5 100644 --- a/lua/shine/extensions/voterandom/server.lua +++ b/lua/shine/extensions/voterandom/server.lua @@ -14,7 +14,7 @@ local Floor = math.floor local GetAllPlayers = Shine.GetAllPlayers local GetNumPlayers = Shine.GetHumanPlayerCount local GetNumSpectators = Server.GetNumSpectators -local GetOwner = Server.GetOwner +local GetClientForPlayer = Shine.GetClientForPlayer local IsType = Shine.IsType local Max = math.max local Random = math.random @@ -370,7 +370,7 @@ function EnforcementPolicy:IsPolicyEnforced( Policy, PlayerCount ) end function EnforcementPolicy:JoinTeam( Plugin, Gamerules, Player, NewTeam, Force ) - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client then return false end local Immune = Shine:HasAccess( Client, "sh_randomimmune" ) diff --git a/lua/shine/extensions/voterandom/team_balance.lua b/lua/shine/extensions/voterandom/team_balance.lua index 7c9477075..9dd57c677 100644 --- a/lua/shine/extensions/voterandom/team_balance.lua +++ b/lua/shine/extensions/voterandom/team_balance.lua @@ -11,7 +11,7 @@ local BalanceModule = {} local Abs = math.abs local Ceil = math.ceil local Clamp = math.Clamp -local GetOwner = Server.GetOwner +local GetClientForPlayer = Shine.GetClientForPlayer local IsType = Shine.IsType local Max = math.max local next = next @@ -133,12 +133,8 @@ local DebugMode = false -- TeamNumber parameter currently unused, but ready for Hive 2.0 BalanceModule.SkillGetters = { GetHiveSkill = function( Ply, TeamNumber ) - local Client = GetOwner( Ply ) + local Client = GetClientForPlayer( Ply ) if Client and Client:GetIsVirtual() then - if DebugMode then - Client.Skill = Client.Skill or Random( 0, 2500 ) - return Client.Skill - end -- Bots are all equal so there's no reason to consider them. return nil end @@ -152,14 +148,6 @@ BalanceModule.SkillGetters = { -- KA/D Ratio. GetKDR = function( Ply, TeamNumber ) - if DebugMode then - local Client = GetOwner( Ply ) - if Client and Client:GetIsVirtual() then - Client.Skill = Client.Skill or Random() * 3 - return Client.Skill - end - end - do local KDR = Plugin:CallModuleEvent( "GetPlayerKDR", Ply, TeamNumber ) if KDR then return KDR end @@ -180,14 +168,6 @@ BalanceModule.SkillGetters = { -- Score per minute played. GetScore = function( Ply, TeamNumber ) - if DebugMode then - local Client = GetOwner( Ply ) - if Client and Client:GetIsVirtual() then - Client.Skill = Client.Skill or Random() * 10 - return Client.Skill - end - end - do local ScorePerMinute = Plugin:CallModuleEvent( "GetPlayerScorePerMinute", Ply, TeamNumber ) if ScorePerMinute then return ScorePerMinute end @@ -206,6 +186,38 @@ BalanceModule.SkillGetters = { end } +if DebugMode then + local OldGetHiveSkill = BalanceModule.SkillGetters.GetHiveSkill + BalanceModule.SkillGetters.GetHiveSkill = function( Ply, TeamNumber ) + local Client = GetClientForPlayer( Ply ) + if Client and Client:GetIsVirtual() then + Client.Skill = Client.Skill or Random( 0, 2500 ) + return Client.Skill + end + return OldGetHiveSkill( Ply, TeamNumber ) + end + + local OldGetKDR = BalanceModule.SkillGetters.GetKDR + BalanceModule.SkillGetters.GetKDR = function( Ply, TeamNumber ) + local Client = GetClientForPlayer( Ply ) + if Client and Client:GetIsVirtual() then + Client.Skill = Client.Skill or Random() * 3 + return Client.Skill + end + return OldGetKDR( Ply, TeamNumber ) + end + + local OldGetScore = BalanceModule.SkillGetters.GetScore + BalanceModule.SkillGetters.GetScore = function( Ply, TeamNumber ) + local Client = GetClientForPlayer( Ply ) + if Client and Client:GetIsVirtual() then + Client.Skill = Client.Skill or Random() * 10 + return Client.Skill + end + return OldGetScore( Ply, TeamNumber ) + end +end + BalanceModule.HappinessHistoryFile = "config://shine/temp/shuffle_happiness.json" function BalanceModule:Initialise() diff --git a/lua/shine/extensions/votesurrender/server.lua b/lua/shine/extensions/votesurrender/server.lua index 731d70d3f..8a995e1c2 100644 --- a/lua/shine/extensions/votesurrender/server.lua +++ b/lua/shine/extensions/votesurrender/server.lua @@ -4,7 +4,7 @@ local Shine = Shine -local GetOwner = Server.GetOwner +local GetClientForPlayer = Shine.GetClientForPlayer local SharedTime = Shared.GetTime local StringFormat = string.format @@ -96,7 +96,7 @@ function Plugin:GetTeamPlayerCount( Team ) local Count = 0 local function CountPlayers( Player ) - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if Shine:IsValidClient( Client ) and self:IsValidVoter( Client ) then Count = Count + 1 end @@ -182,12 +182,11 @@ end ]] function Plugin:PostJoinTeam( Gamerules, Player, OldTeam, NewTeam, Force, ShineForce ) if not Player then return end - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if not Client then return end local Vote = self.Votes[ OldTeam ] - if Vote then Vote:RemoveVote( Client ) end diff --git a/lua/shine/extensions/welcomemessages/server.lua b/lua/shine/extensions/welcomemessages/server.lua index 05cf8b29c..87c9b4561 100644 --- a/lua/shine/extensions/welcomemessages/server.lua +++ b/lua/shine/extensions/welcomemessages/server.lua @@ -6,7 +6,7 @@ local ChatAPI = require "shine/core/shared/chat/chat_api" local Shine = Shine -local GetOwner = Server.GetOwner +local GetClientForPlayer = Shine.GetClientForPlayer local IsType = Shine.IsType local IsValid = debug.isvalid local StringFormat = string.format @@ -267,7 +267,7 @@ function Plugin:PostJoinTeam( Gamerules, Player, OldTeam, NewTeam, Force, ShineF if NewTeam < 0 then return end if not Player then return end - local Client = GetOwner( Player ) + local Client = GetClientForPlayer( Player ) if Client then Client.DisconnectTeam = NewTeam end diff --git a/lua/shine/lib/player.lua b/lua/shine/lib/player.lua index e83c58acb..0127a9abe 100644 --- a/lua/shine/lib/player.lua +++ b/lua/shine/lib/player.lua @@ -129,7 +129,6 @@ end local Abs = math.abs local Floor = math.floor -local GetOwner = Server.GetOwner local IsType = Shine.IsType local pairs = pairs local StringFind = string.find @@ -151,6 +150,13 @@ function Shine:IsValidClient( Client ) return Client and self.GameIDs:Get( Client ) ~= nil end +--[[ + Returns the client associated with the given player. +]] +function Shine.GetClientForPlayer( Player ) + return Player.GetClient and Player:GetClient() +end + function Shine.EqualiseTeamCounts( TeamMembers ) local Marine = TeamMembers[ 1 ] local Alien = TeamMembers[ 2 ] @@ -312,8 +318,7 @@ function Shine.GetTeamClients( Team ) local Ply = Players[ i ] if Ply then - local Client = GetOwner( Ply ) - + local Client = Shine.GetClientForPlayer( Ply ) if Client then Clients[ Count ] = Client Count = Count + 1 From ea5679cce217829df0950d856badf347bddcffa5 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 14 Apr 2020 12:14:52 +0100 Subject: [PATCH 41/49] Refactor extension hooks to remove inner loop Now extension hooks are added directly with Hook.Add using a unique key. This avoids a second level of dispatching which improves performance. --- lua/shine/core/shared/extensions.lua | 195 +++++++++++++++++---------- lua/shine/core/shared/hook.lua | 81 +++++------ lua/shine/lib/codegen.lua | 12 +- 3 files changed, 170 insertions(+), 118 deletions(-) diff --git a/lua/shine/core/shared/extensions.lua b/lua/shine/core/shared/extensions.lua index eb42edd67..e7e00f08c 100644 --- a/lua/shine/core/shared/extensions.lua +++ b/lua/shine/core/shared/extensions.lua @@ -369,9 +369,6 @@ Hook.Add( "OnFirstThink", "ExtensionFirstThink", function() HasFirstThinkOccurred = true end ) --- Handles dispatching plugin events efficiently. -local Dispatcher - local function CheckPluginConflicts( self, Conflicts ) if not Conflicts or not Server then return true end @@ -425,6 +422,110 @@ local function CheckDependencies( self, Dependencies ) return true end +local AddPluginHook +local RemovePluginHook +local RemoveAllPluginHooks +do + local CodeGen = require "shine/lib/codegen" + local select = select + + local OnError = Shine.BuildErrorHandler( "Plugin hook error" ) + + local Callers = CodeGen.MakeFunctionGenerator( { + Template = [[local OnError, Shine, xpcall = ... + return function( Plugin, Method, Event{Arguments} ) + local Success, a, b, c, d, e, f = xpcall( Method, OnError, Plugin{Arguments} ) + + if not Success then + Plugin.__HookErrors = ( Plugin.__HookErrors or 0 ) + 1 + Shine:DebugPrint( "[Hook Error] %s hook failed from plugin '%s'. Error count: %i.", + true, Event, Plugin.__Name, Plugin.__HookErrors ) + + if Plugin.__HookErrors >= 10 then + Shine:DebugPrint( "Unloading plugin '%s' for too many hook errors (%i).", + true, Plugin.__Name, Plugin.__HookErrors ) + + Plugin.__HookErrors = 0 + + Shine:UnloadExtension( Plugin.__Name ) + end + + return nil + end + + return a, b, c, d, e, f + end + ]], + ChunkName = "lua/shine/core/shared/extensions.lua/CallEvent", + -- This should match the value used in the hook system. + InitialSize = 10, + Args = { OnError, Shine, xpcall } + } ) + + local EventKey = Shine.TypeDef() + function EventKey:Init( Plugin ) + self.Plugin = Plugin + return self + end + function EventKey:__tostring() + return StringFormat( "Plugin - %s", self.Plugin:GetName() ) + end + + local EventCaller = Shine.TypeDef() + function EventCaller:Init( Plugin, Event ) + self.Plugin = Plugin + self.Event = Event + return self + end + + function EventCaller:__call( ... ) + return Callers[ select( "#", ... ) ]( self.Plugin, self.Plugin[ self.Event ], self.Event, ... ) + end + + local PluginEventKeys = {} + local PluginEvents = setmetatable( {}, { + __index = function( self, Key ) + local Events = Shine.Set() + + self[ Key ] = Events + + return Events + end + } ) + + AddPluginHook = function( Plugin, Event ) + if not IsType( Plugin[ Event ], "function" ) then return end + + local Keys = PluginEventKeys[ Event ] or {} + PluginEventKeys[ Event ] = Keys + + local Key = Keys[ Plugin ] or EventKey( Plugin ) + Keys[ Plugin ] = Key + + PluginEvents[ Plugin ]:Add( Event ) + + Hook.Add( Event, Key, EventCaller( Plugin, Event ), Hook.MAX_PRIORITY + 1 ) + end + + RemovePluginHook = function( Plugin, Event ) + local Key = PluginEventKeys[ Event ] and PluginEventKeys[ Event ][ Plugin ] + if Key then + Hook.Remove( Event, Key ) + PluginEventKeys[ Event ][ Plugin ] = nil + end + end + + RemoveAllPluginHooks = function( Plugin ) + local Events = PluginEvents[ Plugin ] + + for Event in Events:Iterate() do + RemovePluginHook( Plugin, Event ) + end + + PluginEvents[ Plugin ] = nil + end +end + do local OnInitError = Shine.BuildErrorHandler( "Plugin initialisation error" ) local function MarkAsDisabled( Plugin, FirstEnable ) @@ -452,6 +553,13 @@ do } ) end + local function ResetPluginHooks() + local Events = Hook.GetKnownEvents() + for i = 1, #Events do + Shine:SetupExtensionEvents( Events[ i ] ) + end + end + -- Shared extensions need to be enabled once the server tells it to. function Shine:EnableExtension( Name, DontLoadConfig ) Shine.TypeCheck( Name, "string", 1, "EnableExtension" ) @@ -538,10 +646,9 @@ do end end - -- Flush event cache. We can't be smarter than this, because it could be loading mid-event call. - -- If we edited the specific event table at that point, it would potentially cause double event calls - -- or missed event calls. Plugin load/unload should be a rare occurence anyway. - Dispatcher:FlushCache() + -- Reset all hooks to maintain a consistent calling order. + -- Loading an extension is a rare event, so the cost of this is acceptable. + ResetPluginHooks() -- We need to inform clients to enable the client portion. if Server and Plugin.IsShared and not self.GameIDs:IsEmpty() then @@ -575,9 +682,6 @@ do Plugin.Enabled = false - -- Flush event cache. - Dispatcher:FlushCache() - -- Make sure cleanup doesn't break us by erroring. local Success = xpcall( Plugin.Cleanup, OnCleanupError, Plugin ) if not Success then @@ -589,6 +693,8 @@ do Plugin:ResetModuleEventHistory() + RemoveAllPluginHooks( Plugin ) + if Server and Plugin.IsShared and not self.GameIDs:IsEmpty() then Shine.SendNetworkMessage( "Shine_PluginEnable", { Plugin = Name, Enabled = false }, true ) end @@ -624,69 +730,18 @@ Shine.AllPlugins = AllPlugins local AllPluginsArray = {} Shine.AllPluginsArray = AllPluginsArray -do - local CodeGen = require "shine/lib/codegen" - local select = select - - Dispatcher = Shine.EventDispatcher( AllPluginsArray ) +function Shine:SetupExtensionEvents( Event ) + for i = 1, #AllPluginsArray do + local PluginName = AllPluginsArray[ i ] + local Plugin = Shine.Plugins[ PluginName ] - function Dispatcher:GetListener( PluginName ) - return Shine.Plugins[ PluginName ] - end - - function Dispatcher:IsListenerValidForEvent( Plugin, Event ) - return Plugin and Plugin.Enabled and IsType( Plugin[ Event ], "function" ) - end + if Plugin then + RemovePluginHook( Plugin, Event ) - local Callers = CodeGen.MakeFunctionGenerator( { - Template = [[local Shine, xpcall = ... - return function( Plugin, Method, OnError, Event{Arguments} ) - local Success, a, b, c, d, e, f = xpcall( Method, OnError, Plugin{Arguments} ) - - if not Success then - Plugin.__HookErrors = ( Plugin.__HookErrors or 0 ) + 1 - Shine:DebugPrint( "[Hook Error] %s hook failed from plugin '%s'. Error count: %i.", - true, Event, Plugin.__Name, Plugin.__HookErrors ) - - if Plugin.__HookErrors >= 10 then - Shine:DebugPrint( "Unloading plugin '%s' for too many hook errors (%i).", - true, Plugin.__Name, Plugin.__HookErrors ) - - Plugin.__HookErrors = 0 - - Shine:UnloadExtension( Plugin.__Name ) - end - - return nil - end - - return a, b, c, d, e, f + if Plugin.Enabled then + AddPluginHook( Plugin, Event ) end - ]], - ChunkName = "lua/shine/core/shared/extensions.lua/Dispatcher:CallEvent", - -- This should match the value used in the hook system. - InitialSize = 10, - Args = { Shine, xpcall } - } ) - - function Dispatcher:CallEvent( Plugin, Method, OnError, Event, ... ) - return Callers[ select( "#", ... ) ]( Plugin, Method, OnError, Event, ... ) - end - - --[[ - Calls an event on all active extensions. - Called by the hook system, should not be called directly. - ]] - function Shine:CallExtensionEvent( Event, OnError, ... ) - return Dispatcher:DispatchEvent( Event, OnError, Event, ... ) - end - - --[[ - Broadcasts an event to all active extensions. - Called by the hook system, should not be called directly. - ]] - function Shine:BroadcastExtensionEvent( Event, OnError, ... ) - return Dispatcher:BroadcastEvent( Event, OnError, Event, ... ) + end end end diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index d9ca5624b..04ad451a4 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -19,6 +19,7 @@ local ReplaceMethod = Shine.ReplaceClassMethod local select = select local StringExplode = string.Explode local StringFormat = string.format +local TableQuickCopy = table.QuickCopy local xpcall = xpcall local LinkedList = Shine.LinkedList @@ -33,52 +34,31 @@ local ExtensionIndex = setmetatable( {}, { __tostring = function() return "CallE local HookNodes = {} -- A mapping of event -> hook index -> hook callback (for external use). local HooksByEventAndIndex = {} +-- Known event names. +local KnownEvents = Shine.Set() local OnError = Shine.BuildErrorHandler( "Hook error" ) local Hooks do - -- Use callable tables to avoid creating closures during hook calls. - -- This helps to avoid LuaJIT blacklisting the hook call/broadcast functions. - local PluginCaller = { - __call = function( self, ... ) - return Shine:CallExtensionEvent( self[ 1 ], OnError, ... ) - end, - __tostring = function( self ) - return StringFormat( "CallExtensionEvent - %s", self[ 1 ] ) - end - } - local PluginBroadcaster = { - __call = function( self, ... ) - return Shine:BroadcastExtensionEvent( self[ 1 ], OnError, ... ) - end, - __tostring = function( self ) - return StringFormat( "BroadcastExtensionEvent - %s", self[ 1 ] ) - end - } + if not Shine.SetupExtensionEvents then + Shine.SetupExtensionEvents = function() end + end Hooks = setmetatable( {}, { - -- On first call/addition of an event, setup a default hook to call the event on extensions. + -- On first call/addition of an event, setup the necessary data structures. __index = function( self, Event ) - local HooksByIndex = LinkedList() - local Node = HooksByIndex:Add( { - -- This emulates the old behaviour, placing the event between -20 and -19. - -- No client of the public API can set non-integer priorities. - Priority = -19.5, - Callback = setmetatable( { Event }, PluginCaller ), - BroadcastCallback = setmetatable( { Event }, PluginBroadcaster ), - Index = ExtensionIndex - } ) + KnownEvents:Add( Event ) - HookNodes[ Event ] = { - [ ExtensionIndex ] = Node - } - HooksByEventAndIndex[ Event ] = { - [ ExtensionIndex ] = Node.Value.Callback - } + local HooksByIndex = LinkedList() + HookNodes[ Event ] = {} + HooksByEventAndIndex[ Event ] = {} -- Save the list on the table to avoid invoking this again. self[ Event ] = HooksByIndex + -- Allow extensions to setup their own events. + Shine:SetupExtensionEvents( Event ) + return HooksByIndex end } ) @@ -167,15 +147,6 @@ local function Add( Event, Index, Function, Priority ) end Hook.Add = Add --- Placeholder until the extensions file is loaded. -if not Shine.CallExtensionEvent then - Shine.CallExtensionEvent = function() end -end - -if not Shine.BroadcastExtensionEvent then - Shine.BroadcastExtensionEvent = function() end -end - local Call do -- See the comment in codegen.lua for the reasoning of this seemingly bizarre way of calling hooks. @@ -202,7 +173,13 @@ do end end end]], - ChunkName = "@lua/shine/core/shared/hook.lua/Call", + ChunkName = function( NumArguments ) + return StringFormat( + "@lua/shine/core/shared/hook.lua/CallWith%sArg%s", + NumArguments, + NumArguments == 1 and "" or "s" + ) + end, -- This should equal the largest number of arguments seen by a hook to avoid lazy-generation which can impact -- compilation results. InitialSize = 10, @@ -247,7 +224,13 @@ do end end end]], - ChunkName = "@lua/shine/core/shared/hook.lua/Broadcast", + ChunkName = function( NumArguments ) + return StringFormat( + "@lua/shine/core/shared/hook.lua/BroadcastWith%sArg%s", + NumArguments, + NumArguments == 1 and "" or "s" + ) + end, InitialSize = 10, Args = { Shine, Hooks, OnError, Remove, ExtensionIndex }, OnFunctionGenerated = function( NumArguments, Caller ) @@ -276,6 +259,7 @@ local function ClearHooks( Event ) Hooks[ Event ] = nil HooksByEventAndIndex[ Event ] = nil HookNodes[ Event ] = nil + KnownEvents:Remove( Event ) end Hook.Clear = ClearHooks @@ -301,6 +285,13 @@ function Hook.GetTable() return HooksByEventAndIndex end +--[[ + Provides a list of all known event names that have been fired at least once and not cleared. +]] +function Hook.GetKnownEvents() + return TableQuickCopy( KnownEvents:AsList() ) +end + local function GetNumArguments( Func ) local Offset = 0 if IsType( Func, "table" ) then diff --git a/lua/shine/lib/codegen.lua b/lua/shine/lib/codegen.lua index 18cdb4cb6..2a43361d2 100644 --- a/lua/shine/lib/codegen.lua +++ b/lua/shine/lib/codegen.lua @@ -76,7 +76,7 @@ CodeGen.GenerateTemplatedFunction = GenerateTemplatedFunction local function GenerateFunctionWithArguments( FunctionCode, NumArguments, ChunkName, ... ) Shine.TypeCheck( FunctionCode, "string", 1, "GenerateFunctionWithArguments" ) Shine.TypeCheck( NumArguments, "number", 2, "GenerateFunctionWithArguments" ) - Shine.TypeCheck( ChunkName, { "string", "nil" }, 3, "GenerateFunctionWithArguments" ) + Shine.TypeCheck( ChunkName, { "function", "string", "nil" }, 3, "GenerateFunctionWithArguments" ) local Arguments = { "" } if NumArguments < Huge then @@ -89,8 +89,14 @@ local function GenerateFunctionWithArguments( FunctionCode, NumArguments, ChunkN local ArgumentsList = TableConcat( Arguments, ", " ) local ArgumentsWithoutPrefix = TableConcat( Arguments, ", ", 2 ) + local FinalChunkName + if type( ChunkName ) == "function" then + FinalChunkName = ChunkName( NumArguments ) + else + FinalChunkName = ChunkName + end - return GenerateTemplatedFunction( FunctionCode, ChunkName, { + return GenerateTemplatedFunction( FunctionCode, FinalChunkName, { Arguments = ArgumentsList, FunctionArguments = ArgumentsWithoutPrefix }, ... ) @@ -140,7 +146,7 @@ local NO_ARGS = {} ]] function CodeGen.MakeFunctionGenerator( Options ) local Args = Shine.TypeCheckField( Options, "Args", { "table", "nil" }, "Options" ) or NO_ARGS - local ChunkName = Shine.TypeCheckField( Options, "ChunkName", { "string", "nil" }, "Options" ) + local ChunkName = Shine.TypeCheckField( Options, "ChunkName", { "function", "string", "nil" }, "Options" ) local NumArgs = Shine.TypeCheckField( Options, "NumArgs", { "number", "nil" }, "Options" ) or #Args local Template = Shine.TypeCheckField( Options, "Template", "string", "Options" ) From 92e19f8a2fc36eeddb6b4306a4ed50fba08e8aba Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 14 Apr 2020 13:27:43 +0100 Subject: [PATCH 42/49] Remove flooring on hook callback priority There's no reason for priorities to be integers, and this allows extension callbacks to keep their -19.5 priority. --- lua/shine/core/shared/extensions.lua | 6 +++++- lua/shine/core/shared/hook.lua | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lua/shine/core/shared/extensions.lua b/lua/shine/core/shared/extensions.lua index e7e00f08c..64462b6b1 100644 --- a/lua/shine/core/shared/extensions.lua +++ b/lua/shine/core/shared/extensions.lua @@ -482,6 +482,10 @@ do return Callers[ select( "#", ... ) ]( self.Plugin, self.Plugin[ self.Event ], self.Event, ... ) end + function EventCaller:__tostring() + return StringFormat( "Shine.Plugins.%s:%s()", self.Plugin:GetName(), self.Event ) + end + local PluginEventKeys = {} local PluginEvents = setmetatable( {}, { __index = function( self, Key ) @@ -504,7 +508,7 @@ do PluginEvents[ Plugin ]:Add( Event ) - Hook.Add( Event, Key, EventCaller( Plugin, Event ), Hook.MAX_PRIORITY + 1 ) + Hook.Add( Event, Key, EventCaller( Plugin, Event ), Hook.MAX_PRIORITY + 0.5 ) end RemovePluginHook = function( Plugin, Event ) diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index 04ad451a4..b80f8ee4c 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -10,7 +10,6 @@ local Clamp = math.Clamp local DebugGetInfo = debug.getinfo local DebugGetMetaTable = debug.getmetatable local DebugSetUpValue = debug.setupvalue -local Floor = math.floor local Huge = math.huge local IsCallable = Shine.IsCallable local IsType = Shine.IsType @@ -123,7 +122,7 @@ local function Add( Event, Index, Function, Priority ) Shine.TypeCheck( Priority, "number", 4, "Add" ) end - Priority = Clamp( Floor( Priority or DEFAULT_PRIORITY ), MAX_PRIORITY, MIN_PRIORITY ) + Priority = Clamp( Priority or DEFAULT_PRIORITY, MAX_PRIORITY, MIN_PRIORITY ) -- If this index has already been used, replace it. local Nodes = HookNodes[ Event ] From 603301233a9e116757538d29cfd8d06fc9f47d9f Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 14 Apr 2020 16:10:31 +0100 Subject: [PATCH 43/49] Use a single key for all hooks on a given plugin --- lua/shine/core/shared/extensions.lua | 14 +++------- lua/shine/core/shared/hook.lua | 40 +++++++++++++--------------- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/lua/shine/core/shared/extensions.lua b/lua/shine/core/shared/extensions.lua index 64462b6b1..f76b57296 100644 --- a/lua/shine/core/shared/extensions.lua +++ b/lua/shine/core/shared/extensions.lua @@ -500,11 +500,8 @@ do AddPluginHook = function( Plugin, Event ) if not IsType( Plugin[ Event ], "function" ) then return end - local Keys = PluginEventKeys[ Event ] or {} - PluginEventKeys[ Event ] = Keys - - local Key = Keys[ Plugin ] or EventKey( Plugin ) - Keys[ Plugin ] = Key + local Key = PluginEventKeys[ Plugin ] or EventKey( Plugin ) + PluginEventKeys[ Plugin ] = Key PluginEvents[ Plugin ]:Add( Event ) @@ -512,11 +509,7 @@ do end RemovePluginHook = function( Plugin, Event ) - local Key = PluginEventKeys[ Event ] and PluginEventKeys[ Event ][ Plugin ] - if Key then - Hook.Remove( Event, Key ) - PluginEventKeys[ Event ][ Plugin ] = nil - end + Hook.Remove( Event, PluginEventKeys[ Plugin ] ) end RemoveAllPluginHooks = function( Plugin ) @@ -526,6 +519,7 @@ do RemovePluginHook( Plugin, Event ) end + PluginEventKeys[ Plugin ] = nil PluginEvents[ Plugin ] = nil end end diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index b80f8ee4c..d67843441 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -36,32 +36,30 @@ local HooksByEventAndIndex = {} -- Known event names. local KnownEvents = Shine.Set() -local OnError = Shine.BuildErrorHandler( "Hook error" ) -local Hooks -do - if not Shine.SetupExtensionEvents then - Shine.SetupExtensionEvents = function() end - end +-- Placeholder until the extensions file is loaded. +if not Shine.SetupExtensionEvents then + Shine.SetupExtensionEvents = function() end +end - Hooks = setmetatable( {}, { - -- On first call/addition of an event, setup the necessary data structures. - __index = function( self, Event ) - KnownEvents:Add( Event ) +local OnError = Shine.BuildErrorHandler( "Hook error" ) +local Hooks = setmetatable( {}, { + -- On first call/addition of an event, setup the necessary data structures. + __index = function( self, Event ) + KnownEvents:Add( Event ) - local HooksByIndex = LinkedList() - HookNodes[ Event ] = {} - HooksByEventAndIndex[ Event ] = {} + local HooksByIndex = LinkedList() + HookNodes[ Event ] = {} + HooksByEventAndIndex[ Event ] = {} - -- Save the list on the table to avoid invoking this again. - self[ Event ] = HooksByIndex + -- Save the list on the table to avoid invoking this again. + self[ Event ] = HooksByIndex - -- Allow extensions to setup their own events. - Shine:SetupExtensionEvents( Event ) + -- Allow extensions to setup their own events. + Shine:SetupExtensionEvents( Event ) - return HooksByIndex - end - } ) -end + return HooksByIndex + end +} ) -- Sort nodes by priority. Equal priority nodes will be placed in insertion order -- as the linked list will insert before the first node that is strictly after the From 5b161dead2a1b58362b1871ac1e90d35cc556796 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Tue, 14 Apr 2020 16:48:58 +0100 Subject: [PATCH 44/49] Remove unused BroadcastCallback field on hooks --- lua/shine/core/shared/hook.lua | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index d67843441..70a842fe2 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -134,8 +134,7 @@ local function Add( Event, Index, Function, Priority ) local Node = Callbacks:InsertByComparing( { Priority = Priority, Index = Index, - Callback = Function, - BroadcastCallback = Function + Callback = Function }, NodeComparator ) -- Remember this node for later removal. @@ -208,7 +207,7 @@ do for Node in Callbacks:IterateNodes() do local Entry = Node.Value - local Success = xpcall( Entry.BroadcastCallback, OnError{Arguments} ) + local Success = xpcall( Entry.Callback, OnError{Arguments} ) if not Success then -- If the error came from calling extension events, don't remove the hook -- (though it should never happen). From 990116a809f9b4e762f02162a2d2890e4f97ff06 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Wed, 15 Apr 2020 10:57:27 +0100 Subject: [PATCH 45/49] Remove unused ExtensionIndex --- lua/shine/core/shared/hook.lua | 29 ++++++++------------------- test/core/shared/hook.lua | 36 ++++++++++++++-------------------- 2 files changed, 23 insertions(+), 42 deletions(-) diff --git a/lua/shine/core/shared/hook.lua b/lua/shine/core/shared/hook.lua index 70a842fe2..0e617228c 100644 --- a/lua/shine/core/shared/hook.lua +++ b/lua/shine/core/shared/hook.lua @@ -26,9 +26,6 @@ local LinkedList = Shine.LinkedList local Hook = {} Shine.Hook = Hook --- A unique identifier for the callback that calls plugin hooks. -local ExtensionIndex = setmetatable( {}, { __tostring = function() return "CallExtensionEvent" end } ) - -- A mapping of event -> hook index -> hook node. local HookNodes = {} -- A mapping of event -> hook index -> hook callback (for external use). @@ -148,7 +145,7 @@ do -- See the comment in codegen.lua for the reasoning of this seemingly bizarre way of calling hooks. -- In a nutshell, it's to avoid LuaJIT traces aborting with NYI when handling varargs. local Callers = CodeGen.MakeFunctionGenerator( { - Template = [[local Shine, Hooks, OnError, Remove, ExtensionIndex = ... + Template = [[local Shine, Hooks, OnError, Remove = ... return function( Event{Arguments} ) local Callbacks = Hooks[ Event ] @@ -156,14 +153,9 @@ do local Entry = Node.Value local Success, a, b, c, d, e, f = xpcall( Entry.Callback, OnError{Arguments} ) if not Success then - -- If the error came from calling extension events, don't remove the hook - -- (though it should never happen). - if Entry.Index ~= ExtensionIndex then - Shine:DebugPrint( "[Hook Error] %s hook '%s' failed, removing.", - true, Event, Entry.Index ) + Shine:DebugPrint( "[Hook Error] %s hook '%s' failed, removing.", true, Event, Entry.Index ) - Remove( Event, Entry.Index ) - end + Remove( Event, Entry.Index ) elseif a ~= nil then return a, b, c, d, e, f end @@ -179,7 +171,7 @@ do -- This should equal the largest number of arguments seen by a hook to avoid lazy-generation which can impact -- compilation results. InitialSize = 10, - Args = { Shine, Hooks, OnError, Remove, ExtensionIndex }, + Args = { Shine, Hooks, OnError, Remove }, OnFunctionGenerated = function( NumArguments, Caller ) Hook[ StringFormat( "CallWith%dArg%s", NumArguments, NumArguments == 1 and "" or "s" ) ] = Caller end @@ -201,7 +193,7 @@ Hook.Call = Call local Broadcast do local Broadcasters = CodeGen.MakeFunctionGenerator( { - Template = [[local Shine, Hooks, OnError, Remove, ExtensionIndex = ... + Template = [[local Shine, Hooks, OnError, Remove = ... return function( Event{Arguments} ) local Callbacks = Hooks[ Event ] @@ -209,14 +201,9 @@ do local Entry = Node.Value local Success = xpcall( Entry.Callback, OnError{Arguments} ) if not Success then - -- If the error came from calling extension events, don't remove the hook - -- (though it should never happen). - if Entry.Index ~= ExtensionIndex then - Shine:DebugPrint( "[Hook Error] %s hook '%s' failed, removing.", - true, Event, Entry.Index ) + Shine:DebugPrint( "[Hook Error] %s hook '%s' failed, removing.", true, Event, Entry.Index ) - Remove( Event, Entry.Index ) - end + Remove( Event, Entry.Index ) end end end]], @@ -228,7 +215,7 @@ do ) end, InitialSize = 10, - Args = { Shine, Hooks, OnError, Remove, ExtensionIndex }, + Args = { Shine, Hooks, OnError, Remove }, OnFunctionGenerated = function( NumArguments, Caller ) Hook[ StringFormat( "BroadcastWith%dArg%s", NumArguments, NumArguments == 1 and "" or "s" ) ] = Caller end diff --git a/test/core/shared/hook.lua b/test/core/shared/hook.lua index 968195f41..a05954843 100644 --- a/test/core/shared/hook.lua +++ b/test/core/shared/hook.lua @@ -119,9 +119,7 @@ UnitTest:Test( "SetupGlobalHook - Replaces functions only once", function( Asser NewHookedFunction() - Assert.DeepEquals( "Calling the replaced function should run the hook", { - { ArgCount = 0 } - }, Callback.Invocations ) + Assert.Called( "Calling the replaced function should run the hook", Callback ) end ) local NestedName = "TestHolder"..os.time() @@ -149,9 +147,7 @@ UnitTest:Test( "SetupGlobalHook - Handles nested global values", function( Asser -- Ignores the 5th argument as the number of arguments is the max of the original and replacement. NewHookedFunction( 1, 2, 3, 4, 5 ) - Assert.DeepEquals( "Calling the replaced function should run the hook", { - { ArgCount = 4, 1, 2, 3, 4 } - }, Callback.Invocations ) + Assert.Called( "Calling the replaced function should run the hook", Callback, 1, 2, 3, 4 ) end ) _G[ NestedName ] = nil @@ -188,9 +184,7 @@ UnitTest:Test( "SetupClassHook - Replaces functions only once", function( Assert local Arg = {} NewHookedFunction( Arg, 1, 2 ) - Assert.DeepEquals( "Calling the replaced function should run the hook", { - { ArgCount = 2, Arg, 1 } - }, Callback.Invocations ) + Assert.Called( "Calling the replaced function should run the hook", Callback, Arg, 1 ) end ) local HOOK_RETURN_VALUE = "HOOK_RETURN_VALUE" @@ -282,10 +276,10 @@ for i = 1, #HookModeTests do ) if TestCase.ShouldCallOriginalOnHookReturn then - Assert.Equals( "Should have called the original function", 1, OriginalFunction:GetInvocationCount() ) + Assert.CalledTimes( "Should have called the original function", OriginalFunction, 1 ) else - Assert.Equals( - "Should not have called the original function", 0, OriginalFunction:GetInvocationCount() + Assert.CalledTimes( + "Should not have called the original function", OriginalFunction, 0 ) end end @@ -304,10 +298,10 @@ for i = 1, #HookModeTests do _G[ GlobalKey ]() ) if TestCase.ShouldCallOriginalOnHookNil then - Assert.Equals( "Should have called the original function", 1, OriginalFunction:GetInvocationCount() ) + Assert.CalledTimes( "Should have called the original function", OriginalFunction, 1 ) else - Assert.Equals( - "Should not have called the original function", 0, OriginalFunction:GetInvocationCount() + Assert.CalledTimes( + "Should not have called the original function", OriginalFunction, 0 ) end end @@ -339,10 +333,10 @@ for i = 1, #HookModeTests do ) if TestCase.ShouldCallOriginalOnHookReturn then - Assert.Equals( "Should have called the original function", 1, OriginalFunction:GetInvocationCount() ) + Assert.CalledTimes( "Should have called the original function", OriginalFunction, 1 ) else - Assert.Equals( - "Should not have called the original function", 0, OriginalFunction:GetInvocationCount() + Assert.CalledTimes( + "Should not have called the original function", OriginalFunction, 0 ) end end @@ -361,10 +355,10 @@ for i = 1, #HookModeTests do _G[ ClassName ][ ClassKey ]() ) if TestCase.ShouldCallOriginalOnHookNil then - Assert.Equals( "Should have called the original function", 1, OriginalFunction:GetInvocationCount() ) + Assert.CalledTimes( "Should have called the original function", OriginalFunction, 1 ) else - Assert.Equals( - "Should not have called the original function", 0, OriginalFunction:GetInvocationCount() + Assert.CalledTimes( + "Should not have called the original function", OriginalFunction, 0 ) end end From b932d68e8887088bd5e960daa3ad15a062275383 Mon Sep 17 00:00:00 2001 From: Person8880 Date: Wed, 15 Apr 2020 10:58:23 +0100 Subject: [PATCH 46/49] Fix error when printing tables with ctypes ffi.istype returns true for ctypes as well as cdata, which breaks if the colour or vector ctypes end up in a table that's being printed (e.g. if they're in a stack trace). --- lua/shine/lib/table.lua | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lua/shine/lib/table.lua b/lua/shine/lib/table.lua index 066af315d..ff428b867 100644 --- a/lua/shine/lib/table.lua +++ b/lua/shine/lib/table.lua @@ -380,6 +380,7 @@ do local StringFormat = string.format local StringLower = string.lower local StringRep = string.rep + local StringStartsWith = string.StartsWith local TableConcat = table.concat local tonumber = tonumber local tostring = tostring @@ -429,17 +430,20 @@ do return SafeToString( Value ) end, cdata = function( Value ) - local Success, IsType = pcall( FFIIsType, "Color", Value ) - if Success and IsType then - return StringFormat( "Colour( %s, %s, %s, %s )", Value.r, Value.g, Value.b, Value.a ) - end + -- Hack to detect ctypes, which pass the istype call... + if not StringStartsWith( SafeToString( Value ), "ctype<" ) then + local Success, IsType = pcall( FFIIsType, "Color", Value ) + if Success and IsType then + return StringFormat( "Colour( %s, %s, %s, %s )", Value.r, Value.g, Value.b, Value.a ) + end - Success, IsType = pcall( FFIIsType, "Vector", Value ) - if Success and IsType then - return StringFormat( "Vector( %s, %s, %s )", Value.x, Value.y, Value.z ) + Success, IsType = pcall( FFIIsType, "Vector", Value ) + if Success and IsType then + return StringFormat( "Vector( %s, %s, %s )", Value.x, Value.y, Value.z ) + end end - return tostring( Value ) + return SafeToString( Value ) end } From 15697545e4d6ca2aea6ad5cb1c9abb9978f20daa Mon Sep 17 00:00:00 2001 From: Person8880 Date: Wed, 15 Apr 2020 11:04:10 +0100 Subject: [PATCH 47/49] Add tests for extension loading --- test/core/shared/extensions.lua | 163 ++++++++++++++++++++++++++++++++ test/test_init.lua | 4 + 2 files changed, 167 insertions(+) create mode 100644 test/core/shared/extensions.lua diff --git a/test/core/shared/extensions.lua b/test/core/shared/extensions.lua new file mode 100644 index 000000000..b367a640d --- /dev/null +++ b/test/core/shared/extensions.lua @@ -0,0 +1,163 @@ +--[[ + Extension system tests. +]] + +local UnitTest = Shine.UnitTest +local IsType = Shine.IsType +local Hook = Shine.Hook + +local OldDebugPrint = Shine.DebugPrint +local OldAddErrorReport = Shine.AddErrorReport +local OldAddNotification = Shine.SystemNotifications.AddNotification + +function Shine:DebugPrint() end +function Shine:AddErrorReport() end +function Shine.SystemNotifications:AddNotification() end + +-- Forget the hook to ensure it's not added immediately. +Hook.Clear( "OnTestEvent" ) + +local TestPlugin = Shine.Plugin( "test" ) +Shine:RegisterExtension( "test", TestPlugin ) +Shine.AllPluginsArray[ #Shine.AllPluginsArray + 1 ] = "test" + +TestPlugin.HasConfig = true +TestPlugin.LoadConfig = UnitTest.MockFunction() + +local OnTestEvent = UnitTest.MockFunction() +function TestPlugin:OnTestEvent( Arg1, Arg2, Arg3 ) + return OnTestEvent( self, Arg1, Arg2, Arg3 ) +end + +function TestPlugin:ClientConnect( Client ) + +end + +UnitTest:Before( function() + OnTestEvent:Reset() + + TestPlugin.LoadConfig:Reset() + TestPlugin.LoadConfig:SetImplementation( nil ) + TestPlugin.Initialise = nil + + TestPlugin.Conflicts = nil + TestPlugin.DependsOnPlugins = nil +end ) + +UnitTest:Test( "EnableExtension - Fails if the given plugin does not exist", function( Assert ) + local Loaded, Err = Shine:EnableExtension( "test2" ) + Assert.False( "Should not have loaded a non-existent plugin", Loaded ) + Assert.Equals( "Error should be due to the plugin not existing", "plugin does not exist", Err ) +end ) + +UnitTest:Test( "EnableExtension - Fails if the plugin has a conflict", function( Assert ) + TestPlugin.Conflicts = { + DisableUs = { "basecommands" } + } + + local Loaded, Err = Shine:EnableExtension( "test" ) + Assert.False( "Should not have loaded due to a conflict", Loaded ) + Assert.Equals( "Error should be due to the conflict", "unable to load alongside 'basecommands'.", Err ) + Assert.Nil( "Should not be marked as enabled", TestPlugin.Enabled ) +end ) + +UnitTest:Test( "EnableExtension - Fails if the plugin is missing a dependency", function( Assert ) + TestPlugin.DependsOnPlugins = { "test2" } + + local Loaded, Err = Shine:EnableExtension( "test" ) + Assert.False( "Should not have loaded due to a missing dependency", Loaded ) + Assert.Equals( "Error should be due to the missing dependency", "plugin depends on 'test2'", Err ) + Assert.Nil( "Should not be marked as enabled", TestPlugin.Enabled ) +end ) + +UnitTest:Test( "EnableExtension - Fails if loading the config fails", function( Assert ) + TestPlugin.LoadConfig:SetImplementation( function() error( "failed to load config", 0 ) end ) + + local Loaded, Err = Shine:EnableExtension( "test" ) + Assert.False( "Should not have loaded due to an error loading config", Loaded ) + Assert.Equals( "Error should be due to the thrown error", "Error while loading config: failed to load config", Err ) + Assert.Nil( "Should not be marked as enabled", TestPlugin.Enabled ) +end ) + +UnitTest:Test( "EnableExtension - Fails if initialising throws an error", function( Assert ) + TestPlugin.Initialise = function() + error( "failed to initialise", 0 ) + end + + local Loaded, Err = Shine:EnableExtension( "test" ) + Assert.False( "Should not have loaded due to an error in Initialise", Loaded ) + Assert.Equals( "Error should be due to the thrown error", "Lua error: failed to initialise", Err ) + Assert.Nil( "Should not be marked as enabled", TestPlugin.Enabled ) +end ) + +UnitTest:Test( "EnableExtension - Fails if Initialise returns false", function( Assert ) + TestPlugin.Initialise = function() + return false, "unable to load" + end + + local Loaded, Err = Shine:EnableExtension( "test" ) + Assert.False( "Should not have loaded due to Initialise returning false", Loaded ) + Assert.Equals( "Error should be the returned failure message", "unable to load", Err ) + Assert.Nil( "Should not be marked as enabled", TestPlugin.Enabled ) +end ) + +local function HasPluginHook( Hooks ) + local Found + for Key, Callback in pairs( Hooks ) do + if + IsType( Key, "table" ) and Key.Plugin == TestPlugin and + IsType( Callback, "table" ) and Callback.Plugin == TestPlugin + then + return true + end + end + return false +end + +UnitTest:Test( "EnableExtension - Adds hooks as expected", function( Assert ) + local Loaded, Err = Shine:EnableExtension( "test" ) + Assert.True( Err, Loaded ) + Assert.True( "Plugin should be marked as enabled", TestPlugin.Enabled ) + Assert.CalledTimes( "Should have called LoadConfig on the plugin", TestPlugin.LoadConfig, 1 ) + + local Hooks = Hook.GetTable() + Assert.Nil( "Should not have any hooks for OnTestEvent yet", Hooks.OnTestEvent ) + Assert.IsType( "Should have ClientConnect hooks", Hooks.ClientConnect, "table" ) + Assert.True( "Expected ClientConnect hook callback for enabled plugin", HasPluginHook( Hooks.ClientConnect ) ) +end ) + +UnitTest:Test( "SetupExtensionEvents - Adds hooks as expected", function( Assert ) + Hook.Broadcast( "OnTestEvent", 1, 2, 3 ) + + local Hooks = Hook.GetTable() + Assert.IsType( "Should have OnTestEvent hooks", Hooks.OnTestEvent, "table" ) + Assert.True( "Expected OnTestEvent hook callback for enabled plugin", HasPluginHook( Hooks.OnTestEvent ) ) + Assert.Called( "Should have invoked the plugin method with expected args", OnTestEvent, TestPlugin, 1, 2, 3 ) +end ) + +UnitTest:Test( "UnloadExtension - Removes hooks as expected", function( Assert ) + local Unloaded = Shine:UnloadExtension( "test" ) + Assert.True( "Should have successfully unloaded the plugin", Unloaded ) + Assert.False( "Plugin should be marked as disabled", TestPlugin.Enabled ) + + Unloaded = Shine:UnloadExtension( "test" ) + Assert.False( "Should not be able to unload a plugin twice", Unloaded ) + + local Hooks = Hook.GetTable() + Assert.IsType( "Should have ClientConnect hooks", Hooks.ClientConnect, "table" ) + Assert.False( "Expected ClientConnect hook callback to be removed", HasPluginHook( Hooks.ClientConnect ) ) + + Assert.IsType( "Should have OnTestEvent hooks", Hooks.OnTestEvent, "table" ) + Assert.False( "Expected OnTestEvent hook callback to be removed", HasPluginHook( Hooks.OnTestEvent ) ) + + Hook.Broadcast( "OnTestEvent", 1, 2, 3 ) + Assert.CalledTimes( "Should not have invoked the plugin method after it is disabled", OnTestEvent, 0 ) +end ) + +Shine:UnloadExtension( "test" ) +Shine.Plugins.test = nil +Shine.AllPluginsArray[ #Shine.AllPluginsArray ] = nil + +Shine.DebugPrint = OldDebugPrint +Shine.AddErrorReport = OldAddErrorReport +Shine.SystemNotifications.AddNotification = OldAddNotification diff --git a/test/test_init.lua b/test/test_init.lua index 2ea29cb30..1dec9263e 100644 --- a/test/test_init.lua +++ b/test/test_init.lua @@ -111,6 +111,10 @@ do return #self.Invocations end + function MockFunction:SetImplementation( Impl ) + self.Impl = Impl + end + function MockFunction:__call( ... ) self.Invocations[ #self.Invocations + 1 ] = { ArgCount = select( "#", ... ), ... } if self.Impl then From 8728b7f9b9e4a5b00362a541fb3f5e92512f07fa Mon Sep 17 00:00:00 2001 From: Person8880 Date: Wed, 15 Apr 2020 11:13:09 +0100 Subject: [PATCH 48/49] Add test for plugin hook error handling --- test/core/shared/extensions.lua | 46 ++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/test/core/shared/extensions.lua b/test/core/shared/extensions.lua index b367a640d..b7058d02d 100644 --- a/test/core/shared/extensions.lua +++ b/test/core/shared/extensions.lua @@ -29,10 +29,27 @@ function TestPlugin:OnTestEvent( Arg1, Arg2, Arg3 ) return OnTestEvent( self, Arg1, Arg2, Arg3 ) end +function TestPlugin:OnErrorEvent() + error( "failed", 0 ) +end + function TestPlugin:ClientConnect( Client ) end +local function HasPluginHook( Hooks ) + local Found + for Key, Callback in pairs( Hooks ) do + if + IsType( Key, "table" ) and Key.Plugin == TestPlugin and + IsType( Callback, "table" ) and Callback.Plugin == TestPlugin + then + return true + end + end + return false +end + UnitTest:Before( function() OnTestEvent:Reset() @@ -101,18 +118,27 @@ UnitTest:Test( "EnableExtension - Fails if Initialise returns false", function( Assert.Nil( "Should not be marked as enabled", TestPlugin.Enabled ) end ) -local function HasPluginHook( Hooks ) - local Found - for Key, Callback in pairs( Hooks ) do - if - IsType( Key, "table" ) and Key.Plugin == TestPlugin and - IsType( Callback, "table" ) and Callback.Plugin == TestPlugin - then - return true +UnitTest:Test( "Errors in hooks should disable the extension automatically", function( Assert ) + local Loaded, Err = Shine:EnableExtension( "test" ) + Assert.True( Err, Loaded ) + Assert.True( "Plugin should be marked as enabled", TestPlugin.Enabled ) + + for i = 1, 10 do + Hook.Broadcast( "OnErrorEvent" ) + if i < 10 then + Assert.Equals( "Should have incremented the plugin's hook errors", i, TestPlugin.__HookErrors ) end end - return false -end + + Assert.False( "Should have disabled the plugin due to errors", TestPlugin.Enabled ) + Assert.Equals( "Should have reset the hook error counter", 0, TestPlugin.__HookErrors ) + + local Hooks = Hook.GetTable() + Assert.IsType( "Should have OnErrorEvent hooks", Hooks.OnErrorEvent, "table" ) + Assert.False( "Expected OnErrorEvent hook callback to be removed", HasPluginHook( Hooks.OnErrorEvent ) ) +end ) + +Hook.Clear( "OnErrorEvent" ) UnitTest:Test( "EnableExtension - Adds hooks as expected", function( Assert ) local Loaded, Err = Shine:EnableExtension( "test" ) From a7c443eb5c3e3b69d765c4ffb51372bc1b3e49fc Mon Sep 17 00:00:00 2001 From: Person8880 Date: Wed, 15 Apr 2020 12:02:25 +0100 Subject: [PATCH 49/49] Optimise resetting of hooks in EnableExtension Avoid needing to check every plugin if no plugins have hooked into a given event or the newly enabled plugin has no method for it. --- lua/shine/core/shared/extensions.lua | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lua/shine/core/shared/extensions.lua b/lua/shine/core/shared/extensions.lua index f76b57296..6e52eeebe 100644 --- a/lua/shine/core/shared/extensions.lua +++ b/lua/shine/core/shared/extensions.lua @@ -423,6 +423,7 @@ local function CheckDependencies( self, Dependencies ) end local AddPluginHook +local HasPluginHook local RemovePluginHook local RemoveAllPluginHooks do @@ -496,6 +497,7 @@ do return Events end } ) + local EventsWithPlugins = Shine.Multimap() AddPluginHook = function( Plugin, Event ) if not IsType( Plugin[ Event ], "function" ) then return end @@ -504,12 +506,18 @@ do PluginEventKeys[ Plugin ] = Key PluginEvents[ Plugin ]:Add( Event ) + EventsWithPlugins:Add( Event, Plugin ) Hook.Add( Event, Key, EventCaller( Plugin, Event ), Hook.MAX_PRIORITY + 0.5 ) end + HasPluginHook = function( Event ) + return EventsWithPlugins:Get( Event ) ~= nil + end + RemovePluginHook = function( Plugin, Event ) Hook.Remove( Event, PluginEventKeys[ Plugin ] ) + EventsWithPlugins:RemoveKeyValue( Event, Plugin ) end RemoveAllPluginHooks = function( Plugin ) @@ -551,10 +559,18 @@ do } ) end - local function ResetPluginHooks() + local function ResetPluginHooks( PluginBeingLoaded ) local Events = Hook.GetKnownEvents() for i = 1, #Events do - Shine:SetupExtensionEvents( Events[ i ] ) + if IsType( PluginBeingLoaded[ Events[ i ] ], "function" ) then + if HasPluginHook( Events[ i ] ) then + -- If there's at least one plugin hooked to this event, re-add all plugins in the correct order. + Shine:SetupExtensionEvents( Events[ i ] ) + else + -- Otherwise just add the new plugin. + AddPluginHook( PluginBeingLoaded, Events[ i ] ) + end + end end end @@ -646,7 +662,7 @@ do -- Reset all hooks to maintain a consistent calling order. -- Loading an extension is a rare event, so the cost of this is acceptable. - ResetPluginHooks() + ResetPluginHooks( Plugin ) -- We need to inform clients to enable the client portion. if Server and Plugin.IsShared and not self.GameIDs:IsEmpty() then