Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented hook.Exists and hook.Temporary function into the hook module #2092

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

The-Lord-of-Owls
Copy link

Summary:
Implemented a simple hook.Exists() function to check if a hook exists or not. I've verified that this function works as intended and that the code is consistent with what is already in the hook module. Args are validated just the same as with other functions already in the module.

Why this was implemented:
This would be a much cleaner and straightforward way to check if a hook exists instead of having to do a work around such as hook.GetTable()[ event_name ][ name ]

Name: Exists
Args: string event_name, string name
Desc: Returns if the hook exists or not

Usage:

//Check if hook doesn't exist yet
if ( not hook.Exists( "InitPostEntity", "test" ) ) then
	//Add the hook if it doesn't exist
	hook.Add( "InitPostEntity", "test", function()
		print( "howdy!" )
	end )
end

//Check if the hook exists now
if ( hook.Exists( "InitPostEntity", "test" ) ) then
	print( "hook exists" )	//This will print because the hook exists now
end

Implemented a simple hook.Exists function to check if a hook exists or not.

Purpose:
A cleaner way to check if a hook exists instead of having to do something ugly like ( hool.GetTable()[ "event" ][ "identifier" ] )
@Aws0mee
Copy link
Contributor

Aws0mee commented Jul 5, 2024

I don't understand how

if ( hook.Exists( "InitPostEntity", "test" ) ) then
	print( "hook exists" )	//This will print because the hook exists now
end

is better than

if hook.GetTable().InitPostEntity.test then
	print( "hook exists" )	//This will print because the hook exists now
end

It might be a little cleaner, but all of those validation checks will likely make it a little slower too. I don't think this addition is 100% needed, but I guess it wouldn't really hurt to add.

@Zaurzo
Copy link
Contributor

Zaurzo commented Jul 5, 2024

I don't understand how

if ( hook.Exists( "InitPostEntity", "test" ) ) then
	print( "hook exists" )	//This will print because the hook exists now
end

is better than

if hook.GetTable().InitPostEntity.test then
	print( "hook exists" )	//This will print because the hook exists now
end

It might be a little cleaner, but all of those validation checks will likely make it a little slower too. I don't think this addition is 100% needed, but I guess it wouldn't really hurt to add.

That will error if no InitPostEntity hooks were added

@garryspins
Copy link

Whats one use for this outside of debugging

@Aws0mee
Copy link
Contributor

Aws0mee commented Jul 5, 2024

I don't understand how

if ( hook.Exists( "InitPostEntity", "test" ) ) then
	print( "hook exists" )	//This will print because the hook exists now
end

is better than

if hook.GetTable().InitPostEntity.test then
	print( "hook exists" )	//This will print because the hook exists now
end

It might be a little cleaner, but all of those validation checks will likely make it a little slower too. I don't think this addition is 100% needed, but I guess it wouldn't really hurt to add.

That will error if no InitPostEntity hooks were added

Pretty sure every server will have at least 1 InitPostEntity hook, but you can just check for that too if you want.

local hooks = hook.GetTable()
if hooks.InitPostEntity and hooks.InitPostEntity.test then
	print( "hook exists" )	//This will print because the hook exists now
end

@The-Lord-of-Owls
Copy link
Author

The use case I've primarily been using this for is when dealing with temporary hooks that are created on an as needed basis that may depend on other hooks existing alongside them, or may need to skip parts of their logic while one of these temporary hooks are in use. This has been really beneficial to dealing with hooks that run very often such as HUDPaint or Think/Tick. Namely with server wide events where something temporarily changes but immediately reverts back the moment one of these temporary hooks is removed.

It becomes especially beneficial when having to check if multiple hooks exist at a time

Added hook..Temporary() function to hook module.

Purpose: Provides a quick and simple way to create hooks that will remove themselves after a certain amount of times being ran.

Args: string hook_event, any identifier, integer max_runs, function func

Note: If only event_name, name and max_runs are provided, it will assume that the max_runs is the hook functions and that this hook should only be ran once before removing itself. This is a failsafe
@The-Lord-of-Owls
Copy link
Author

The-Lord-of-Owls commented Jul 8, 2024

UPDATE TO PULL REQUEST

I have also added my hook.Temporary() function to hook module. Perhaps also providing that for others may help see some reasons for the other function I added to exist as well.

NAME: hook.Temporary()
ARGS: String event_name, String identifier, Integer/Function max_runs, Function func

PURPOSE: With the hook.Temporary() function you can now specify a predetermined amount of times the hook should run before removing itself. when the hook runs it imposes very minimal overhead as it is only increasing a local variable by 1, and performing a >= check on two variables to determine when the hook should be removed

NOTE: If only event_name, name and max_runs are provided, it will assume that the max_runs is the hook function and that this hook should only be ran once before removing itself. This makes it rather simple to to write onetime use hooks or limited use hooks depending on needs of the addon developer using them.

EXAMPLE 1

This example below demonstrates the use of both addons as a way of determining how many rounds a gamemode should run a match for before doing a map change for the next game. I'm sure these two functions together will see a lot of use. I've found temporary hooks to be very useful in my projects and I think having these two functions added to the game would help make a lot of things easier for addon creators by having the logic for it behind a simple built in function to do it.

