Release Notes for 564

I opened some discussion on it but we’re probably not going to immediately revert the deprecation.

As you can see from the code I just posted BreakJoints is such a messy old function that there’s good reason for it to be deprecated. Though as you note from a practical perspective having some better API to work with joints would be quite useful. Maybe separating out break internal vs external joints for instance.

3 Likes

9/3/2023:

Someone asked me for what I did in that Doomspire clone in more detail. Here’s what I sent them…

My full solution for resolving performance issues was to collect all joints to be deleted into a big list, defer (if I didn’t already) and then destroy all the joints in one go at the end of the tick/frame.

I empirically found deferring before destroying joints to be the most performant. The hunch I had that led me to try deferring to resolve the big lag spikes I was getting was that in the microprofiler I saw assemblies were getting recalculated almost immediately after destroying joints (likely triggered by some kind of queries or accesses that I or the engine does after destroying the joints?). I figured maybe doing all the updates all at once could maybe reduce the amount of recalculations down to just one, which it certainly seemed to or at least seemed to simplify the work somehow as it did result in much better performance, and the microprofiler showed all those recalculations as one big bar immediately proceeding the deferred threads bar.

Presumably something must cause the engine to do recalculations after joints are destroyed even mid-tick. But deferring gets around this by doing all the destructions together at once with no interruptions, presumably resulting in only one recalculation of the assembly/model being done.

My implementation roughly looked like this:

local BreakJointsService = {}

local jointsToDestroy = {}
local destroyJointsThread
function BreakJointsService:_deferAndDestroy()
	-- If a joint destruction thread is already active, cancel
	if destroyJointsThread then
		return
	end

	-- Spawn a thread which will destroy the joints
	destroyJointsThread = task.defer(function()
		-- Clear the destroy joints thread so that another may be spawned
		destroyJointsThread = nil

		-- Destroy all the pending joints
		for _, joint in jointsToDestroy do
			joint:Destroy()
		end

		-- Replace the joints to destroy table with a brand new fresh table
		jointsToDestroy = {}
	end)
end

function BreakJointsService:_destroyJoints(instance: Instance)
	-- If the instance is not a BasePart, it doesn't have any joints to be destroyed
	if not instance:IsA("BasePart") then
		return
	end

	-- Get the joints of the part
	local joints = instance:GetJoints()

	-- Copy the joints into the joints destruction list
	table.move(joints, 1, #joints, #jointsToDestroy + 1, jointsToDestroy)

	-- Defer & destroy all pending joints
	self:_deferAndDestroy()
end

-- Breaks the joints of the given instance and all of its descendants
function BreakJointsService:BreakJoints(instance: Instance)
	self:_destroyJoints(instance)
	for _, instance in instance:GetDescendants() do
		self:_destroyJoints(instance)
	end
end

return BreakJointsService

My stance on ipairs/pairs has also changed to align with tnavarts’ a lot more. Generalized iteration does seem to result in more readable code with less noise.

My logic of communicating if something is an array via ipairs or pairs doesn’t really have a huge benefit when generalized iteration makes that irrelevant anyways, because the way that you mutate tables will make them clearly arrays or non-arrays, and any mutations that go against that aren’t really going to affect the iteration, so, it ends up being a bit redundant.

Ultimately, I have switched to using generalized iteration for the cleaner code, and because of the __iter metamethod. I did not value the __iter metamethod enough until recently, it’s pretty great.

Old post

Summary

I don’t think BreakJoints is getting deprecated? Nevermind, I guess I didn’t read enough haha.

You can do this for parts:

for _, joint in ipairs(part:GetJoints()) do
    joint:Destroy()
end

Model:BreakJoints() is less trivial but it would be pretty much the same thing, just over all BasePart descendants of the model.

Also note that unlike BreakJoints this would also break RigidConstraint and all other constraints for that matter. WeldConstraint is the only Constraint instance that will be broken by BreakJoints.

And a note on performance… In a little weird sci-fi themed doomspire clone I made, I actually had to re-implement BreakJoints myself because the engine method was causing ginormous lag spikes lasting 3-4 seconds long.

I narrowed the cause down to some sort of assembly recalculation thing that was always triggered immediately after a call to BreakJoints, and after re-implementing it myself, I get consistently (and noticeably) better performance particularly when destroying 100 or more joints. (Whereas iirc lower numbers of joints seemed to perform a little bit slower than BreakJoints but not to an unreasonable extent). I couldn’t tell you why that might be, but that was my observation, it was consistent, and, it took many hours to discover the root cause.

I have a strong (personal) preference for using ipairs and pairs because it explicitly indicates that I’m expecting and using a table, and it’s a little bit more specific about the kind of table operation I want to do. For example in that case I would use ipairs because I expect an array. Another problem I have with generalized iteration is that it is (sometimes) a bit more ambiguous about what is going on due to __iter. ipairs & pairs guarantee that I am looping over the data I want (the data in the table). I reserve my own use of generalized iteration to cases where that’s actually what I want, e.g. in APIs or modules that want abstract iterators and don’t care about table data.

But again, that’s just my preference, I just wanted to point it out because neither method is really objectively better than the other or anything. :smile:

3 Likes

FWIW I have literally the inverse take: I have a strong preference for the new generalized iteration because I want to know right away if there is the wrong kind of data in my table. I don’t want to use ipairs and have it silently mask the fact that there’s the wrong type of data in the table creating hard to debug issues.

I’ve definitely run into that in practice before where I pass a different table than I intended to ipairs and no errors get thrown but now my code now isn’t working right. Especially in cases where I have both a map and an array of the same data with the map acting as some kind of index.

7 Likes

Reposting here because I posted on the wrong release (this one might be wrong too because it was the update I just had):

This update appears to have broken the copy/paste feature with Studio. While it works specifically with Studio, the moment I paste code anywhere outside of Studio, it loses the indentation and has weird symbols instead:

image

Also sorry, I didn’t mean to reply to you specifically. Just realized that. I meant to reply to the main topic.

This appears to be fixed now.

1 Like

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.