What my code is suppose to do? When a player is close to another player and press the key r something happens
My current code works to a certain extent but not as I would like it to. The current code if I press the key r 3x in one second it only shows 1 time that I have pressed the R key
I do believe that the problem is the way I am checking if the r key is pressed
local players = game:GetService("Players")
local RunService = game:GetService("RunService")
local uiService = game:GetService("UserInputService")
local player = players.LocalPlayer
local radius = 10
local ePressed = false
game:GetService("UserInputService").InputBegan:connect(function(inputObject, gameProcessedEvent)
if inputObject.KeyCode == Enum.KeyCode.R then
ePressed = true
wait(1)
ePressed = false
end
end)
local debounce = true
uiService.InputBegan:Connect(function(key)
for i,v in pairs (players:GetChildren()) do
if v.Name ~= player.Name and debounce == true then
if (player.Character.HumanoidRootPart.Position-v.Character.HumanoidRootPart.Position).magnitude < radius and ePressed == true then
--Player is within radius
--Display billboard gui on player root part
debounce = false
print("Within radius!")
debounce = true
end
end
end
end)
Probably should add a debounce on whenever the key “R” is pressed. Also, you might want to put the debounce for the second event before you start the cycle.
Doesn’t make a difference provided no yielding methods have been called. Even if the event is triggered again it doesn’t get processed until something else yields.
Processing isn’t actually parallel here, it just quickly swaps to do the next job in the queue whenever the thread yields.
My advice would be to do a single InputBegan connection. There’s no reason to have them split.
Also if you’ve defined uiService then there’s no need to do GetService again for that first connection.
Minor stuff that won’t make much difference to performance at the moment but in terms of others understanding the code and maintainability it makes sense to only have 1 connection and to use your variables.
Edit: one extra minor thing. If you swap your conditions around in the second connection you can save processing. If the first term of an and condition is false, the rest don’t get evaluated, so put your ePressed comparison first, and then the magnitude check will only happen if ePressed is the right state. Even better, put the ePressed and debounce checks before you even open the for loop and you can save yourself the need to run any of it if either of them are false.
I think the solution is to do a single InputBegan (as BanTech suggested) and remove the debounce. So, something like this:
uiService.InputBegan:connect(function(inputObject, gameProcessedEvent)
if inputObject.KeyCode ~= Enum.KeyCode.R then return end
--Put the block of code that checks which players are close.
end)
------
Since the problem is that repeated presses are sunk, the cause of it would be the debounce. Removing it will solve this problem. *However,* I do not suggest that you remove the debounce as you will not want multiple threads running in parallel doing the same task -- interfering with each other.
Additionally, I want to point out, you have a wait here:
if inputObject.KeyCode == Enum.KeyCode.R then
ePressed = true
wait(1)
ePressed = false
end
I think this is also part of the problem since you have no debounce on this event, multiple threads are running, switching this true and false at random. Thus, causing some cases where there should’ve of been print being sunk.
Thank you for giving me feed back @utrain & @BanTech. I took it into consideration. Here is the updated version
local players = game:GetService("Players")
local RunService = game:GetService("RunService")
local uiService = game:GetService("UserInputService")
local player = players.LocalPlayer
local my_Player_char = player.Character
local radius = 10
local withinPlayerDistance = false
game:GetService("UserInputService").InputBegan:connect(function(inputObject, gameProcessedEvent)
if inputObject.KeyCode == Enum.KeyCode.R and withinPlayerDistance == true then
print("R have been clicked and the player was with in distance of another player")
withinPlayerDistance = true
wait(1)
withinPlayerDistance = false
end
end)
local debounce = true
local showingBbGUI = false
uiService.InputBegan:Connect(function(key)
local getPlayers = players:GetPlayers()
for i,v in pairs(getPlayers) do
if v.Name ~= player.Name then
if (my_Player_char.HumanoidRootPart.Positionv.Character.HumanoidRootPart.Position).magnitude < radius then
withinPlayerDistance = true
--Within Distance
if showingBbGUI == false then
showingBbGUI = true
local billBoardGui = player.PlayerGui.BbGuiPressE
billBoardGui.Adornee = v.Character.HumanoidRootPart
billBoardGui.Enabled = true
print("Billboard Gui is set")
end
else
withinPlayerDistance = false
showingBbGUI = false
local billBoardGui = player.PlayerGui.BbGuiPressE
billBoardGui.Adornee = nil
billBoardGui.Enabled = false
--Not within Distance anymore/or is not in distance of a player
end
end
end
end)
Hey there! I’d like to bring to attention what are, in my eyes, a few pitfalls of this revision.
I highly advise you keep your codebase modular. Very modular. Should you make a function that does a simple assignment? Not really, but don’t be afraid of using them as aliases to rename a certain function.
Don’t worry about prefixing variables with “my”, if you wanted to, prefix every script with your copyright license! (Joking aside, it’s redundant. It’s a great habit to get yourselves out of)
Keep your code consistent, adopt a style guide. This will really iron out a lot of issues in the long run. (Keep services either PascalCaseI advise doing this! or camelCase; pick one. Keep function and variable names snake_case, camelCaseI advise this too!, or PascalCase; pick one).
I made a rendition of your snippet given to demonstrate these practices. Realistically, you’d probably make utility modules to aid in these functions across your entire game. When possible and within reason, keep your code DRY—Don’t Repeat Yourself.
local Players = game:GetService("Players")
local RunService = game:GetService("RunService")
local UserInputService = game:GetService("UserInputService")
--[[
Controls whether or not this LocalScript should emit debugging messages.
@type {boolean}
]]
local DEBUG = true
--[[
The radius (in studs) to be observed by this LocalScript from the LocalPlayer.
@readonly
@type {integer}
]]
local RADIUS = 10
--[[
The gui element to be expressed when a player enters the given radius.
@readonly (this variable alone should never be in the left side of an assignment statement)
(This value's properties are still mutable, so we shouldn't use LOUD_SNAKE_CASE.
@type {BillboardGui}
]]
local eGuiElement = Players.LocalPlayer.PlayerGui.PressEGui
--[[
A hoisted variable that stores another player that is currently closest and selected to the local player.
Once selected, this value will not change until said player leave's the local player's radius.
@type {Player?}
]]
local selectedPlayer
--[[
Retrieves every player except for the local player.
@returns {Player[]}
]]
local function getOtherPlayers()
local players = Players:GetPlayers()
local otherPlayers = {}
for _, player in ipairs(players) do
if player.Name ~= Players.LocalPlayer.Name then
table.insert(otherPlayers, player)
end
end
return otherPlayers
end
--[[
Calculates the distance between two player's models.
@param {Player} playerOne
@param {Player} playerTwo
@returns {number}
]]
local function calculateDistanceBetweenPlayers(playerOne, playerTwo)
local playerOnePosition = playerOne.Character.HumanoidRootPart.Position
local playerTwoPosition = playerTwo.Character.HumanoidRootPart.Position
return (playerOnePosition - playerTwoPosition).Magnitude
end
--[[
Retrieves every player whose character is within a radius.
@returns {Player[]}
]]
local function getOtherPlayersInRadius()
local otherPlayers = getOtherPlayers()
local otherPlayersInRadius = {}
for _, otherPlayer in ipairs(otherPlayers) do
local distance = calculateDistanceBetweenPlayers(Players.LocalPlayer, otherPlayer)
if distance < RADIUS then
table.insert(otherPlayersInRadius, otherPlayer)
end
end
return otherPlayersInRadius
end
--[[
A function that returns the closest player within a radius.
If no player is close, returns nil.
@returns {Player?}
]]
local function getClosestPlayerInRadius()
local otherPlayersInRadius = getOtherPlayersInRadius()
local closestDistance = math.huge
local closestPlayer
for _, player in ipairs(otherPlayersInRadius) do
local distance = calculateDistanceBetweenPlayers(Players.LocalPlayer, player)
if distance < closestDistance then
closestDistance = distance
closestPlayer = player
end
end
return closestPlayer
end
--[[
Returns if the gui elements are currently enabled.
(An alias for an expression whose purpose isn't immediately evident)
@returns {boolean}
]]
local function isEGuiElementEnabled()
return eGuiElement.Adornee ~= nil
end
--[[
Enables the local player's "PressEGui" and attaches it to the target player's character.
@param {Player} targetPlayer
@returns {nil}
]]
local function enableEGuiElement(targetPlayer)
eGuiElement.Adornee = targetPlayer.Character.HumanoidRootPart
eGuiElement.Enabled = true
if DEBUG then
local message = ("Billboard GUI element set to %q"):format(targetPlayer.Name)
print(message)
end
end
--[[
Unattaches the local player's "PressEGui" and disables it.
@returns {nil}
]]
local function disableEGuiElement()
eGuiElement.Adornee = nil
eGuiElement.Enabled = false
end
--[[
Sets the selected player to targetPlayer and updates the coresponding GUI elements.
If targetPlayer is nil, disables and unattaches the corresponding GUI elements.
@param {Player?} targetPlayer
@returns {nil}
]]
local function setSelectedPlayer(targetPlayer)
if targetPlayer then
if not isEGuiElementEnabled() then
enableEGuiElement(targetPlayer)
end
elseif isEGuiElementEnabled() then
disableEGuiElement()
end
selectedPlayer = targetPlayer
end
--[[
Updates the selected player. Does no actions if the currently selected player is still valid.
@returns {nil}
]]
local function updateSelectedPlayer()
-- Check the selected player is still valid
if selectedPlayer then
local distance = calculateDistanceBetweenPlayers(Players.LocalPlayer, selectedPlayer)
local isValid = distance <= RADIUS
if not isValid then
-- Set selectedPlayer to nil directly as opposed to using setSelectedPlayer
-- to force the function to "fall through" to the next "case", similar to a
-- switch statement.
selectedPlayer = nil;
end
end
if not selectedPlayer then
local closestPlayerInRadius = getClosestPlayerInRadius()
setSelectedPlayer(closestPlayerInRadius)
end
end
--[[
Event listener for when the local player's "R" key is pressed.
@returns {nil}
]]
local function onRKeyPressed()
if selectedPlayer then
if DEBUG then
local message = ("R has been clicked while another player, %q, was within the radius."):format(selectedPlayer.Name)
print(message)
end
end
end
--[[
Event listener for when the local player supples input.
@param {InputObject} input
@returns {nil}
]]
local function onInputBeganListener(input)
if input.KeyCode == Enum.KeyCode.R then
updateSelectedPlayer()
onRKeyPressed(selectedPlayer)
end
end
UserInputService.InputBegan:Connect(onInputBeganListener)
I recommend copying and pasting this into your IDE of choice.
If you have any questions about the rationale behind something, ask! This isn’t the best style, since style is subjective, it just shows how consistency and modularity can bring clarity to your code.
I have just tested your code and I believe there are some missing parameters (line 68)
for _, otherPlayer in ipairs(otherPlayers) do
if calculateDistanceBetweenPlayers(Players.LocalPlayer, otherPlayer) < RADIUS then --Tryed to fix it by passing thorough Players.LocalPlayer, otherPlayer
table.insert(otherPlayersInRadius, otherPlayer)
end
end
And line 108
--Where you are calling this function you are not passing a parameter and it seem to be a number which I don't quite understand yet
local function enableEGuiElement(targetPlayer)
eGuiElement.Adornee = targetPlayer.Character.HumanoidRootPart
eGuiElement.Enabled = true
if DEBUG then
local message = ("Billboard GUI element set to %q"):format(targetPlayer.Name)
print(message)
end
end
Other then that I do appreciate your replay this is truly mind opening on different ways of scripting
You’re absolutely correct as to my type mismatch. The revised code for that function is as follows (I’ll edit my last post to reflect this as well):
--[[
Retrieves every player whose character is within a radius.
@returns {Player[]}
]]
local function getOtherPlayersInRadius()
local otherPlayers = getOtherPlayers()
local otherPlayersInRadius = {}
for _, otherPlayer in ipairs(otherPlayers) do
-- Previously forgot to supply both player parameters
local distance = calculateDistanceBetweenPlayers(Players.LocalPlayer, otherPlayer)
if distance < RADIUS then
table.insert(otherPlayersInRadius, otherPlayer)
end
end
return otherPlayersInRadius
end
And the other fix:
--[[
Sets the selected player to targetPlayer and updates the coresponding GUI elements.
If targetPlayer is nil, disables and unattaches the corresponding GUI elements.
@param {Player?} targetPlayer
@returns {nil}
]]
local function setSelectedPlayer(targetPlayer)
if targetPlayer then
if not isEGuiElementEnabled() then
-- Previously forgot to supply the 'targetPlayer' parameter.
enableEGuiElement(targetPlayer)
end
elseif isEGuiElementEnabled() then
disableEGuiElement()
end
selectedPlayer = targetPlayer
end
I previously have entered those parameter but that did not fix this problem:
I can’t understand why it is a number because when I try to print the parameter targetPlayer it prints a number
This error is coming from the function enableEGuiElement() --line 109
local function enableEGuiElement(targetPlayer)
print(targetPlayer)
eGuiElement.Adornee = targetPlayer.Character.HumanoidRootPart
eGuiElement.Enabled = true
if DEBUG then
local message = ("Billboard GUI element set to %q"):format(targetPlayer.Name)
print(message)
end
end
Yikes! This is why I’m much more comfortable writing code I can write a spec for. I believe I found the issue, though:
getClosestPlayerInRadius didn’t send the closest player, it sent the closest distance, thus the rogue number.
I rebuilt getClosestPlayerInRadius to be much more sensible:
--[[
A function that returns the closest player within a radius.
If no player is close, returns nil.
@returns {Player?}
]]
local function getClosestPlayerInRadius()
local otherPlayersInRadius = getOtherPlayersInRadius()
local closestDistance = math.huge
local closestPlayer
for _, player in ipairs(otherPlayersInRadius) do
local distance = calculateDistanceBetweenPlayers(Players.LocalPlayer, player)
if distance < closestDistance then
closestDistance = distance
closestPlayer = player
end
end
return closestPlayer
end
I am truly enjoying reading the code the way that you coded it
But I have this question…if you wanted the gui to be visible upon approach to the nearest player not upon pressing the r key how would you have gona to write it? I am assuming you still would have used a function that loops like so as I did in my code
local uiService = game:GetService("UserInputService")
uiService.InputBegan:Connect(function()
end)
And then upon press of a key and near a player then some action happens