--This hook will run 25 times before removing itself
--It will return the winner of the round to where it can be displayed for all players
--If the hook has removed itself then our code knows that it is time to do a map transition to the next map
hook.Temporary( "EndRound", "CheckWinner", 25, function( redTeamScore, blueTeamScore )
	if redTeamScore > blueTeamScore then
		return redTeamScore
	elseif redTeamScore < blueTeamScore then
		return blueTeamScore
	else
		return redTeamScore == blueTeamScore
	end
end )

--Determine if our gamemode should run another round
hook.Add( "ShouldStartRound", "CheckRounds", function()
	return hook.Exists( "EndRound", "CheckWinner" ) or false
end)

local nextMatchMap = "gm_flatgrass_soccer"
function StartRound()
	local shouldStart = hook.Call( "ShouldStartRound" )

	if shouldStart then
		hook.Call( "StartRound" )
	else
		RunConsoleCommand( nextMatchMap )
	end
end

EXAMPLE 2

The hook.Temporary() function is also useful during server startup and handling things such as loading config files and spawning things in. This example shows how to make a one time running hook on InitPostEntity

hook.Temporary( "InitPostEntity", "SpawnEntities", function()
	--Some code that spawns entities at server startup
	--Maybe also load a config file from the data folder
	--Perhaps even perform an sql query to a database on startup
	--What ever this hook does, will run once and then remove itself
end )

USECASES

  1. Startup hooks used to handle configuration and will no longer be needed after that
  2. Hooks used that should only be called a certain amount of times and can be used to determine when something else should happen if used in conjunction with hook.Exists
  3. Allowing to easily have dynamic changes to occur based on the existence or lack of existence of a hook
  4. Allow for temporary features on a moments notice without having to constantly have every hook loaded into the hook table at all times.

@The-Lord-of-Owls The-Lord-of-Owls changed the title Implemented hook.Exists function into the hook module Implemented hook.Exists and hook.Temporary function into the hook module Jul 8, 2024
@Aws0mee
Copy link
Contributor

Aws0mee commented Jul 8, 2024

I don't quite understand the use-case of InitPostEntity hooks. It should only be called once anyway, so it doesn't really matter if the hook is removed right after or not. Also you could already do this:

hook.Add("InitPostEntity", "something", function()
	print("do stuff")
	hook.Remove("InitPostEntity", "something")
end)

I'll admit, your addition is a little cleaner as it removes the need to remove your hook manually, but again I don't really see the point here.

I don't really understand the use-case of hooks that run for a set amount of times either. I understand sometimes you want to add a hook to modify something for a specific player/entity/whatever and then remove it when that player/entity/whatever is removed or leaves the game, but I don't really understand a use case where you would only want to run a hook for x amount of times.

Also, again this feature is really simple to add yourself, it only takes 3 extra lines of code. We can use your example

hook.Temporary( "EndRound", "CheckWinner", 25, function( redTeamScore, blueTeamScore )
	if redTeamScore > blueTeamScore then
		return redTeamScore
	elseif redTeamScore < blueTeamScore then
		return blueTeamScore
	else
		return redTeamScore == blueTeamScore
	end
end )

Without your addition, this functionality can easily be added like this.

local n = 0
hook.Add( "EndRound", "CheckWinner", function( redTeamScore, blueTeamScore )
	if n >= 25 then hook.Remove( "EndRound", "CheckWinner") return end
	if redTeamScore > blueTeamScore then
		return redTeamScore
	elseif redTeamScore < blueTeamScore then
		return blueTeamScore
	else
		return redTeamScore == blueTeamScore
	end
	n = n + 1
end )

I personally feel like the use-cases are too niche and the functionality added are both too small to add to the base game. I'm not trying to be a hater or anything, but I honestly think these additions would be best suited for a third-party library. Maybe there's a use-case I'm missing out on though.

I'd also like to add that using your functions will be slightly cleaner, but will also have slightly more overhead. Adding one of your temporary hooks will keep track of the times the hook is called as it should, but now we're also calling isfunction() multiple times on the same function since hook.Add will also call it, as well as a couple validation checks and assignments in your function.

Utilized the same logic as hook.Add and hook.Remove inside hook.Temporary to reduce overhead from double validating args
@The-Lord-of-Owls
Copy link
Author

In regards to what you said about the isfunction validation happening multiple times. I've made a slight change and moved the same logic from hook.Add and hook.Remove into the hook.Temporary function. So now validation on event_name, name, max_runs and func should only occur once.

So the overhead when adding the hook and the overhead when removing it should no longer be an issue.

@garryspins
Copy link

garryspins commented Jul 9, 2024

I actually really like hook.Temporary, although i dont think its useful enough to be in std

@robotboy655 robotboy655 added the Addition The pull request adds new functionality. label Jul 9, 2024
@StarLight-Oliver
Copy link
Contributor

Another issue with this, is that the second argument to hook.Add isn't a string, its any. so your hook.Exists doesn't work for all possible hooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Addition The pull request adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants