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

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

Extend the shared version, add the needed import above?

[GreaseMonk]

I will add it, however it will add another cleanup warning, but i do agree for readability is best to add it

[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

[GreaseMonk]

strange my IDE is saying it can safely be removed

[GreaseMonk]

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.

Thank you for the review. I've updated the code to match your suggestions and comments.
Should be ready for another review, or, approval- up to you.

[dvir001]

Marking as good to go, waiting for @whatston3 to confirm if needed.

Changes made.

[whatston3]

A few cleanup suggestions - "ChildClass : ParentClass" is pretty much the norm. Not sure exactly what configuration's setting that up for the IDE offhand, if there's a style guide config in the workspace.

Really minor stuff, and yes, sorry, I was wrong about the Client side MarketSystem.

[whatston3]

Changes are really minor, just going to batch commit them and it's an approval from my side.

[dvir001]

Not broken it seems