Skip to content

Commit

Permalink
Changed event guard
Browse files Browse the repository at this point in the history
  • Loading branch information
Sleitnick committed Sep 25, 2023
1 parent 9327dd0 commit 9520c61
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions modules/comm/Client/ClientRemoteProperty.lua
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,15 @@ function ClientRemoteProperty.new(
resolveOnReadyPromise = resolve
end)
self._changed = self._rs:Connect(function(value)
local changed = value ~= self._value
self._value = value
if not self._ready then
self._ready = true
resolveOnReadyPromise(value)
end
self.Changed:Fire(value)
if changed then
self.Changed:Fire(value)
end
end)

return self
Expand Down Expand Up @@ -131,9 +134,7 @@ end
]=]
function ClientRemoteProperty:Observe(observer: (any) -> ())
if self._ready then
task.defer(function()
observer(self._value)
end)
task.defer(observer, self._value)

This comment has been minimized.

Copy link
@ambergamefam

ambergamefam Sep 26, 2023

Contributor

This is incorrect. This needs to be:

        task.defer(function()
			observer(self._value)
		end)

If you call task.defer with the current value, it will emit that value in a deferred thread, regardless if the value changed between now and when the observer runs. An example of this is when the value changes multiple times when multiple client events get processed in serial.

For example:

  • The client runs and asks the server the ID of their house.
  • The server receives this request and returns the ID of their house. Coincidentally (or not), a property of their house changes twice.
  • There are now 3 events queued up to the client: House ID, and then the two property changes
  • The first event processes (house ID). After the client receives the house ID, they want to begin observing these properties. They connect a callback to the remote property. This defers a thread with an old value of this property.
  • The client then processes the next two events, changing this property. The callback to :Observe() fires twice with these values
  • The deferred thread resumes, with the old value as an argument.
end
return self.Changed:Connect(observer)
end
Expand Down

0 comments on commit 9520c61

Please sign in to comment.