I think it would be helpful to restart this post and state what you’re needing. Initially it looked like you were asking if you needed a single remote or multiple.
To, hopefully, help answer your first question… that would really come down to preference and your needs. Having multiple remotes makes it easy to have a subscription/publication model, but you can always do the same thing with a single one, it’s just more difficult to implement and probably not worth your time. Using folders to separate remotes by, say, task might prove helpful in organization. If you’re planning to have a lot of tasks and the idea of organizing all of these remotes seems unwieldy then maybe consider coming up with a way to do it all through one. I would also consider the code overhead in using just one in that case. It wouldn’t be complicated, per se, but it might get a bit convoluted.
A question for you though would be, what are you using all of these remotes for? Are you simply using them as a means of alerting the player of the new task? Depending on use, you may be able to pretty easily get away with just one remote without all of the convoluted if/else logic going on.
As for the code you have here, I’m curious what you’re trying to achieve with it. Based on the code it looks like you’re trying to link all of the tasks in the TaskData table to a player.
One thing I notice off the bat is this block of code:
for i, v in pairs(TaskData) do
PlayerTasks[player.UserId] = {}
table.insert(PlayerTasks[player.UserId], i)
return
end
Here you’re reassigning PlayerTasks[player.UserId] each time the loop runs, I don’t think that’s what you desire here. You should probably move that line outside of that loop. As it is right now, each time that loop runs you’ll be losing every entry in that table since you reassign it to an empty table.
The second thing I notice is that you have:
for i, v in pairs(TaskData) do
for _, currentTask in pairs(PlayerTasks[player.UserId]) do
-- Check if task is already assigned
if i == currentTask then return end
end
table.insert(PlayerTasks[player.UserId], i)
return
end
Since you’re executing the AssignTask function multiple times this is likely not functioning the way you think. When currentTask is in the PlayerTasks[player.UserId] table, you actually return and drop out of this function entirely. This is guaranteed to stop both of your loops here, which I’m guessing is also not desired here. I would change this to something like:
for i, v in pairs(TaskData) do
local playerHasTask = false
for _, currentTask in pairs(PlayerTasks[player.UserId]) do
-- Check if task is already assigned
if i == currentTask then
playerHasTask = true
break
end
end
if not playerHasTask then
table.insert(PlayerTasks[player.UserId], i)
return
end
end
As for the return statement at the end of your loop here. I’m curious what the intention is for it. Is it used to only add one task at a time? I believe that would be the current functionality. Since you’re calling this AssignTask function multiple times I’m going to assume that’s the desired functionality.
Here’s what I’m guessing you’re wanting:
function TaskService:AssignTask(player)
if PlayerTasks[player.UserId] then
print(#PlayerTasks[player.UserId], unpack(PlayerTasks[player.UserId]))
if #PlayerTasks[player.UserId] >= 3 then return end
print(6343737)
for i, v in pairs(TaskData) do
local playerHasTask = false
for _, currentTask in pairs(PlayerTasks[player.UserId]) do
-- Check if task is already assigned
if i == currentTask then
playerHasTask = true
break
end
end
if not playerHasTask then
table.insert(PlayerTasks[player.UserId], i)
return
end
end
else
print('else')
PlayerTasks[player.UserId] = {}
for i, v in pairs(TaskData) do
table.insert(PlayerTasks[player.UserId], i)
return
end
end
end
Hope this was helpful.
Here’s another implementation where you actually give the function the task to assign.
--- Assigns a task to a given player
--
-- @param player Roblox Player object
-- @string taskName The name of the task to be assigned
-- @param task The task object to assign.
-- @return Returns true of the player didn't have the task already
-- otherwise it returns false.
function TaskService:AssignTask(player, taskName, task)
PlayerTasks[player.UserId] = PlayerTasks[player.UserId] or {}
if (not PlayerTasks[player.UserId][taskName]) then
PlayerTasks[player.UserId][taskName] = task;
return true
end
return false
end
Depending on your needs this should work pretty well for you. It uses an object instead of an array for much more efficient task lookups. It will add the task if it doesn’t already exist, if it adds a new task it returns true, otherwise it returns false. It also has the added benefit of giving you more control over the tasks being added.