My player collision module

It’s mostly copied from Detecting Collisions | Roblox Creator Documentation, but I rewrote it.

It turns off collisions between players. I would like to see whether I write modules (in general) the right way.

I’m especially worried about the local variables there, that are not inside the playerCollision table (like PhysicsService, or onPlayerAdded, onCharacterAdded), will that clutter the namespace of the script that requires this module? Should I put them inside playerCollision?

Here is the code

local playerCollision = {}

local PhysicsService = game:GetService("PhysicsService")
local Players = game:GetService("Players")

PhysicsService:CreateCollisionGroup("Players")
PhysicsService:CollisionGroupSetCollidable("Players", "Players", false)

local previousCollisionGroups = {}

function playerCollision.setCollisionGroup(object)
	if object:IsA("BasePart") then
		previousCollisionGroups[object] = object.CollisionGroupId
		PhysicsService:SetPartCollisionGroup(object, "Players")
	end
	for _,descendant in pairs(object:GetDescendants()) do
		if descendant:IsA("BasePart") then
			PhysicsService:SetPartCollisionGroup(descendant, "Players")
		end
	end
end

function playerCollision.resetCollisionGroup(object)
	if object:IsA("BasePart") then
		local previousCollisionGroupId = previousCollisionGroups[object]
		if not previousCollisionGroupId then return end 

		local previousCollisionGroupName = PhysicsService:GetCollisionGroupName(previousCollisionGroupId)
	end
end

function playerCollision.run()
	local function onCharacterAdded(character)
		playerCollision.setCollisionGroup(character)
		
		character.DescendantAdded:Connect(playerCollision.setCollisionGroup)
		character.DescendantRemoving:Connect(playerCollision.resetCollisionGroup)
	end

	local function onPlayerAdded(player)
		player.CharacterAdded:Connect(onCharacterAdded)
	end

	Players.PlayerAdded:Connect(onPlayerAdded)
end
return playerCollision

Edit: No replies yet

Putting local variables outside of the module part of the code is actually perfectly fine and probably better (since you aren’t re-defining them every time you call a function)

I will note that any players that are in the server before module.run() is ran won’t actually get affected (since the Players.PlayerAdded) won’t be trigged. This can be solved by a simple for loop.

	local function onPlayerAdded(player)
		player.CharacterAdded:Connect(onCharacterAdded)
	end

	Players.PlayerAdded:Connect(onPlayerAdded)
    for _,plr in ipairs(Players:GetPlayers()) do
        onPlayerAdded(plr)
    end
1 Like

Doesn’t it clutter the namespace though?

If you’re using the same function over and over again I think it’s much much better to define functions outside the scope, however at the end of the day the same problems can be solved by multiple different solutions. So you can write code however you see it best.

Does it clutter the namespace? Meaning if I require that module in another script, could I somehow overwrite these variables from there?

personally I would change this into

function playerCollision.resetCollisionGroup(object)
    if object:IsA("BasePart") and previousCollisionGroups[object] then
        local previousCollisionGroupName = PhysicsService:GetCollisionGroupName(previousCollisionGroupId)
    end
end

I need to define this variable first which is what I do in those lines.

I also need to check whether the object even had a collision group before trying to set it to its previous group.

function playerCollision.resetCollisionGroup(object)
    local previousCollisionGroupId = previousCollisionGroups[object]

    if object:IsA("BasePart") and previousCollisionGroupId then
        local previousCollisionGroupName = PhysicsService:GetCollisionGroupName(previousCollisionGroupId)
    end
end

my bad

ah my bad for misunderstanding.
If you write the code to override those variables in the module then yes

local something = "test" -- applies to all requires modules

local module = {}
module.internalValue = "Hi" -- seperate for each require
function module.set(value)
    something = value -- set it for all modules
end
function module.get()
    return something
end
return module;

No, I mean, if I require my player collision module from a script, and then make a function in that script, called onPlayerAdded, will that break the script?