GridFixtureSystem Optimizations#6415
Open
Aidenkrz wants to merge 5 commits intospace-wizards:masterfrom
Open
Conversation
Contributor
Author
|
accidentally left the PR title as the commit name and cant change it now 👎 |
Member
|
github stays winning |
Member
|
Apparently you can still change it from the mobile app. You can also just tell me what title you'd like and I'll change it for you. |
Contributor
Author
Thanks I used GitHub mobile |
moonheart08
approved these changes
Feb 20, 2026
Contributor
moonheart08
left a comment
There was a problem hiding this comment.
Looking for footage of this being tested, ideally, just as a "this obviously works". The changes lgtm though.
DrSmugleaf
approved these changes
Mar 19, 2026
Member
DrSmugleaf
left a comment
There was a problem hiding this comment.
Outside of DungeonJob it seems like the only other caller to CheckSplits is SharedMapSystem.RegenerateCollision, which seem fine at a glance
Contributor
Author
grid.mp4 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
About the PR
Optimizes
GridFixtureSystemin a few hot paths that were doing more work (and allocating more garbage) than they really needed to.Ended up doing a bunch more fuckery to SharedGridFixtureSystem as well after looking through it as well, refer to technical details for info on that.
Why / Balance
One of my maintainers pointed out that there was some evil LINQ garbage in this system that called
.Sum()a bajillion times wheneverCheckSplitsran. Not the end of the world, but also easily avoidable. This cleans that up and makes the whole thing scale way better without changing behavior. Also noticed system was allocating new collections constantly, so i changed that as well.Technical details
The BFS scratch structures (
_splitFrontierqueue and_splitGridslist) are now reused as class fields instead of being allocated everyCheckSplitscall.The sort comparator was doing repeated LINQ
$O(N \times M \times \log N)$ work.
$O(N \times M + N \times \log N)$ .
.Sum()calls, resulting in something likeThis is now replaced with a precomputed
gridSizesdictionary, bringing it down toAs a rough example:
I didn't do any real benchmarking as I don't have dottrace, but I mean, the math checks out.
In
CreateNodes, the tileHashSetnow only tracks non-empty tiles (capacity set tochunk.FilledTiles) instead of blindly allocating for all 256 chunk positions.Cross-chunk neighbor detection now bails early for interior tiles, since only edge tiles can possibly have neighbors in adjacent chunks.
MapGridComponentis now passed through the full call chain as a nullable parameter with TryComp/Comp fallback.Grid fixture diffing in
UpdateFixturenow cross-references old and new fixtures by ID instead of using a fullFixture.Equalscomparison. The old approach always failed because content (e.g.ShuttleSystem.OnGridFixtureChange) modifies fixture density after creation, causing every fixture to be destroyed and recreated on every tile change. The new approach compares shapes only viaEqualsApprox, preserving content-set properties like density. This also eliminated a redundant shape check that existed solely as a workaround for the broken equality comparison, and fixed a latent bug wherechunk.Fixtures(aHashSet<string>) was being modified duringforeachenumeration.During grid splits, entity reparenting now does a single broadphase query per node using the union bounding box of all its tiles, instead of one query per tile. Entities are filtered by converting their position to a tile index and checking membership in a
HashSet<Vector2i>. This reduces broadphase queries from O(tiles) to O(1) per node.