Remove startup traversal entirely #5365
Conversation
| public virtual EntityUid SpawnAttachedTo(string? protoName, EntityCoordinates coordinates, ComponentRegistry? overrides = null, Angle rotation = default, bool traversal = true) | ||
| { | ||
| if (!coordinates.IsValid(this)) | ||
| throw new InvalidOperationException($"Tried to spawn entity {protoName} on invalid coordinates {coordinates}."); | ||
|
|
||
| var entity = CreateEntityUninitialized(protoName, coordinates, overrides, rotation); | ||
| var entity = CreateEntityUninitialized(protoName, coordinates, overrides, rotation, traversal); |
There was a problem hiding this comment.
Considering we're using overloads for different functionality it would be better for traversal to just not run.
There was a problem hiding this comment.
do you mean to disable startup traversal completely? wouldn't that be a breaking change?
There was a problem hiding this comment.
yes, what's wrong with breaking changes?
I'd rather just rip off a bandaid instead of making the already slow entity spawning even slower.
n00kii
changed the title
SpawnAttachedTo bypassing initial traversal|
|
||
| private void OnStartup(ref TransformStartupEvent ev) | ||
| { | ||
| CheckTraverse(ev.Entity); | ||
| } |
There was a problem hiding this comment.
This is going to be a massively breaking change and the corresponding spawn code needs to be adjusted to handle this.
There was a problem hiding this comment.
so if im understanding what each usecase for the different spawn functions are, here's how this should look like:
(1) base Spawn: spawn in nullspace
(2) Spawn with MapCoordinates overload: spawn on map, reparent to grid if overlap
(3) SpawnAttachedTo: spawn with parent and position provided by EntitiyCoordinates
(4) SpawnAtPosition: same as (2) but transforms provided EntityCoordinates to MapCoordinates first
(5) TrySpawnInContainer, SpawnInContainerOrDrop, TrySpawnNextTo, SpawnNextToOrDrop: as their names imply
ignoring (1) since trivial case, and (5) since those functions are built up from (1..4). (2) and (4) just need to make sure they check for overlapping grids and reparent if one is found. is it fine to do have it implemented with the changes i've made to the MapCoordinates overload of CreateEntityUninitialized? (3) uses the EntityCoordinates overload of CreateEntityUninitialized which doesn't check for grids so no need to worry about traversing in that case
There was a problem hiding this comment.
but now im confused because this is just making traversal implicitly built into certain spawn functions rather than relying on the system for it; is that what we want?
when
TransformComponentstarts up, it runs an initial grid traversal check. however, you can't flip theGridTraversalbool of the transform component before it's already started when you spawn an entity.therefore, i add an argument (defaulting to true) to set theGridTraversalboolean as soon as possible, before the transform component is started up.i've only added it toSpawnAttachedTobecause intuitively, if you use this function you're expecting whatever you attach to whatever to STAY attached, and not silently grid-traverse detach; so now you have the option.now, traversal no longer subscribes to
TransformStartupEvent, so you can modify traversal as you wish before it actually happens