Make signals returned by GetPropertyChangedSignal return the Instance in the callback

As a developer it is currently impossible to reuse functions when dealing with signals from GetPropertyChangedSignal. This means that when trying to manipulate the Instance a signal originates from, new functions with at least one upvalue needs to be created, which can have a substantial impact on performance depending upon how many signals are being listened to.

At first glance, it might seem redundant to pass the Instance the signal came from as an argument to any listeners, since these signals are actually unique to that particular Instance. However, it would solve the problem of having to make new functions by letting them identify what Instance was being passed and removing the need for an upvalue.

For example, let’s say for some reason I wanted to listen to the Color property of some Parts and print their name when it changed. To do that currently, you have to do this:

local parts = {table of parts}
for _, v in pairs(parts) do
  v:GetPropertyChangedSignal("Color"):Connect(function()
    print(v.Name)
  end)
end

That’s a bunch of unique functions that only exist because of not being able to identify what’s been changed. If this were possible, the code would become:

local function printNameOnColorChanged(part)
  print(part.Name)
end

local parts = {table of parts}
for _, v in pairs(parts) do
  v:GetPropertyChangedSignal("Color"):Connect(printNameOnColorChanged)
end

This only uses one function and is thus a more efficient solution.

This is a bit of an abstract example, to demonstrate the point. A more noticeable and realistic use case would be making an Explorer GUI of some sort that mimicked the behavior of the Explorer widget in Studio. In order to keep the names of Instances displayed by the explorer up to date, you would need to listen for the Instance’s Name property changing, and adjust the text displayed by the GUI accordingly. This leads to quite a few unique functions, each with an associated upvalue. This change would prevent that issue entirely by making the callback for updating the text context free.

If this change were added, it would make my life easier because it would mean less optimizing in places where performance was important (like an Explorer GUI, which is honestly why I bring this up).

1 Like

I would consider that API clutter/inconsistency. You could apply this same logic to many other signals of many classes and the argument would still hold, but that doesn’t mean we should actually go ahead and do this for all of those cases (and if those signals already have a parameter, you can’t actually apply this at all, meaning you’d be forced to accept inconsistency anyhow).

I don’t think the added benefit (saving some Lua closures) is sufficiently high for actually making this change (and then only for this signal specifically, which would be weird in itself).

6 Likes

To add on, I’d much rather GetPropertyChangedSignal pass the new value as a parameter instead, it’s an incredibly more common use of the event.

2 Likes