How to make it so players cant collide with NPCs?

You are using PhysicsService a bit wrong, here i made a working example of how you could implement it

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

PhysicsService:CreateCollisionGroup('Players')
PhysicsService:CreateCollisionGroup('NPCs')
PhysicsService:CollisionGroupSetCollidable('Players', 'NPCs', false)

local NPC = workspace.Model

local function SetModelGroup(Model, Group)
	
	for _, Part in ipairs(Model:GetChildren())
	do
		if (Part:IsA 'BasePart') then
		
			PhysicsService:SetPartCollisionGroup(Part, Group)
		end
	end
end

local function OnClientAdded(Client)
	
	local function OnCharacterLoaded(Character)
		
		SetModelGroup(Character, 'Players')
	end
	
	Client.CharacterAppearanceLoaded:Connect(OnCharacterLoaded)
end

SetModelGroup(NPC, 'NPCs')

Players.PlayerAdded:Connect(OnClientAdded)

In my case it loops through children although you can make it loop through GetDescendants if need it, for players it is unnecessary.

The script creates 2 collision groups, NPCs and Players, they are then set to not be collideable. You don't need to create new collision groups for every itteration in a loop as you did, it's redundant.

You can call SetModelGroup on any model, second parameter is name of the collision group.

I strongly suggest using CharacterAppearanceLoaded event which is similar to CharacterAdded but it fires when your accessories are loaded, in this case there is a lot higher chance that body parts are going to be loaded.

If you are having problem that some parts are not yet replicated, you can try using DescendantAdded event on the character and set collision group for invididual parts of the character when they replicate.

Here are all the sources i used

• DescendantAdded
• GetDescendants
• CharacterAppearanceLoaded
• PhysicsService