Decal Performance 2

[CrazyPhantom779]

Short description

Decals were still lagging on creation due to them dirtying the whole chunk if anything was added, So I made them delta the changes and only sync the diff to clients unless they are only getting it for the first time they they get the whole chunk.

Why we need to add this

Perf fix

Media (Video/Screenshots)

Checks

• I do not require assistance to complete the PR.
• Before posting/requesting review of a PR, I have verified that the changes work.
• I have added screenshots/videos of the changes, or this PR does not change in-game mechanics.
• I affirm that my changes are licensed under the MIT License and grant permission for use in this repository under its conditions.

Changelog

🆑 CrazyPhantom

• fix: Fixed decal performance issue on decal creation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 430b7aeb-7672-442f-b0a9-dd070a79bfe1

📥 Commits

Reviewing files that changed from the base of the PR and between 4e31dc0 and 1b1ee17.

📒 Files selected for processing (2)

• Content.Client/Decals/DecalSystem.cs
• Content.Server/Decals/DecalSystem.cs

🚧 Files skipped from review as they are similar to previous changes (2)

• Content.Client/Decals/DecalSystem.cs
• Content.Server/Decals/DecalSystem.cs

---

📝 Walkthrough

Walkthrough

DecalChunkUpdateEvent now transmits per-chunk decal diffs. The server records decal mutations and sends full snapshots or incremental updates based on player visibility, while the client merges removals and upserts into existing chunks.

Changes

Decal chunk synchronization

Layer / File(s)	Summary
Shared diff event contract Content.Shared/Decals/DecalChunkUpdateEvent.cs	The update event now carries per-grid, per-chunk DecalChunkDiff payloads containing upserted and removed decal IDs.
Server delta tracking and batching Content.Server/Decals/DecalSystem.cs	Decal mutations are recorded per chunk and scoped to PVS batches; player updates distinguish full snapshots from dirty known chunks and serialize incremental diffs accordingly.
Client incremental diff application Content.Client/Decals/DecalSystem.cs	The client validates target grids and merges removals and upserts into existing decal chunks and decal indices.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DecalSystem
  participant UpdatePlayer
  participant SendChunkUpdates
  participant DecalChunkUpdateEvent
  participant ClientDecalSystem
  DecalSystem->>UpdatePlayer: record decal deltas and select visible chunks
  UpdatePlayer->>SendChunkUpdates: pass fullChunks and diffChunks
  SendChunkUpdates->>DecalChunkUpdateEvent: serialize snapshots and chunk diffs
  DecalChunkUpdateEvent->>ClientDecalSystem: deliver decal update event
  ClientDecalSystem->>ClientDecalSystem: merge removals and upserted decals

Loading

Possibly related PRs

• ss14Starlight/space-station-14#4623: Modifies the server-side decal chunk update flow and player chunk batching.

🚥 Pre-merge checks | ✅ 5 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
No Magic Numbers	⚠️ Warning	Nontrivial literals remain in Content.Server/Decals/DecalSystem.cs, including pool capacity 64 and distance 0.75f, without named constants.	Extract the pool capacity, decal range, and any other repeated thresholds into clearly named constants; keep only obvious literals like 0/1/2.
Title check	❓ Inconclusive	The title points to decal performance, but "2" makes it too vague to describe the actual sync-diff change.	Rename it to something like "Sync decal chunk diffs instead of dirtying whole chunks".

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description matches the change: it explains delta syncing decals for new and existing clients.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No Hardcoded Ecs Parameters	✅ Passed	No entity-level ECS parameters were hardcoded; the PR only adds decal diff sync logic, and DecalGridComponent still carries storage, not behavior config.
Avoid Service Locator	✅ Passed	PASS — No IoCManager/Resolve/GetService usage was added; the new decal sync code relies on explicit [Dependency] injection. Nice cleanup!

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Content.Client/Decals/DecalSystem.cs`:
- Around line 174-180: Update the removal loop in the diff merge path to invoke
OnDecalRemoved before removing the corresponding entry from chunk.Decals,
preserving the hook’s requirement that the decal still exists. Keep
gridComp.DecalIndex.Remove(removedId) after the hook and ensure each removal is
processed only when the decal is present.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: b954997a-5cd6-4ec1-94ea-558c9c2e6450

📥 Commits

Reviewing files that changed from the base of the PR and between f88ef61 and 561ab90.

📒 Files selected for processing (3)

• Content.Client/Decals/DecalSystem.cs
• Content.Server/Decals/DecalSystem.cs
• Content.Shared/Decals/DecalChunkUpdateEvent.cs

[redmushie]

Looks great as always Crazy, just one pair of missing comments and some naming nitpicks to reduce confusion when reading