GM hook calls #878

Merged
merged 1 commit into from Mar 26, 2015

Projects

None yet

4 participants

@TheFreeman193
Contributor

Cleans up gamemode hook calls using SendLua. Prevents errors in non-sandbox gamemodes.

  • All GM method calls via SendLua now use hook.Run
@willox
Collaborator
willox commented Feb 24, 2015

What's wrong with hook.Run?

@TheFreeman193
Contributor

Nothing as such, but it only gets the gamemode with gmod.GetGamemode and then passes everything onto hook.Call. It's good for module environments where the GAMEMODE global isn't directly accessible, but for anything in the standard environment, it's quicker and simpler just to use hook.Call.

@willox
Collaborator
willox commented Feb 24, 2015

The GAMEMODE global is completely useless for gamemode methods. Use self.

If you want your useless microseconds of speed at least get it right.

@TheFreeman193
Contributor

As far as I am aware, both work inside GM methods. You are right in that I should be using self, however; I'll swap it in a sec.

@willox
Collaborator
willox commented Feb 24, 2015

If I sound like an asshole it's because I don't like your change. I just don't understand why people favour worse code for such a small optimization.

The SendLua changes are fine (but could surely use hook.Run too heh).

@TheFreeman193
Contributor

Is there any particular benefit, other than that it looks a tad nicer? It's the sum of small differences that produces the biggest effects, and I doubt the interpreter (or users) give a shit how pretty the code is.

Saving the world, one line of ugly code at a time

@willox
Collaborator
willox commented Feb 24, 2015

Pretty code is important. If you want a faster game go optimize the widget system's PlayerTick hook. That's a real issue.

@TheFreeman193
Contributor

Readable code is important. An extra arg is worthwhile to avoid two needless calls, in my opinion.

-snip- base GM changes removed

@TheFreeman193
Contributor

Since I'm making changes to cl_init on the base GM in #865, I'm going to remove the changes from this one. While I'm at it, what are your thoughts on this, @robotboy655? hook.Run or hook.Call? There's no point in changes if noone approves.

@robotboy655
Collaborator

Before I look into this, remove all the whitespace changes from all your commits. It clogs up the things you actually want to change. I said numerous times that you shouldn't bother removing whitespace from entire file. I will do it myself once I deal with most of the PRs.

@robotboy655
Collaborator

Don't change hook.Run to hook.Call, but you may change it other way around.

@TheFreeman193
Contributor

I will do. Okay, that settles it.

Oh, and no hard feelings @wiox, I don't take things personally.

@TheFreeman193 TheFreeman193 GM hook calls
All GM method calls via SendLua now use hook.Run
f47677d
@TheFreeman193
Contributor

(Sorted & rebased)

@thegrb93
Contributor
thegrb93 commented Mar 2, 2015

I know it's not a part of this pr, but I hate the SendLua functions being used for hints. Slow and potentially exploitable. Luckily wiremod uses net messages for their e2 hints.

@TheFreeman193
Contributor

I'm not especially fond of it either. I reckon it'd be nice to have a global function serverside, using net to call hooks on clients, but this is something to think about after the update and when all these pulls have been sorted out.

@robotboy655 robotboy655 merged commit 5803c78 into garrynewman:master Mar 26, 2015
@TheFreeman193 TheFreeman193 deleted the TheFreeman193:gm-hook-calls branch Mar 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment