Conversation
WalkthroughРефакторинг спауна призраков с добавлением режима наблюдателя лобби и снимков внешности; клиентские per‑entity post‑shader для тонирования призраков и управление alpha_cutoff в outline; массовое добавление прототипов визуальных наблюдателей по видам и новые шейдеры ресурсов. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Content.Server/Ghost/GhostSystem.cs`:
- Around line 859-878: TryCaptureMindHumanoidSnapshot currently spawns a full
entity in Nullspace and calls CopyInventorySnapshot which creates real item
instances (with active components); change the snapshot workflow so spawned
snapshot entities are visual-only: modify CopyInventorySnapshot (or add a new
CreateVisualInventorySnapshot helper) to spawn lightweight visual placeholders
or clone only appearance/state (using CloneAppearance-like logic) instead of
full prototypes, or spawn entities with initialization suppressed/stripped of
runtime components (remove/avoid SolutionContainerComponent,
DeviceNetworkComponent, etc.) before they initialize; ensure
_mindHumanoidSnapshotSources still maps to the placeholder snapshot and that
CloneAppearance/CopyDamageSnapshot operate on that visual-only entity so no
side-effectful components run in Nullspace.
🧹 Nitpick comments (7)
Resources/Prototypes/Entities/Mobs/Player/observer.yml (1)
95-143: Значительное дублирование компонентов сMobObserverBase.
MobObserverVisualBaseпрактически полностью дублирует список компонентов изMobObserverBase(lines 31–83), с добавлениемNoNormalInteraction,Spectralи тегаAllowGhostShownByEvent. ЕслиMobObserverBaseизменится (например, добавится новый компонент для призраков),MobObserverVisualBaseнужно будет синхронизировать вручную.Рассмотрите возможность наследования
MobObserverVisualBaseотMobObserverBase(или общего абстрактного родителя) с добавлением только различающихся компонентов, чтобы избежать рассинхронизации.Content.Client/Interactable/Components/InteractionOutlineComponent.cs (1)
74-82:Ownervsuid— незначительная несогласованность.
MakeNewShaderиспользуетOwnerдля проверкиGhostComponent, тогда как вызывающие методы (OnMouseEnter,UpdateInRange) получаютuidв качестве параметра. ХотяOwner == uidв данном контексте, передачаuidвMakeNewShaderбыла бы более явной и согласованной с остальным кодом компонента.♻️ Предлагаемое изменение
- private ShaderInstance MakeNewShader(bool inRange, int renderScale) + private ShaderInstance MakeNewShader(bool inRange, int renderScale, EntityUid uid) { var shaderName = inRange ? ShaderInRange : ShaderOutOfRange; var instance = _prototypeManager.Index<ShaderPrototype>(shaderName).InstanceUnique(); instance.SetParameter("outline_width", DefaultWidth * renderScale); - instance.SetParameter("alpha_cutoff", _entMan.HasComponent<GhostComponent>(Owner) ? GhostAlphaCutoff : DefaultAlphaCutoff); + instance.SetParameter("alpha_cutoff", _entMan.HasComponent<GhostComponent>(uid) ? GhostAlphaCutoff : DefaultAlphaCutoff); return instance; }И обновить вызовы:
- _shader = MakeNewShader(inInteractionRange, renderScale); + _shader = MakeNewShader(inInteractionRange, renderScale, uid);- _shader = MakeNewShader(_inRange, _lastRenderScale); + _shader = MakeNewShader(_inRange, _lastRenderScale, uid);Content.Client/Ghost/GhostSystem.cs (1)
230-244: ПерезаписьPostShaderможет конфликтовать с другими системами.
EnsureGhostCompositeShaderбезусловно назначаетsprite.PostShader, затирая любой ранее установленный шейдер (например, от системы аутлайнов или кастомных эффектов). Если другая система уже задалаPostShaderдля этого спрайта, он будет потерян без возможности восстановления.Стоит рассмотреть сохранение предыдущего
PostShaderперед назначением и его восстановление вRemoveGhostCompositeShader, либо как минимум документировать, что призраки-наблюдатели не поддерживают другие пост-шейдеры.Content.Server/Ghost/GhostSystem.cs (4)
790-810:TryPickLobbyObserverJob— выбирает толькоJobPriority.High.Метод берёт первую работу с приоритетом
High. Если у игрока несколько таких работ, выбор зависит от порядка итерацииDictionary<ProtoId<JobPrototype>, JobPriority>. Это недетерминировано — разные запуски могут давать разные результаты для одного и того же профиля.Если это намеренно (любая High-работа подойдёт), стоит добавить комментарий. Если нет — рассмотреть сортировку или использование
profile.PreferenceUnavailable/ приоритизацию по другому критерию.
912-930: Хрупкий парсинг имени прототипа куклы.
TryGetVisualObserverPrototypeFromDollполагается на жёсткую конвенциюMob{Species}Dummyдля извлечения имени вида. Это работает для стандартных прототипов, но любое отклонение от конвенции (например,MobCustomSpeciesDoll,MobHumanoidDummy) приведёт к генерации неправильного имени.Это скорее brittle code, а не баг, т.к. fallback на
VisualObserverPrototypeNameобеспечивает безопасный путь. Однако стоит добавить комментарий, документирующий ожидаемый формат.📝 Предложение: добавить документирующий комментарий
+ /// <summary> + /// Extracts species ID from doll prototype following the convention "Mob{SpeciesId}Dummy". + /// Returns null if the prototype name doesn't match this pattern. + /// </summary> private static string? TryGetVisualObserverPrototypeFromDoll(EntProtoId dollPrototype)
598-608: Два публичных методаSpawnGhostс разными сигнатурами — потенциальная путаница в API.Существуют два перегрузки
SpawnGhost:
SpawnGhost(mind, targetEntity, canReturn)(строка 598) — принимаетEntityUid, используетtargetEntityи как точку спауна, и как sourceEntity.SpawnGhost(mind, spawnPosition, canReturn, sourceEntity)(строка 628) — принимаетEntityCoordinates?.Плюс
SpawnLobbyObserverGhost(строка 605).Перегрузка на строке 598-603 принимает
EntityUid targetEntity, вычисляет из него координаты и передаёт тот жеtargetEntityкакsourceEntity. Это предполагает, чтоtargetEntity— это тело игрока. НазваниеtargetEntityвводит в заблуждение —sourceBodyилиbodyEntityбыло бы яснее.💡 Предложение: переименовать параметр для ясности
- public EntityUid? SpawnGhost(Entity<MindComponent?> mind, EntityUid targetEntity, + public EntityUid? SpawnGhost(Entity<MindComponent?> mind, EntityUid sourceBody, bool canReturn = false) { - _transformSystem.TryGetMapOrGridCoordinates(targetEntity, out var spawnPosition); - return SpawnGhost(mind, spawnPosition, canReturn, targetEntity); + _transformSystem.TryGetMapOrGridCoordinates(sourceBody, out var spawnPosition); + return SpawnGhost(mind, spawnPosition, canReturn, sourceBody); }
104-104: Словарь_mindHumanoidSnapshotSourcesне очищается приRoundRestartCleanupEvent.Если раунд перезапускается, а сущности разума уничтожаются (с вызовом
EntityTerminatingEvent), снапшоты будут очищены поштучно. Однако, если по какой-то причинеEntityTerminatingEventне сработает для всех mind-сущностей, словарь будет содержать stale EntityUid. Рекомендуется подписаться наRoundRestartCleanupEventдля полной очистки.🧹 Предложение: очистка при перезапуске раунда
В
Initialize():SubscribeLocalEvent<MindComponent, EntityTerminatingEvent>(OnMindTerminating); + SubscribeLocalEvent<RoundRestartCleanupEvent>(OnRoundRestart);Новый метод:
+ private void OnRoundRestart(RoundRestartCleanupEvent ev) + { + foreach (var snapshot in _mindHumanoidSnapshotSources.Values) + { + if (Exists(snapshot) && !TerminatingOrDeleted(snapshot)) + QueueDel(snapshot); + } + _mindHumanoidSnapshotSources.Clear(); + }
| private bool TryCaptureMindHumanoidSnapshot(EntityUid mindId, EntityUid sourceEntity) | ||
| { | ||
| if (!Exists(sourceEntity) || !TryGetVisualObserverPrototype(sourceEntity, out var ghostPrototype)) | ||
| return false; | ||
|
|
||
| ClearMindHumanoidSnapshot(mindId); | ||
|
|
||
| var snapshot = Spawn(ghostPrototype, MapCoordinates.Nullspace); | ||
| if (!HasComp<HumanoidAppearanceComponent>(snapshot)) | ||
| { | ||
| QueueDel(snapshot); | ||
| return false; | ||
| } | ||
|
|
||
| _mindHumanoidSnapshotSources[mindId] = snapshot; | ||
| _humanoid.CloneAppearance(sourceEntity, snapshot); | ||
| CopyDamageSnapshot(sourceEntity, snapshot); | ||
| CopyInventorySnapshot(sourceEntity, snapshot); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Снапшот-сущность в Nullspace содержит полные клоны предметов инвентаря.
TryCaptureMindHumanoidSnapshot спаунит сущность в Nullspace и вызывает CopyInventorySnapshot, который создаёт полные экземпляры предметов (не визуальные заглушки). Эти предметы могут содержать компоненты с побочными эффектами (например, SolutionContainerComponent, DeviceNetworkComponent и т.д.), которые будут инициализированы при спауне.
Для чисто визуального снапшота это избыточно и потенциально проблематично — побочные эффекты инициализации компонентов, утечки ресурсов, нагрузка на сеть и ECS.
🤖 Prompt for AI Agents
In `@Content.Server/Ghost/GhostSystem.cs` around lines 859 - 878,
TryCaptureMindHumanoidSnapshot currently spawns a full entity in Nullspace and
calls CopyInventorySnapshot which creates real item instances (with active
components); change the snapshot workflow so spawned snapshot entities are
visual-only: modify CopyInventorySnapshot (or add a new
CreateVisualInventorySnapshot helper) to spawn lightweight visual placeholders
or clone only appearance/state (using CloneAppearance-like logic) instead of
full prototypes, or spawn entities with initialization suppressed/stripped of
runtime components (remove/avoid SolutionContainerComponent,
DeviceNetworkComponent, etc.) before they initialize; ensure
_mindHumanoidSnapshotSources still maps to the placeholder snapshot and that
CloneAppearance/CopyDamageSnapshot operate on that visual-only entity so no
side-effectful components run in Nullspace.
| private void CopyInventorySlotVisualSnapshot(EntityUid sourceItem, EntityUid ghost, string slotName, EntityCoordinates ghostCoordinates) | ||
| { | ||
| var prototype = MetaData(sourceItem).EntityPrototype?.ID; | ||
| if (prototype == null || !_prototypeManager.HasIndex<EntityPrototype>(prototype)) | ||
| return; | ||
|
|
||
| var clone = SpawnAtPosition(prototype, ghostCoordinates); | ||
|
|
||
| if (TryComp<ItemComponent>(sourceItem, out var sourceItemComp) && | ||
| TryComp<ItemComponent>(clone, out var cloneItemComp)) | ||
| { | ||
| _item.CopyVisuals(clone, sourceItemComp, cloneItemComp); | ||
| } | ||
|
|
||
| if (TryComp<ClothingComponent>(sourceItem, out var sourceClothingComp) && | ||
| TryComp<ClothingComponent>(clone, out var cloneClothingComp)) | ||
| { | ||
| _clothing.CopyVisuals(clone, sourceClothingComp, cloneClothingComp); | ||
| } | ||
|
|
||
| _appearance.CopyData(sourceItem, clone); | ||
|
|
||
| if (!_inventory.TryEquip(ghost, clone, slotName, silent: true, force: true)) | ||
| QueueDel(clone); | ||
| } |
There was a problem hiding this comment.
Полный спаун предметов для визуального клонирования — риск побочных эффектов.
CopyInventorySlotVisualSnapshot создаёт полноценные сущности через SpawnAtPosition по прототипу исходного предмета. Каждый такой предмет проходит полную инициализацию всех компонентов (MapInit, ComponentStartup и т.д.), что может вызвать:
- Регистрацию в сетевых системах устройств
- Инициализацию реагентов/растворов
- Срабатывание триггеров
- Регистрацию в системах питания/атмосферы
Для визуальной копии одежды на призраке достаточно было бы спаунить упрощённые визуальные заглушки или отключать ненужные компоненты после спауна.
Пометил мои изменения
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@Content.Client/Ghost/GhostSystem.cs`:
- Around line 239-246: The code is using the non-existent lowercase property
component.color; update the three usages to the correct PascalCase property
GhostComponent.Color — specifically replace component.color in the
shader.SetParameter("ghost_tint", ...), in the Math.Clamp line for
"ghost_alpha", and in the final sprite.Color assignment where
RemoveGhostCompositeShader is called; ensure you reference GhostComponent.Color
and keep the rest of the expressions (VisualObserverAlphaMultiplier,
RemoveGhostCompositeShader, shader.SetParameter) unchanged.
In `@Content.Server/Ghost/GhostSystem.cs`:
- Around line 795-815: TryPickLobbyObserverJob currently returns the first
High-priority entry from profile.JobPriorities which is non-deterministic
because Dictionary iteration order is undefined; instead collect all entries
where priority == JobPriority.High and
_prototypeManager.HasIndex<JobPrototype>(job), then pick one deterministically
(e.g. OrderBy the job's unique identifier or name) so selection is stable across
runs; keep the existing overflow fallback logic and return jobId/default the
same way. Reference: TryPickLobbyObserverJob, profile.JobPriorities,
JobPriority.High, _prototypeManager.HasIndex<JobPrototype>, and
SharedGameTicker.FallbackOverflowJob.
- Line 104: _mindHumanoidSnapshotSources can retain mappings for long-lived mind
entities leaving snapshots in Nullspace; in addition to removing entries on
EntityTerminatingEvent, update TrySpawnBodySnapshotGhost to remove (or null out)
the entry for the mind when a snapshot is successfully consumed: locate
TrySpawnBodySnapshotGhost and after the code path that successfully spawns/uses
the snapshot, call _mindHumanoidSnapshotSources.Remove(mindEntity) (or otherwise
clear the mapping) so the snapshot reference is released and not left in
Nullspace.
🧹 Nitpick comments (3)
Content.Client/Ghost/GhostSystem.cs (1)
27-27: Словарь_compositeGhostShadersне очищается приShutdownсистемы.При переподключении или пересоздании системы шейдеры не будут dispose'нуты. Рекомендуется переопределить
Shutdown()и вычистить/dispose все шейдеры из словаря.♻️ Добавить очистку при Shutdown
+ public override void Shutdown() + { + base.Shutdown(); + + foreach (var shader in _compositeGhostShaders.Values) + shader.Dispose(); + + _compositeGhostShaders.Clear(); + }Content.Server/Ghost/GhostSystem.cs (2)
739-793: ВTrySpawnLobbyObserverGhostпри неудачеLoadProfileили экипировки ghost-сущность не удаляется.Если
_humanoid.LoadProfileвыбросит исключение или_stationSpawning.EquipStartingGearупадёт, спаунённая сущностьghostостанется, но метод вернётtrueс частично настроенным призраком. Это может привести к визуальным артефактам. Рекомендуется обернуть в try-catch или валидировать состояние перед возвратом.Также, если
TryPickLobbyObserverJobне находит подходящую работу (строки 765-769), призрак создаётся без одежды — убедитесь, что это ожидаемое поведение.
917-935:TryGetVisualObserverPrototypeFromDollиспользует хрупкую строковую эвристику для извлечения species ID.Метод предполагает формат
Mob{SpeciesId}Dummyдля doll-прототипов. Если формат именования изменится, метод молча перестанет работать. Это допустимо как fallback, но стоит добавить комментарий о предполагаемом формате.
|
|
||
| private EntityQuery<GhostComponent> _ghostQuery; | ||
| private EntityQuery<PhysicsComponent> _physicsQuery; | ||
| private readonly Dictionary<EntityUid, EntityUid> _mindHumanoidSnapshotSources = new(); |
There was a problem hiding this comment.
_mindHumanoidSnapshotSources может накапливать сущности в Nullspace, если mind-сущности живут долго.
Снапшоты очищаются только при EntityTerminatingEvent на mind-сущности (строка 334-337). Если mind-сущность существует дольше, чем нужен снапшот (например, игрок возвращается в тело), снапшот остаётся в Nullspace до конца раунда. Стоит рассмотреть очистку снапшота и при успешном использовании в TrySpawnBodySnapshotGhost.
♻️ Очистка снапшота после использования
private EntityUid? ResolveVisualSnapshotSource(EntityUid mindId, EntityUid? sourceEntity)
{
if (sourceEntity is { } source && Exists(source) && HasComp<HumanoidAppearanceComponent>(source))
+ {
+ // Source entity is still available, clean up any stale snapshot
+ ClearMindHumanoidSnapshot(mindId);
return source;
+ }
return TryGetCachedMindHumanoidSnapshot(mindId, out var cached) ? cached : null;
}🤖 Prompt for AI Agents
In `@Content.Server/Ghost/GhostSystem.cs` at line 104,
_mindHumanoidSnapshotSources can retain mappings for long-lived mind entities
leaving snapshots in Nullspace; in addition to removing entries on
EntityTerminatingEvent, update TrySpawnBodySnapshotGhost to remove (or null out)
the entry for the mind when a snapshot is successfully consumed: locate
TrySpawnBodySnapshotGhost and after the code path that successfully spawns/uses
the snapshot, call _mindHumanoidSnapshotSources.Remove(mindEntity) (or otherwise
clear the mapping) so the snapshot reference is released and not left in
Nullspace.
| private bool TryPickLobbyObserverJob(HumanoidCharacterProfile profile, out ProtoId<JobPrototype> jobId) | ||
| { | ||
| foreach (var (job, priority) in profile.JobPriorities) | ||
| { | ||
| if (priority != JobPriority.High || !_prototypeManager.HasIndex<JobPrototype>(job)) | ||
| continue; | ||
|
|
||
| jobId = job; | ||
| return true; | ||
| } | ||
|
|
||
| var overflowJob = new ProtoId<JobPrototype>(SharedGameTicker.FallbackOverflowJob); | ||
| if (_prototypeManager.HasIndex<JobPrototype>(overflowJob)) | ||
| { | ||
| jobId = overflowJob; | ||
| return true; | ||
| } | ||
|
|
||
| jobId = default; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
TryPickLobbyObserverJob берёт только первую работу с JobPriority.High.
Если у профиля несколько High-приоритетных работ, будет выбрана первая из Dictionary итерации, порядок которой не детерминирован. Это может приводить к непредсказуемому выбору работы при каждом спауне.
🤖 Prompt for AI Agents
In `@Content.Server/Ghost/GhostSystem.cs` around lines 795 - 815,
TryPickLobbyObserverJob currently returns the first High-priority entry from
profile.JobPriorities which is non-deterministic because Dictionary iteration
order is undefined; instead collect all entries where priority ==
JobPriority.High and _prototypeManager.HasIndex<JobPrototype>(job), then pick
one deterministically (e.g. OrderBy the job's unique identifier or name) so
selection is stable across runs; keep the existing overflow fallback logic and
return jobId/default the same way. Reference: TryPickLobbyObserverJob,
profile.JobPriorities, JobPriority.High,
_prototypeManager.HasIndex<JobPrototype>, and
SharedGameTicker.FallbackOverflowJob.
1 participant
Описание PR
Новые призраки для всех игровых рас!
Поддерживает заход из лобби и "вроде бы" не ломает кастомных гостов.
Все повреждения на призраке отражают повреждения на момент смерти.
Бууу!
Медиа
Список
trim.CF68E131-A262-48A4-B1D6-834812DBDB14.MOV
Изменения
🆑 nekosich