Refactor gmod_hands hooks

[sarahsturgeon]

Currently, gmod_hands creates a hook on OnViewModelChanged on both realms when initialized.

On our server, likely due to some addon conflict (ULX?), these hooks aren't cleaned up when the hands are deleted, leading to a massive pileup of hooks with entities as identifiers. Entity-identified hooks are supposed to be cleaned up by the hook system, so this isn't technically an issue with Gmod, but I don't see any reason not to use a string anyway.

This PR aims to address three issues:

1. Creating a hook with an entity as an identifier is technically less performant than using strings, as the entities will need to be validity checked every time the hook is run
2. gmod_hands currently do not handle their own hook cleanup
3. Hooks are created on every client, for every gmod_hands created. This is unnecessary. Because these directly relate to view models, only the client who owns the hands needs to care about them

Extra notes:

• I believe that the hooks do not need to be made serverside at all, but I've been instructed to leave them to maintain compatibility. If others feel strongly otherwise, it's an easy change to ignore the hook serverside

• While I was able to briefly test on the three major gamemodes (sandbox, darkrp, ttt) without issue, I'm unsure of how custom content might rely on the existing behavior, so if anyone has more insight on that, please share

• There is another check in ENT:OnViewModelChanged that I don't believe needs to be there anymore, but that won't hurt to leave in. I'll work on testing it with and without to make sure it won't break anything, but I'm opening the PR without that change because it'll work either way

• I believe the best way to address all of these issues succinctly is to make gmod_hands a clientside entity, but I didn't want to go overboard, so this is a middle-ground solution until a better one is proposed

• While addressing the problem of other players' hands adding hooks to each client, I added an ownership check to make sure that self:GetOwner() == LocalPlayer(). This is fine so long as the hands have their owner set properly before running :Spawn() on them. The default gamemodes do this properly by calling ENT:DoSetup( ply ), but anything else that relies on this without setting that ownership won't have the hooks added on any client (self:GetOwner() == nil, nil ~= LocalPlayer())

[GitSparTV]

When the hook is called they will be removed because they are not valid, does it still filling the RAM?

[Kefta]

@GitSparTV

On our server, likely due to some addon conflict (ULX?), these hooks aren't cleaned up when the hands are deleted, leading to a massive pileup of hooks with entities as identifiers.

[GitSparTV]

Oh lmao yeah ulx

[robotboy655]

This is trying to solve a problem only you have. You do realize even if we merge this, other addons using entities as identifiers will cause issues for you?

[robotboy655]

Hooks are created on every client, for every gmod_hands created. This is unnecessary. Because these directly relate to view models, only the client who owns the hands needs to care about them

It is necessary for spectators, even though I am not sure it works out of the box properly at the moment.

[Kefta]

I think we should be moving away from entity identifiers in the codebase since the hook system can only have one entity identifier per-entity per-hook, but gmod_hands should be refactored to be purely clientside anyway.

[thegrb93]

Doesn't seem to be ulib's problem. https://github.com/TeamUlysses/ulib/blob/master/lua/ulib/shared/hook.lua#L132

[thegrb93]

It does have a memory leak however. https://github.com/TeamUlysses/ulib/issues/67

[sarahsturgeon]

This is trying to solve a problem only you have. You do realize even if we merge this, other addons using entities as identifiers will cause issues for you?

Right, I hope I didn't imply that this would completely fix the problem I was facing. I identified the hands as being susceptible to this bug, figured they could use an update either way. Even if it's not with this PR.

Doesn't seem to be ulib's problem. https://github.com/TeamUlysses/ulib/blob/master/lua/ulib/shared/hook.lua#L132

Interesting. I'll start looking in to other suspects.