Separate crate machine from market system

[GreaseMonk]

About the PR

• Crate machine now separated from market system
• Added MarketItemSpawnerComponent for spawning market items and moved ItemsToSpawn to it.
• No change in code other than splitting up code and adding more documentation

Why / Balance

• Allows crate machine to be used in other systems and makes it a reusable entity

How to test

Start a round and aghost
teleport to cargo depot
sell iron ore
buy iron ore at market console
check if animation is correct and items and crate spawn correctly
check if animation closes when moving crate off the crate hole.

Media

Requirements

• I have read and am following the Pull Request and Changelog Guidelines.
• I have added media to this PR or it does not require an ingame showcase.

Breaking changes

none

Changelog

[whatston3]

Looked through this - one issue and one small gripe.
I think you could fire off events from the animation system itself instead of using delegates, seems closer to ECS and less OO.
I also think the CrateMachineVisualState could be scoped outside of the component.

Looks fine skimming it, will test briefly.

[whatston3]

Put in a few small suggestions.

I would honestly recommend testing and pulling in https://github.com/new-frontiers-14/frontier-station-14/compare/rack-attack...whatston3:frontier-station-14:2025-01-20-rack-attack-suggestions?expand=1

Seems to work fine under modest tests.

Edit: The TransformSystem distance calc. is probably cleaner vs. Vector2, I'd recommend using that.

[whatston3]

Suggested change

	var currentDistance = CalculateDistance(compXform.Coordinates, Transform(from).Coordinates);
	var isTooFarAway = currentDistance > maxDistance;
	var isTooFarAway = Vector2.Distance(compXform.Coordinates.Position, Transform(from).Coordinates.Position) > maxDistance;

Vector2 has a convenient function for this, and TransformSystem does too. I believe the snippet above should work - I also wrong a version using TransformSystem, which is even better since it'll handle weird parenting better - no false positives.

[whatston3]

Seems odd to assign some conditions to variables when they're only used in an if statement but eh.

[whatston3]

Suggested change

	/// <summary>
	/// Calculates distance between two EntityCoordinates on the same grid.
	/// Used to check for cargo pallets around the console instead of on the grid.
	/// </summary>
	/// <param name="point1">the first point</param>
	/// <param name="point2">the second point</param>
	/// <returns></returns>
	private static double CalculateDistance(EntityCoordinates point1, EntityCoordinates point2)
	{
	var xDifference = point2.X - point1.X;
	var yDifference = point2.Y - point1.Y;
	return Math.Sqrt(xDifference * xDifference + yDifference * yDifference);
	}

Just use Vector2.Distance.

[whatston3]

Suggested change

[Dependency] private readonly IEntityManager _entityManager = default!;

Shouldn't need this - why isn't this extending SharedCrateMachineSystem? EntitySystem should already has a reference in EntityManager.

[whatston3]

Suggested change

	public sealed partial class CrateMachineSystem
	public sealed partial class CrateMachineSystem : SharedCrateMachineSystem

Extend the shared version, add the needed import above?

[whatston3]

Suggested change

	public sealed class CrateMachineSystem: SharedCrateMachineSystem
	public sealed partial class CrateMachineSystem : SharedCrateMachineSystem

per https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/partial-classes-and-methods:
All the parts must use the partial keyword

[whatston3]

Recommend pushing this into FindNearestUnoccupied.

[whatston3]

Suggested change

	using CrateMachineComponent = Content.Shared._NF.CrateMachine.Components.CrateMachineComponent;
	using Content.Shared._NF.CrateMachine.Components;