Merge pull request #116 from christopher-buss/feature/janitor-connections

perf: cleanup player connections

Author: christopher-buss

src/server/player/character/character-service.ts

@@ -139,9 +139,9 @@ export default class CharacterService implements OnStart, OnPlayerJoin {
 	}
 
 	private onRigLoaded(playerEntity: PlayerEntity, rig: CharacterRig): void {
-		const { name, player } = playerEntity;
+		const { name, janitor, player } = playerEntity;
 
-		addToCollisionGroup(rig, CollisionGroup.Character);
+		janitor.Add(addToCollisionGroup(rig, CollisionGroup.Character));
 		rig.AddTag(Tag.PlayerCharacter);
 		this.characterRigs.set(player, rig);
 
@@ -150,18 +150,22 @@ export default class CharacterService implements OnStart, OnPlayerJoin {
 		debug.profilebegin("Lifecycle_Character_Added");
 		{
 			for (const { id, event } of this.characterAddedEvents) {
-				Promise.defer(() => {
-					debug.profilebegin(id);
-					event.onCharacterAdded(rig, playerEntity);
-				}).catch(err => {
-					this.logger.Error(`Error in character lifecycle ${id}: ${err}`);
-				});
+				janitor
+					.Add(
+						Promise.defer(() => {
+							debug.profilebegin(id);
+							event.onCharacterAdded(rig, playerEntity);
+						}),
+					)
+					.catch(err => {
+						this.logger.Error(`Error in character lifecycle ${id}: ${err}`);
+					});
 			}
 		}
 
 		debug.profileend();
 
-		this.characterAppearanceLoaded(player, rig).catch(err => {
+		janitor.Add(this.characterAppearanceLoaded(player, rig)).catch(err => {
 			this.logger.Info(`Character appearance did not load for`, player, err);
 		});
 	}

src/server/player/player-service.ts

@@ -175,12 +175,16 @@ export default class PlayerService implements OnStart {
 		debug.profilebegin("Lifecycle_Player_Join");
 		{
 			for (const { id, event } of this.playerJoinEvents) {
-				Promise.defer(() => {
-					debug.profilebegin(id);
-					event.onPlayerJoin(playerEntity);
-				}).catch(err => {
-					this.logger.Error(`Error in player lifecycle ${id}: ${err}`);
-				});
+				janitor
+					.Add(
+						Promise.defer(() => {
+							debug.profilebegin(id);
+							event.onPlayerJoin(playerEntity);
+						}),
+					)
+					.catch(err => {
+						this.logger.Error(`Error in player lifecycle ${id}: ${err}`);
+					});
 			}
 		}
 

src/shared/util/physics-util.ts

@@ -4,8 +4,26 @@
  *
  * @param object - The instance to add to the collision group.
  * @param group - The collision group to add the object to.
+ * @param trackNewDescendants - Whether to handle future descendants of the
+ *   object to the collision group. Defaults to false.
+ * @returns A connection that will add any new descendants to the collision
+ *   group.
  */
-export function addToCollisionGroup(object: Instance, group: string): void {
+export function addToCollisionGroup(
+	object: Instance,
+	group: string,
+	trackNewDescendants?: true,
+): () => void;
+export function addToCollisionGroup(
+	object: Instance,
+	group: string,
+	trackNewDescendants?: false,
+): undefined;
+export function addToCollisionGroup(
+	object: Instance,
+	group: string,
+	trackNewDescendants = false,
+): (() => void) | undefined {
 	if (object.IsA("BasePart")) {
 		object.CollisionGroup = group;
 	}
@@ -15,4 +33,18 @@ export function addToCollisionGroup(object: Instance, group: string): void {
 			descendant.CollisionGroup = group;
 		}
 	}
+
+	if (!trackNewDescendants) {
+		return undefined;
+	}
+
+	const connection = object.DescendantAdded.Connect(descendant => {
+		if (descendant.IsA("BasePart")) {
+			descendant.CollisionGroup = group;
+		}
+	});
+
+	return () => {
+		connection.Disconnect();
+	};
 }