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

Micro optimizations to player.GetBy* functions #2124

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

Conversation

joeyjumper94
Copy link
Contributor

@joeyjumper94 joeyjumper94 commented Sep 29, 2024

speed up player.GetBy____ID(ID) functions by relying primarily on indexing a table

speed up player.getby____ID(ID) functions by relying primarily on indexing a table
some fine tuning
@robotboy655 robotboy655 added the Enhancement The pull request enhances current functionality. label Oct 2, 2024
Copy link
Collaborator

@robotboy655 robotboy655 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this even necessary? I can't imagine these functions being used in such contexts where they'd cause serious performance issues, for the trade off of uglifying the code and introducing potential reliability issues.

Have you even tested these changes? I am seeing multiple issues just by skimming over the changes.

Not to mention the PR does not adhere to the contributing guidelines.

garrysmod/lua/includes/extensions/player.lua Show resolved Hide resolved
garrysmod/lua/includes/extensions/player.lua Outdated Show resolved Hide resolved
garrysmod/lua/includes/extensions/player.lua Outdated Show resolved Hide resolved
@robotboy655 robotboy655 changed the title Update player.lua Micro optimizations to player.GetBy* functions Oct 2, 2024
@robotboy655
Copy link
Collaborator

I have also just noticed that this is effectively duplicate of #2094, but with also affecting the other functions.

@joeyjumper94
Copy link
Contributor Author

here's the code i used to test my changes

for i=1,128 do
	local p=Entity(i)
	if!p:IsValid()then continue end
	if!p:IsPlayer()then break end
	MsgN"_____________________________________________\n"
	MsgN(p)
	local ID=p:AccountID()
	MsgN("player.GetByAccountID(",ID,")=",player.GetByAccountID(ID))
	local ID=p:SteamID()
	MsgN("player.GetBySteamID\"",ID,"\"=",player.GetBySteamID(ID))
	local ID=p:SteamID64()
	MsgN("player.GetBySteamID64\"",ID,"\"=",player.GetBySteamID64(ID))
	local ID=p:UniqueID()
	MsgN("player.GetByUniqueID\"",ID,"\"=",player.GetByUniqueID(ID))
end
MsgN"_____________________________________________\n"

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

Successfully merging this pull request may close these issues.

2 participants