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

[The-Lord-of-Owls]

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

[Aws0mee]

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]

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]

Whats one use for this outside of debugging

[Aws0mee]

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]

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

[The-Lord-of-Owls]

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.

[Aws0mee]

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.

[The-Lord-of-Owls]

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]

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

[StarLight-Oliver]

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

[The-Lord-of-Owls]

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

The wiki says that, tho when looking at the actual hook.lua file it doesn't seem to be the case unless I'm misreading this. It looks like if the id of the hook is anything other than a string it would call ErrorNoHaltWithStack on line 34. So I don't see why checking for a string would be an issue if that the only one it sees as a valid type to add to the hook table.

It would probably be useful to have it instead convert those to strings rather than throwing an error. It might also be useful to return the string if that happens too just so the dev is aware of it happening but I've not thought of that till now so I'm unsure what unforeseen issues or overhead that would cause. But given its just for hook.Add it probably wouldn't cause too much issues or break things.

I'll play around with the idea of converting to string from other types and returning the converted ID from hook.Add. I'm not sure if I should include that in this pull request or not however. I'll let somebody else tell me first if I should or make a separate pull

https://github.com/Facepunch/garrysmod/blob/master/garrysmod/lua/includes/modules/hook.lua#L23-L42