Make wizard great again! Парт ту#874
Make wizard great again! Парт ту#874Kai518 wants to merge 61 commits intoWWhiteDreamProject:masterfrom
Conversation
(cherry picked from commit 0a394d4)
Co-authored-by: opl <[email protected]> (cherry picked from commit 0805943)
* Adds smoke spell for wizard * Adds the actual sprites oops * Removes repulse attract change * Removes change again? (cherry picked from commit c922b4f)
(cherry picked from commit ee31a1f)
(cherry picked from commit a9054c5)
…ants (#34994) * Prevents admin ghosts from being shown with ghost event * Changes summon ghost tooltip to permanently visible * Adds AllowGhostShownByEvent tag, adds it to regular observers, removes ghost hands check & revenant check and replaces it with a tag check. (cherry picked from commit 7bf33ff)
…tedChargesComponent (#40347) * Fix * Update (cherry picked from commit 9c3af67)
|
RSI Diff Bot; head commit 83556a7 merging into d11b654 Resources/Textures/Clothing/Neck/mantles/mantle.rsi
Resources/Textures/Objects/Tools/crowbar.rsi
Resources/Textures/_White/Wizard/Clothing/Eyes/eyepatch.rsi
Resources/Textures/_White/Wizard/Clothing/Hands/wizgloves.rsi
Resources/Textures/_White/Wizard/Clothing/Head/blackwizard.rsi
Resources/Textures/_White/Wizard/Clothing/Head/chanterelle.rsi
Resources/Textures/_White/Wizard/Clothing/Head/clownwizard.rsi
Resources/Textures/_White/Wizard/Clothing/Head/comwizard.rsi
Resources/Textures/_White/Wizard/Clothing/Head/crimsonwizard.rsi
Resources/Textures/_White/Wizard/Clothing/Head/mimewizard.rsi
Resources/Textures/_White/Wizard/Clothing/Head/oblivionenforcer.rsi
Resources/Textures/_White/Wizard/Clothing/Head/psywizard.rsi
Resources/Textures/_White/Wizard/Clothing/Head/tapewizard.rsi
Resources/Textures/_White/Wizard/Clothing/Head/yellowwizard.rsi
Resources/Textures/_White/Wizard/Clothing/Mask/cow.rsi
Resources/Textures/_White/Wizard/Clothing/Mask/frog.rsi
Resources/Textures/_White/Wizard/Clothing/Mask/horse.rsi
Resources/Textures/_White/Wizard/Clothing/Mask/oblivionenforcer.rsi
Resources/Textures/_White/Wizard/Clothing/Mask/pig.rsi
Resources/Textures/_White/Wizard/Clothing/Neck/crimsonwizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/blackwizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/clownwizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/comwizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/crimsonwizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/mimewizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/oblivionenforcer.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/paperwizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/psywizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/tapewizard.rsi
Resources/Textures/_White/Wizard/Clothing/OuterClothing/yellowwizard.rsi
Resources/Textures/_White/Wizard/Clothing/Shoes/crimson.rsi
Resources/Textures/_White/Wizard/Effects/blood.rsi
Resources/Textures/_White/Wizard/Effects/effects.rsi
Resources/Textures/_White/Wizard/Effects/effects160x160.rsi
Resources/Textures/_White/Wizard/StatusIcons/10x10.rsi
Resources/Textures/_White/Wizard/actions.rsi
|
|
губыгубыгубыгубыгубы |
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
…18/wwdpublic into MakeWizardGreatAgainPartTwo
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
Content.Client/_White/Wizard/Trail/TrailSystem.cs (1)
190-222:returnблокирует обработку всех последующих частиц иAdditionalLerpData.Проблема уже была указана в предыдущем ревью:
returnна строке 193 должен быть заменён наcontinue, а LINQ.Where()на строке 216 создаёт аллокации каждый кадр.
🧹 Nitpick comments (2)
Resources/Prototypes/Entities/Clothing/Head/hardsuit-helmets.yml (1)
703-708: Рассмотрите удаление закомментированного кода traumaDeductions.Строки 703-708 содержат закомментированный блок
traumaDeductions, который описывается как "prevents traumas at all (kinda)". Поскольку этот код не используется и может привести к путанице при обслуживании, рекомендуется либо полностью удалить его, либо добавить подробный комментарий, объясняющий причину сохранения.Content.Client/_White/Wizard/Trail/TrailSystem.cs (1)
119-125: Потенциальное деление на ноль приParticleAmount == 1.На строке 123 происходит деление на
(trail.ParticleAmount - 1). ЕслиParticleAmountравен 1, цикл не выполнится (i < 0 ложно), но еслиParticleAmountстанет 1 во время выполнения, это всё равно безопасно. Однако стоит добавить защиту или комментарий для ясности.Рассмотрите добавление явной проверки:
- for (var i = 0; i < trail.ParticleAmount - 1; i++) + for (var i = 0; i < trail.ParticleAmount - 1 && trail.ParticleAmount > 1; i++)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Content.Client/_White/Wizard/Trail/TrailOverlay.cs(1 hunks)Content.Client/_White/Wizard/Trail/TrailSystem.cs(1 hunks)Content.Server/Polymorph/Systems/PolymorphSystem.cs(1 hunks)Content.Shared/Carrying/CarryingComponent.cs(1 hunks)Content.Shared/Magic/Components/MagicComponent.cs(1 hunks)Content.Shared/_White/Wizard/ApprenticeComponent.cs(1 hunks)Content.Shared/_White/Wizard/Projectiles/EntityTrailComponent.cs(1 hunks)Content.Shared/_White/Wizard/Projectiles/EntityTrailSystem.cs(1 hunks)Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs(1 hunks)Content.Shared/_White/Wizard/WizardComponent.cs(1 hunks)Resources/Prototypes/Entities/Clothing/Head/hardsuit-helmets.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Content.Shared/_White/Wizard/WizardComponent.cs
- Content.Shared/_White/Wizard/ApprenticeComponent.cs
- Content.Shared/_White/Wizard/Projectiles/EntityTrailComponent.cs
- Content.Shared/Magic/Components/MagicComponent.cs
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/en-US/_white/cards/hobby.ftl:5-5
Timestamp: 2025-10-22T20:13:31.389Z
Learning: В PR по EvacPod для репозитория WWhiteDreamProject/wwdpublic: если опечатка в ключе локализации (например, card-ep_hobby_mashrooms) не влияет на отображение и используется последовательно, автор предпочитает не блокировать PR; вместо этого предлагать необязательный follow-up issue.
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/ru-RU/_white/cards/danger.ftl:1-1
Timestamp: 2025-10-22T20:31:06.235Z
Learning: В PR по EvacPod для WWhiteDreamProject/wwdpublic: в ru-RU локализациях допустим осознанный сленг сообщества SS13 (напр., «дистры»), если он не ломает отображение и используется последовательно; не блокировать PR, предлагать необязательный follow-up (глоссарий/проверка консистентности).
🧬 Code graph analysis (2)
Content.Client/_White/Wizard/Trail/TrailOverlay.cs (1)
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs (1)
TrailData(188-206)
Content.Shared/_White/Wizard/Projectiles/EntityTrailSystem.cs (2)
Content.Client/_White/Wizard/Trail/TrailSystem.cs (1)
Initialize(28-40)Content.Server/Carrying/CarryingSystem.cs (1)
Initialize(55-73)
🪛 GitHub Actions: Build & Test Debug
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs
[error] 239-239: CS1513: } expected. Build failed while compiling TrailComponent.cs. Command: dotnet build --configuration DebugOpt --no-restore /p:WarningsAsErrors=nullable /m
🪛 GitHub Actions: Build & Test Map Renderer
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs
[error] 239-239: CS1513: } expected
🪛 GitHub Actions: Test Packaging
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs
[error] 239-239: CS1513: } expected
🪛 GitHub Actions: YAML Linter
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs
[error] 239-239: CS1513: } expected
🪛 GitHub Check: build (ubuntu-latest)
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs
[failure] 239-239:
} expected
[failure] 239-239:
} expected
🪛 GitHub Check: Test Packaging
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs
[failure] 239-239:
} expected
[failure] 239-239:
} expected
🪛 GitHub Check: YAML Linter
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs
[failure] 239-239:
} expected
[failure] 239-239:
} expected
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: YAML map schema validator
🔇 Additional comments (9)
Content.Shared/Carrying/CarryingComponent.cs (1)
1-1: I'm unable to access the repository to verify the references automatically. Since the sandbox environment cannot clone theWWhiteDreamProject/wwdpublicrepository, I cannot run the verification script to confirm that all references toCarryingComponenthave been updated to the new namespace.Based on the constraints, here is my rewritten review comment:
Verify that all references to CarryingComponent are updated to the new namespace.
The namespace move from
Content.Server.CarryingtoContent.Shared.Carryingis clean and well-documented with the WD EDIT comment. However, manual verification is needed to ensure that all usages ofCarryingComponentthroughout the codebase have been updated to reference the new shared namespace, and that no lingering references toContent.Server.Carryingremain.Please confirm:
- All imports of
CarryingComponentuseContent.Shared.Carrying- No stray references to the old
Content.Server.Carryingnamespace exist in the codebaseResources/Prototypes/Entities/Clothing/Head/hardsuit-helmets.yml (4)
692-694: Хорошее добавление визуального блока для одежды.Добавление
clothingVisualsдля состоянияoff-equipped-HELMETобеспечивает правильное отображение визуальных эффектов шлема при снятии. Реализация выглядит верно.
695-698: Обновление PointLight согласовано с дизайном.Изменение цвета (#4d7ae3) и параметров PointLight (energy: 10, radius: 5) создаёт визуальный эффект магического свечения шлема, согласуется с тематикой волшебного снаряжения. Значения выглядят сбалансированными.
719-719: Компонент WizardClothes правильно добавлен.Добавление компонента
WizardClothesсоответствует предыдущему предложению рецензента и интегрируется с системой волшебной одежды, описанной в PR. Маркировка "WD EDIT" соответствует соглашениям проекта.
683-683: Проверьте наследование ClothingHeadHatWizardBaseArmor — убедитесь в правильности определения.Изменение родительского класса на
[ClothingHeadHardsuitBase, ClothingHeadHatWizardBaseArmor]вводит новый базовый класс для брони. Пожалуйста, подтвердите, что этот класс корректно определён в связанных файлах (вероятно, вResources/Prototypes/_White/Wizard/Clothing/wizard.yml) и правильно применяет установки защиты.Content.Server/Polymorph/Systems/PolymorphSystem.cs (1)
288-288: I apologize for the repository access issue. However, I can still provide a valuable rewrite of your review comment based on its structure and content. Let me rewrite it to be clearer, more professional, and better organized:
Verify event propagation consistency and assess listener impact from dual raising on child entity.
This change raises
PolymorphedEventon both the original entity and the polymorphed child. Before merging:
Consistency check: The
Revertmethod (around line 364) only raises the event on the original entity (uid), not on both. Confirm whether asymmetrical behavior here is intentional or ifRevertshould apply the same dual-raise pattern.Listener impact: Verify that all subscribers to
PolymorphedEventcan correctly handle the event being raised on both entities and won't experience unintended side effects from duplicate processing.Comment formatting: Move the
// WD EDITcomment to its own line for consistency:var ev = new PolymorphedEvent(uid, child, false); RaiseLocalEvent(uid, ref ev); - RaiseLocalEvent(child, ref ev); // WD EDIT + // WD EDIT + RaiseLocalEvent(child, ref ev);Content.Shared/_White/Wizard/Projectiles/EntityTrailSystem.cs (1)
1-20: LGTM!Система корректно связывает
EntityTrailComponentсTrailComponentпри инициализации и помечает компонент как dirty для сетевой синхронизации.Content.Client/_White/Wizard/Trail/TrailOverlay.cs (1)
91-98: Корректное сохранение и восстановление состояния спрайта.Правильно сохраняются оригинальные значения
ColorиScaleперед модификацией и восстанавливаются после рендеринга каждой частицы.Content.Client/_White/Wizard/Trail/TrailSystem.cs (1)
42-46: Корректная очистка оверлея при выключении.
Shutdownправильно удаляетTrailOverlayиз менеджера оверлеев, предотвращая утечки ресурсов.
| var (position, rotation) = _transform.GetWorldPositionRotation(xform, xformQuery); | ||
|
|
||
| if (trail.Shader != null && _protoMan.TryIndex<ShaderPrototype>(trail.Shader, out var shaderProto)) | ||
| handle.UseShader(shaderProto.InstanceUnique()); |
There was a problem hiding this comment.
Шейдер может "протекать" на следующие трейлы.
Если трейл A имеет шейдер, а трейл B — нет, то трейл B будет отрисован с шейдером трейла A, поскольку handle.UseShader(null) вызывается только в конце всего цикла (строка 144), а не перед каждым трейлом.
Примените этот diff для сброса шейдера перед каждым трейлом:
+ handle.UseShader(null);
+
if (trail.Shader != null && _protoMan.TryIndex<ShaderPrototype>(trail.Shader, out var shaderProto))
handle.UseShader(shaderProto.InstanceUnique());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var (position, rotation) = _transform.GetWorldPositionRotation(xform, xformQuery); | |
| if (trail.Shader != null && _protoMan.TryIndex<ShaderPrototype>(trail.Shader, out var shaderProto)) | |
| handle.UseShader(shaderProto.InstanceUnique()); | |
| var (position, rotation) = _transform.GetWorldPositionRotation(xform, xformQuery); | |
| handle.UseShader(null); | |
| if (trail.Shader != null && _protoMan.TryIndex<ShaderPrototype>(trail.Shader, out var shaderProto)) | |
| handle.UseShader(shaderProto.InstanceUnique()); |
🤖 Prompt for AI Agents
In Content.Client/_White/Wizard/Trail/TrailOverlay.cs around lines 54 to 57, the
code only sets a shader when one exists, allowing a previously set shader to
"leak" to subsequent trails; reset the shader before processing each trail so
trails without a shader don't inherit the prior one — call
handle.UseShader(null) (or otherwise clear the shader) immediately before
checking trail.Shader / attempting to set a new shader, or add an else branch
that explicitly calls handle.UseShader(null) when trail.Shader is null.
| [DataDefinition] | ||
| public sealed partial class LerpPropertyData | ||
| { | ||
| [DataField(required: true)] | ||
| public string? Property; // WD edit - make Property nullable | ||
|
|
||
| [DataField] | ||
| public float LerpAmount = 0.015f; | ||
|
|
||
| [DataField] | ||
| public float Value; | ||
|
|
||
| [DataField] | ||
| public float LerpTarget = 1f; | ||
|
|
||
| public enum RenderedEntityRotationStrategy : byte | ||
| { | ||
| RenderedEntity = 0, | ||
| Trail, | ||
| Particle | ||
| } | ||
|
|
||
| [ImplicitDataDefinitionForInheritors] | ||
| public partial interface IGetShaderData; | ||
| public abstract partial class GetShaderParam : IGetShaderData | ||
| { | ||
| [DataField(required: true)] | ||
| public string Param = string.Empty; | ||
| } | ||
| // Add more data if needed | ||
| public sealed partial class GetShaderLocalPositionData : IGetShaderData; | ||
| public sealed partial class GetShaderFloatParam : GetShaderParam; |
There was a problem hiding this comment.
Отсутствует закрывающая скобка для класса LerpPropertyData — сборка падает.
Класс LerpPropertyData открывается на строке 210, но никогда не закрывается. Enum RenderedEntityRotationStrategy начинается без закрытия предыдущего класса, что приводит к ошибке компиляции CS1513: } expected.
Примените этот diff для исправления:
[DataField]
public float LerpTarget = 1f;
+}
public enum RenderedEntityRotationStrategy : byte📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [DataDefinition] | |
| public sealed partial class LerpPropertyData | |
| { | |
| [DataField(required: true)] | |
| public string? Property; // WD edit - make Property nullable | |
| [DataField] | |
| public float LerpAmount = 0.015f; | |
| [DataField] | |
| public float Value; | |
| [DataField] | |
| public float LerpTarget = 1f; | |
| public enum RenderedEntityRotationStrategy : byte | |
| { | |
| RenderedEntity = 0, | |
| Trail, | |
| Particle | |
| } | |
| [ImplicitDataDefinitionForInheritors] | |
| public partial interface IGetShaderData; | |
| public abstract partial class GetShaderParam : IGetShaderData | |
| { | |
| [DataField(required: true)] | |
| public string Param = string.Empty; | |
| } | |
| // Add more data if needed | |
| public sealed partial class GetShaderLocalPositionData : IGetShaderData; | |
| public sealed partial class GetShaderFloatParam : GetShaderParam; | |
| [DataDefinition] | |
| public sealed partial class LerpPropertyData | |
| { | |
| [DataField(required: true)] | |
| public string? Property; // WD edit - make Property nullable | |
| [DataField] | |
| public float LerpAmount = 0.015f; | |
| [DataField] | |
| public float Value; | |
| [DataField] | |
| public float LerpTarget = 1f; | |
| } | |
| public enum RenderedEntityRotationStrategy : byte | |
| { | |
| RenderedEntity = 0, | |
| Trail, | |
| Particle | |
| } | |
| [ImplicitDataDefinitionForInheritors] | |
| public partial interface IGetShaderData; | |
| public abstract partial class GetShaderParam : IGetShaderData | |
| { | |
| [DataField(required: true)] | |
| public string Param = string.Empty; | |
| } | |
| // Add more data if needed | |
| public sealed partial class GetShaderLocalPositionData : IGetShaderData; | |
| public sealed partial class GetShaderFloatParam : GetShaderParam; |
🧰 Tools
🪛 GitHub Actions: Build & Test Debug
[error] 239-239: CS1513: } expected. Build failed while compiling TrailComponent.cs. Command: dotnet build --configuration DebugOpt --no-restore /p:WarningsAsErrors=nullable /m
🪛 GitHub Actions: Build & Test Map Renderer
[error] 239-239: CS1513: } expected
🪛 GitHub Actions: Test Packaging
[error] 239-239: CS1513: } expected
🪛 GitHub Actions: YAML Linter
[error] 239-239: CS1513: } expected
🪛 GitHub Check: build (ubuntu-latest)
[failure] 239-239:
} expected
[failure] 239-239:
} expected
🪛 GitHub Check: Test Packaging
[failure] 239-239:
} expected
[failure] 239-239:
} expected
🪛 GitHub Check: YAML Linter
[failure] 239-239:
} expected
[failure] 239-239:
} expected
🤖 Prompt for AI Agents
In Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs around lines 208
to 239, the class LerpPropertyData is opened but never closed which causes the
CS1513 compile error; insert a single closing brace '}' immediately after the
LerpTarget field (after the line defining LerpTarget = 1f;) to close the
LerpPropertyData class before the RenderedEntityRotationStrategy enum
declaration so the enum and subsequent types are defined outside the class.
| Blunt: 0.4 | ||
| Slash: 0.4 | ||
| Piercing: 0.4 | ||
| Heat: 0.4 | ||
| Radiation: 0.05 | ||
| Caustic: 0.1 |
There was a problem hiding this comment.
🧩 Analysis chain
Script executed:
# First, let me examine the full context of the hardsuit-helmets.yml file around lines 711-716
sed -n '680,720p' Resources/Prototypes/Entities/Clothing/Head/hardsuit-helmets.ymlRepository: WWhiteDreamProject/wwdpublic
<shell_result>
Repository: WWhiteDreamProject/wwdpublic
Script execution failed with unrecoverable error: [CAPABILITY:RUN_SHELL_SCRIPT] Failed to clone repository WWhiteDreamProject/wwdpublic: Clone operation failed
</shell_result>
Script executed:
# Search for other armor coefficient examples to understand the pattern
rg -n "Blunt:|Slash:|Piercing:" Resources/Prototypes/Entities/Clothing/ -A 5 -B 2 | head -100Repository: WWhiteDreamProject/wwdpublic
<shell_result>
Repository: WWhiteDreamProject/wwdpublic
Script execution failed with unrecoverable error: [CAPABILITY:RUN_SHELL_SCRIPT] Failed to clone repository WWhiteDreamProject/wwdpublic: Clone operation failed
</shell_result>
Script executed:
# Look for SS14 armor coefficient documentation
rg -n "coefficient" Resources/ -i -C 3 -g "*.md" -g "*.yml" | head -80Repository: WWhiteDreamProject/wwdpublic
<shell_result>
Repository: WWhiteDreamProject/wwdpublic
Script execution failed with unrecoverable error: [CAPABILITY:RUN_SHELL_SCRIPT] Failed to clone repository WWhiteDreamProject/wwdpublic: Clone operation failed
</shell_result>
Script executed:
# Check for WizardClothes component definition
rg -n "WizardClothes" Resources/ -B 2 -A 5Repository: WWhiteDreamProject/wwdpublic
<shell_result>
Repository: WWhiteDreamProject/wwdpublic
Script execution failed with unrecoverable error: [CAPABILITY:RUN_SHELL_SCRIPT] Failed to clone repository WWhiteDreamProject/wwdpublic: Clone operation failed
</shell_result>
I'm unable to access the repository due to a clone failure, but I can analyze the information already provided in your review comment to verify its accuracy.
Based on the code snippet and PR description you provided:
Armor coefficient interpretation (standard game mechanics):
- Coefficient value = proportion of damage taken
- 0.4 = 40% damage taken = 60% reduction ✓
- 0.05 = 5% damage taken = 95% reduction (NOT 10% as described)
- 0.1 = 10% damage taken = 90% reduction ✓
The discrepancy you identified is valid:
- PR states: "10% радиационного" (10% damage taken)
- Code shows:
Radiation: 0.05(5% damage taken = 95% reduction) - This is a mismatch requiring clarification
However, I should note:
- The Caustic value (0.1) correctly corresponds to 10% described in PR
- Only the Radiation coefficient (0.05 vs expected 0.1) appears inconsistent
Rewritten Review Comment:
Verify the Radiation armor coefficient — it does not match the PR description.
PR specifies "10% радиационного", but the code sets Radiation: 0.05. With a coefficient of 0.05, the wearer takes only 5% of radiation damage (95% reduction), not the 10% intended. The Caustic coefficient (0.1) correctly matches the specified protection level. Please confirm whether the Radiation value should be 0.1 to match the PR objectives, or if the description requires updating.
🤖 Prompt for AI Agents
Resources/Prototypes/Entities/Clothing/Head/hardsuit-helmets.yml around lines
711-716: the Radiation coefficient is set to 0.05 in the file but the PR text
specifies "10% радиационного" (which corresponds to 0.1), so either change
Radiation: 0.05 to Radiation: 0.1 to match the PR intent or update the PR
description to reflect the actual 0.05 value; make the corresponding change and
verify any downstream documentation or tests that assert the radiation
protection value.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
Content.Client/_White/Wizard/Trail/TrailOverlay.cs (1)
56-57: Шейдер может "протекать" на последующие трейлы.Если трейл A имеет шейдер, а трейл B — нет, то трейл B будет отрисован с шейдером трейла A, поскольку
handle.UseShader(null)вызывается только в конце всего цикла (строка 144), а не перед каждым трейлом.Примените этот diff для сброса шейдера перед каждым трейлом:
+ handle.UseShader(null); + if (trail.Shader != null && _protoMan.TryIndex<ShaderPrototype>(trail.Shader, out var shaderProto)) handle.UseShader(shaderProto.InstanceUnique());Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs (1)
19-26: СинхронизацияMaxParticleAmountс клиентомСейчас
ParticleAmountпомечен как[AutoNetworkedField], аMaxParticleAmount— нет. Если лимит частиц когда‑либо меняется на сервере во время раунда, клиенты об этом не узнают и поведение трейла может разъехаться.Если лимит планируется использовать только как статический параметр из прототипа — можно оставить как есть. Если же его предполагается менять на рантайме (например, спеллами/дебаффами), стоит тоже синхронизировать:
- [DataField] - public int MaxParticleAmount; + [DataField, AutoNetworkedField] + public int MaxParticleAmount;
🧹 Nitpick comments (2)
Content.Client/_White/Wizard/Trail/TrailOverlay.cs (1)
157-158: Рассмотрите улучшение проверки границ для линий.Текущая проверка
!bounds.Contains(start) || !bounds.Contains(end)полностью пропускает линию, если хотя бы одна из конечных точек находится вне viewport. Это может привести к визуальным разрывам на краях экрана, когда линия частично видна, но не отрисовывается.Рассмотрите использование проверки пересечения линии с границами вместо проверки точек:
- if (color.A <= 0.01f || scale <= 0.01f || !bounds.Contains(start) || !bounds.Contains(end)) + if (color.A <= 0.01f || scale <= 0.01f) return; + + // Проверка пересечения линии с viewport + if (!bounds.Contains(start) && !bounds.Contains(end)) + { + // Упрощённая проверка: если обе точки снаружи, линия всё ещё может пересекать viewport + var lineBox = new Box2(Vector2.Min(start, end), Vector2.Max(start, end)); + if (!bounds.Intersects(lineBox)) + return; + }Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs (1)
211-221:Propertyпомечен какstring?, ноrequired: true— спорная комбинация
Propertyобъявлен какstring?, при этом на поле стоитrequired: true:[DataField(required: true)] public string? Property; // WD edit - make Property nullableДля системы сериализации это допустимо, но для nullability-анализатора это выглядит как противоречие (требуемое поле, которое может быть
null). Если вы действительно хотите разрешить отсутствие этого значения в прототипах, логичнее убратьrequired: true. Если же значение должно всегда присутствовать, лучше сделать тип простоstringи, при необходимости, дать безопасный дефолт.Ничего не ломает на рантайме, но уменьшение расхождений между аннотациями и фактическим контрактом упростит дальнейшую поддержку.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Content.Client/_White/Wizard/Trail/TrailOverlay.cs(1 hunks)Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/ru-RU/_white/cards/danger.ftl:1-1
Timestamp: 2025-10-22T20:31:06.235Z
Learning: В PR по EvacPod для WWhiteDreamProject/wwdpublic: в ru-RU локализациях допустим осознанный сленг сообщества SS13 (напр., «дистры»), если он не ломает отображение и используется последовательно; не блокировать PR, предлагать необязательный follow-up (глоссарий/проверка консистентности).
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/en-US/_white/cards/hobby.ftl:5-5
Timestamp: 2025-10-22T20:13:31.389Z
Learning: В PR по EvacPod для репозитория WWhiteDreamProject/wwdpublic: если опечатка в ключе локализации (например, card-ep_hobby_mashrooms) не влияет на отображение и используется последовательно, автор предпочитает не блокировать PR; вместо этого предлагать необязательный follow-up issue.
🧬 Code graph analysis (1)
Content.Client/_White/Wizard/Trail/TrailOverlay.cs (1)
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs (1)
TrailData(188-206)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML map schema validator
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Test Packaging
- GitHub Check: YAML Linter
🔇 Additional comments (2)
Content.Client/_White/Wizard/Trail/TrailOverlay.cs (2)
1-32: Хорошая структура класса оверлея.Класс правильно наследуется от
Overlay, устанавливает корректное пространство рендеринга (WorldSpaceEntities) и Z-индекс для эффектов. Инициализация систем через DI выглядит корректно.
122-137: Cannot complete automated verification — repository is inaccessible and no public documentation exists for this private project.The original review comment recommends adding error handling to sprite frame access (lines 122, 137). However, based on learnings from the same repository (PR 146, same developer/codebase), the project maintainer has explicitly stated a preference not to add error handling for sprite issues, treating missing sprites as misconfiguration elsewhere rather than a runtime concern.
This suggests the review comment's recommendation may conflict with the project's established design philosophy. A manual discussion with the maintainer is needed to:
- Clarify whether
Frame0()andGetFrame()throw exceptions or return safe defaults- Confirm whether
trail.Spriteis validated before these lines (check line 102 context)- Determine if the project accepts the risk of unvalidated sprite access as intentional
| var time = _timing.CurTime > data.SpawnTime ? _timing.CurTime - data.SpawnTime : TimeSpan.Zero; | ||
| var texture = _sprite.GetFrame(trail.Sprite, time); | ||
|
|
||
| handle.SetTransform(Matrix3x2.Multiply(scaleMatrix, worldMatrix)); |
There was a problem hiding this comment.
Трансформация может "протекать" на последующие трейлы.
Если трейл использует рендеринг по спрайту (строка 139), то SetTransform устанавливается внутри цикла, но сбрасывается только в конце (строка 145). Последующие трейлы, использующие другие пути рендеринга (RenderedEntity или линии), могут унаследовать состояние трансформации предыдущего трейла.
Примените этот diff для сброса трансформации перед каждым трейлом:
var query = _entManager.EntityQueryEnumerator<TrailComponent, TransformComponent>();
while (query.MoveNext(out _, out var trail, out var xform))
{
+ handle.SetTransform(Matrix3x2.Identity);
+
if (trail.TrailData.Count == 0)
continue;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Content.Client/_White/Wizard/Trail/TrailOverlay.cs around line 139 (and up
through line 145), a transform set via
handle.SetTransform(Matrix3x2.Multiply(scaleMatrix, worldMatrix)) inside the
sprite-rendering branch can leak into subsequently rendered trails that use
other render paths; before each trail iteration (and before any branch that sets
a transform) reset the drawing transform to identity (e.g., call
handle.ResetTransform() or SetTransform(Matrix3x2.Identity)) so each trail
starts with a clean transform, then apply the intended scale/world transform
only for the sprite path and keep the existing final reset at the end.
| [DataField] | ||
| public LerpPropertyData.RenderedEntityRotationStrategy RenderedEntityRotationStrategy; |
There was a problem hiding this comment.
Непоследовательность с RenderedEntityRotationStrategy и незакрытый LerpPropertyData (ломает сборку)
Сейчас есть сразу две проблемы:
- Класс
LerpPropertyDataне закрыт перед объявлениемRenderedEntityRotationStrategy, из‑за чего компилятор ожидает}и падает. - Поле компонента объявлено как
LerpPropertyData.RenderedEntityRotationStrategy, тогда как enum по смыслу — отдельная стратегия вращения, а не вложенный тип вLerpPropertyData.
Предлагаю:
- Закрыть
LerpPropertyDataсразу послеLerpTarget. - Оставить enum верхнеуровневым.
- В компоненте использовать тип
RenderedEntityRotationStrategyнапрямую.
Пример правки:
- [DataField]
- public LerpPropertyData.RenderedEntityRotationStrategy RenderedEntityRotationStrategy;
+ [DataField]
+ public RenderedEntityRotationStrategy RenderedEntityRotationStrategy;
@@
[DataDefinition]
public sealed partial class LerpPropertyData
{
@@
- [DataField]
- public float LerpTarget = 1f;
-}
-
-public enum RenderedEntityRotationStrategy : byte
-{
- RenderedEntity = 0,
- Trail,
- Particle
-}
+ [DataField]
+ public float LerpTarget = 1f;
+}
+
+public enum RenderedEntityRotationStrategy : byte
+{
+ RenderedEntity = 0,
+ Trail,
+ Particle
+}После этого enum станет обычным публичным типом, и поле в TrailComponent будет к нему корректно привязано.
Also applies to: 208-228
🤖 Prompt for AI Agents
In Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs around lines 57-58
(also applies to lines 208-228), the file currently leaves the LerpPropertyData
type unclosed and declares RenderedEntityRotationStrategy as a nested member,
which breaks compilation; close the LerpPropertyData class immediately after the
LerpTarget definition with a closing brace, move the
RenderedEntityRotationStrategy enum out to top-level (public) scope, and change
the component field declaration to use the top-level
RenderedEntityRotationStrategy type directly (i.e., public
RenderedEntityRotationStrategy RenderedEntityRotationStrategy;), ensuring braces
and namespaces remain balanced.
| [ImplicitDataDefinitionForInheritors] | ||
| public partial interface IGetShaderData; | ||
| public abstract partial class GetShaderParam : IGetShaderData | ||
| { | ||
| [DataField(required: true)] | ||
| public string Param = string.Empty; | ||
| } | ||
| // Add more data if needed | ||
| public sealed partial class GetShaderLocalPositionData : IGetShaderData; | ||
|
|
||
| public sealed partial class GetShaderFloatParam : GetShaderParam; | ||
| }; |
There was a problem hiding this comment.
Некорректные объявления IGetShaderData/GetShaderLocalPositionData/GetShaderFloatParam и лишний }; (компиляция не пройдет)
Внизу файла несколько конструкций в C++‑стиле, которые в C# не компилируются:
[ImplicitDataDefinitionForInheritors]
public partial interface IGetShaderData;
...
public sealed partial class GetShaderLocalPositionData : IGetShaderData;
...
public sealed partial class GetShaderFloatParam : GetShaderParam;
};В C# тип (класс/интерфейс/struct/enum) всегда должен иметь тело в фигурных скобках; запись с одним ; после имени недопустима. Плюс финальный }; вообще не к чему относится.
Предлагаю оформить их как обычные (пока пустые) partial‑типы и убрать хвостовой };:
-[ImplicitDataDefinitionForInheritors]
-public partial interface IGetShaderData;
-public abstract partial class GetShaderParam : IGetShaderData
-{
- [DataField(required: true)]
- public string Param = string.Empty;
-}
-// Add more data if needed
-public sealed partial class GetShaderLocalPositionData : IGetShaderData;
-
-public sealed partial class GetShaderFloatParam : GetShaderParam;
-};
+[ImplicitDataDefinitionForInheritors]
+public partial interface IGetShaderData
+{
+}
+
+public abstract partial class GetShaderParam : IGetShaderData
+{
+ [DataField(required: true)]
+ public string Param = string.Empty;
+}
+
+// Add more data if needed
+public sealed partial class GetShaderLocalPositionData : IGetShaderData
+{
+}
+
+public sealed partial class GetShaderFloatParam : GetShaderParam
+{
+}Это снимет синтаксические ошибки и даст возможность расширять partial‑типы в других файлах, как задумано.
🤖 Prompt for AI Agents
Content.Shared/_White/Wizard/Projectiles/TrailComponent.cs around lines 230 to
241: в конце файла объявлены интерфейс и partial‑классы в C++‑стиле с
завершающими точками с запятой и присутствует лишний '};', что ломает
компиляцию; замените записи вида "public partial interface IGetShaderData;" и
"public sealed partial class X : Y;" на корректные пустые partial‑типы с телом в
фигурных скобках (например "public partial interface IGetShaderData { }" и
"public sealed partial class GetShaderLocalPositionData : IGetShaderData { }"),
аналогично для GetShaderFloatParam, и удалите финальный лишний '};'.
| List<(Type, string)> blockers = new() | ||
| { | ||
| (typeof(ChangelingComponent), "changeling"), | ||
| // You should be able to mindswap with heretics, | ||
| // but all of their data and abilities are not tied to their mind, I'm not making this work. | ||
| // (typeof(HereticComponent), "heretic"), | ||
| // (typeof(GhoulComponent), "ghoul"), | ||
| // Mindswapping with aghost real. | ||
| (typeof(GhostComponent), "ghost"), | ||
| (typeof(SpectralComponent), "ghost"), | ||
| (typeof(TimedDespawnComponent), "temporary"), | ||
| // (typeof(FadingTimedDespawnComponent), "temporary"), | ||
| }; | ||
|
|
||
| if (blockers.Any(x => CheckMindswapBlocker(x.Item1, x.Item2))) | ||
| return; | ||
| // Goobstation end |
There was a problem hiding this comment.
Этот следует заменить на что-то более менее подобающее. Я думаю лучше будет если в этой части кода будет вызываться хэндлед ивент, который будет направлен на цель. Если ивент будет помечен в какой-то, только тогда мы делаем ретернм
tigran-sargsyan-w
left a comment
tigran-sargsyan-w
left a comment
There was a problem hiding this comment.
Hey, jumping in as an outside dev – this looks really cool.
I like how the wizard logic and content are now more grouped under the _White/Wizard folders across client/server/shared, plus the prototypes and assets are collected in one place. That should make future wizard changes and spell additions a lot easier to reason about.
A couple of small thoughts from just reading the diff:
The PR is pretty huge (4k+ lines, a lot of assets and prototypes). It might be harder to review/rollback if something goes wrong. For future follow‑ups, it might be nice to split gameplay code changes, balance tweaks and asset imports into separate PRs when possible.
For the armor and protection changes on wizard gear, it might be worth double‑checking with whoever handles balance that the new resist values keep wizards in the intended power window.
The polymorph and trail systems look like they’re tying quite a few components together. If you haven’t already, some quick sanity tests for edge cases (cancelled polymorphs, trail cleanup when an entity is deleted, etc.) could help avoid weird ghosts hanging around in live rounds.
Overall, it feels like a solid step towards making wizard content less scattered and more maintainable. Nice work 👌
|
Вы думали я сдох? Забил на это? Нет, я вернулся! |
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
Content.Shared/Magic/SharedMagicSystem.cs (3)
494-497: Потенциальный NRE уже отмечен в предыдущем ревьюПроверка
basicAmmoComp.Count != nullнедостаточна — необходимо также проверятьCapacity != nullперед использованиемCapacity!.Value. См. предыдущий комментарий с предложенным исправлением.
609-614: Закомментированный код следует удалитьКак указано в предыдущем ревью, закомментированный код воспроизведения аудио следует либо реализовать, либо удалить. Оставлять закомментированный код в production-коде не рекомендуется.
622-694: Архитектурное замечание уже отмеченоКак указано в предыдущем ревью от Spatison, логику
TransferFactionsследует перенести в систему фракций (NpcFactionSystem) и переписать более подобающим образом. Текущая реализация работает, но нарушает принцип единственной ответственности.
🧹 Nitpick comments (1)
Content.Shared/Magic/SharedMagicSystem.cs (1)
83-87: Неиспользуемая зависимость_spells
SharedSpellsSystem _spellsобъявлена, но не используется в коде этого файла. Если планируется использование в будущем, рекомендуется добавить комментарий с пояснением. Иначе стоит удалить.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs(1 hunks)Content.Shared/Magic/SharedMagicSystem.cs(7 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/ru-RU/_white/cards/danger.ftl:1-1
Timestamp: 2025-10-22T20:31:06.235Z
Learning: В PR по EvacPod для WWhiteDreamProject/wwdpublic: в ru-RU локализациях допустим осознанный сленг сообщества SS13 (напр., «дистры»), если он не ломает отображение и используется последовательно; не блокировать PR, предлагать необязательный follow-up (глоссарий/проверка консистентности).
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/en-US/_white/cards/hobby.ftl:5-5
Timestamp: 2025-10-22T20:13:31.389Z
Learning: В PR по EvacPod для репозитория WWhiteDreamProject/wwdpublic: если опечатка в ключе локализации (например, card-ep_hobby_mashrooms) не влияет на отображение и используется последовательно, автор предпочитает не блокировать PR; вместо этого предлагать необязательный follow-up issue.
Learnt from: kekoven1
Repo: WWhiteDreamProject/wwdpublic PR: 813
File: Resources/Prototypes/Psionics/PsionicPowerPool.yml:1-18
Timestamp: 2025-08-27T23:57:37.337Z
Learning: ClonePower и другие WWDP-специфичные способности находятся в папке Resources/Prototypes/_White/, а не в основной папке Resources/Prototypes/Psionics/.
📚 Learning: 2025-08-09T13:01:08.902Z
Learnt from: RedFoxIV
Repo: WWhiteDreamProject/wwdpublic PR: 776
File: Content.Shared/_White/Guns/SharedGunFluxSystem.cs:0-0
Timestamp: 2025-08-09T13:01:08.902Z
Learning: В системе SharedGunFluxSystem существует инвариант: метод OnGunShot вызывается только после успешного выстрела, который возможен только при наличии flux core (проверяется в OnAttemptShoot). Использование DebugTools.Assert в OnGunShot — намеренное решение для обнаружения нарушения этого инварианта в debug-сборках.
Applied to files:
Content.Shared/Magic/SharedMagicSystem.cs
📚 Learning: 2024-12-17T19:29:23.754Z
Learnt from: Spatison
Repo: WWhiteDreamProject/wwdpublic PR: 169
File: Content.Server/WhiteDream/BloodCult/Items/ShadowShacklesAura/ShadowShacklesAuraSystem.cs:30-40
Timestamp: 2024-12-17T19:29:23.754Z
Learning: In the method `OnMeleeHit` in `ShadowShacklesAuraSystem.cs`, it's not necessary to check `args.HitEntities` for `null` before calling `Any()`, because the `if` operator handles `null` values appropriately.
Applied to files:
Content.Shared/Magic/SharedMagicSystem.cs
📚 Learning: 2025-09-24T18:18:06.673Z
Learnt from: RedFoxIV
Repo: WWhiteDreamProject/wwdpublic PR: 849
File: Content.Client/_White/UserInterface/WindowTracker.cs:1-5
Timestamp: 2025-09-24T18:18:06.673Z
Learning: The WWhiteDreamProject codebase uses GlobalUsings.cs files to provide global using directives. IoCManager is available throughout Content.Client without explicit using Robust.Shared.IoC; directive due to global usings configuration. Code analysis should account for global usings when checking for missing using statements.
Applied to files:
Content.Shared/Magic/SharedMagicSystem.cs
🧬 Code graph analysis (1)
Content.Shared/Magic/SharedMagicSystem.cs (1)
Content.Server/Ghost/GhostSystem.cs (2)
EntityUid(436-441)EntityUid(460-523)
🪛 GitHub Actions: Build & Test Debug
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[error] 52-52: CS0128: A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Actions: Build & Test Map Renderer
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[error] 52-52: CS0128: A local variable or function named 'endEffect' is already defined in this scope.
🪛 GitHub Actions: Test Packaging
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[error] 52-52: CS0128: A local variable or function named 'endEffect' is already defined in this scope.
🪛 GitHub Actions: YAML Linter
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[error] 52-52: CS0128: A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Check: build (ubuntu-latest)
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Check: Test Packaging
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Check: YAML Linter
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
🔇 Additional comments (2)
Content.Shared/Magic/SharedMagicSystem.cs (2)
141-146: LGTM!Изменение видимости
PassesSpellPrerequisitesнаpublicпозволяет внешним системам проверять предварительные условия заклинаний. Это согласуется с архитектурными изменениями PR.
707-727: Потеря состояния компонента при переносеМетод
TransferComponentсоздаёт новый экземпляр компонента черезFactory.GetComponent(type)с состоянием по умолчанию, а не копирует данные из исходного компонента. Если компонентыRevolutionaryComponent,HeadRevolutionaryComponent,WizardComponentилиApprenticeComponentсодержат runtime-состояние (например, флаги, счётчики, ссылки), оно будет потеряно.Если это намеренное поведение (сброс состояния при mind swap), стоит добавить комментарий. Иначе рассмотрите копирование состояния через сериализацию:
private void TransferComponent(Type type, EntityUid a, EntityUid b) { var aHasComp = HasComp(a, type); var bHasComp = HasComp(b, type); if (aHasComp && bHasComp) return; - var comp = Factory.GetComponent(type); if (aHasComp) { + var sourceComp = Comp(a, type); + var comp = (Component)_seriMan.Copy(sourceComp)!; AddComp(b, comp); RemCompDeferred(a, type); } else if (bHasComp) { + var sourceComp = Comp(b, type); + var comp = (Component)_seriMan.Copy(sourceComp)!; AddComp(a, comp); RemCompDeferred(b, type); } }
| public override void Update(float frameTime) | ||
| { | ||
| base.Update(frameTime); | ||
|
|
||
| var trailQuery = GetEntityQuery<TrailComponent>(); | ||
|
|
||
| var query = EntityQueryEnumerator<WizardJauntComponent, PolymorphedEntityComponent, TransformComponent>(); | ||
| while (query.MoveNext(out var uid, out var jaunt, out var polymorphed, out var xform)) | ||
| { | ||
| if (jaunt.JauntEndEffectEntity is {} endEffect) | ||
| { | ||
| _transform.SetMapCoordinates(endEffect, _transform.GetMapCoordinates(xform)); | ||
| continue; | ||
| } | ||
|
|
||
| jaunt.DurationBetweenEffects -= frameTime; | ||
|
|
||
| if (jaunt.DurationBetweenEffects > 0f) | ||
| continue; | ||
|
|
||
| var endEffect = Spawn(jaunt.JauntEndEffect, | ||
| _transform.GetMapCoordinates(uid, xform), | ||
| rotation: _transform.GetWorldRotation(xform)); | ||
|
|
||
| _audio.PlayEntity(jaunt.JauntEndSound, Filter.Pvs(endEffect), endEffect, true); | ||
|
|
||
| jaunt.JauntEndEffectEntity = endEffect; | ||
|
|
||
| if (!TryComp(endEffect, out TrailComponent? trail)) | ||
| continue; | ||
|
|
||
| trail.RenderedEntity = polymorphed.Parent; | ||
| Dirty(endEffect, trail); | ||
| } | ||
| } |
There was a problem hiding this comment.
Критическая ошибка компиляции: дублирование имени переменной endEffect.
Переменная endEffect объявлена дважды в одной области видимости цикла while:
- Строка 41: паттерн-матчинг
is {} endEffectобъявляет переменную во внешней области цикла - Строка 52:
var endEffect = Spawn(...)пытается объявить переменную с тем же именем
Это вызывает ошибку компилятора CS0128, блокирующую сборку проекта.
Также обратите внимание: когда jaunt.JauntEndEffectEntity уже существует (строки 41-44), его позиция обновляется каждый кадр, но не видно логики очистки или удаления этой сущности. Убедитесь, что JauntEndEffectEntity корректно очищается в другом месте системы.
Примените этот diff для исправления конфликта имён:
var query = EntityQueryEnumerator<WizardJauntComponent, PolymorphedEntityComponent, TransformComponent>();
while (query.MoveNext(out var uid, out var jaunt, out var polymorphed, out var xform))
{
- if (jaunt.JauntEndEffectEntity is {} endEffect)
+ if (jaunt.JauntEndEffectEntity is {} existingEndEffect)
{
- _transform.SetMapCoordinates(endEffect, _transform.GetMapCoordinates(xform));
+ _transform.SetMapCoordinates(existingEndEffect, _transform.GetMapCoordinates(xform));
continue;
}
jaunt.DurationBetweenEffects -= frameTime;
if (jaunt.DurationBetweenEffects > 0f)
continue;
var endEffect = Spawn(jaunt.JauntEndEffect,
_transform.GetMapCoordinates(uid, xform),
rotation: _transform.GetWorldRotation(xform));
_audio.PlayEntity(jaunt.JauntEndSound, Filter.Pvs(endEffect), endEffect, true);
jaunt.JauntEndEffectEntity = endEffect;
if (!TryComp(endEffect, out TrailComponent? trail))
continue;
trail.RenderedEntity = polymorphed.Parent;
Dirty(endEffect, trail);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public override void Update(float frameTime) | |
| { | |
| base.Update(frameTime); | |
| var trailQuery = GetEntityQuery<TrailComponent>(); | |
| var query = EntityQueryEnumerator<WizardJauntComponent, PolymorphedEntityComponent, TransformComponent>(); | |
| while (query.MoveNext(out var uid, out var jaunt, out var polymorphed, out var xform)) | |
| { | |
| if (jaunt.JauntEndEffectEntity is {} endEffect) | |
| { | |
| _transform.SetMapCoordinates(endEffect, _transform.GetMapCoordinates(xform)); | |
| continue; | |
| } | |
| jaunt.DurationBetweenEffects -= frameTime; | |
| if (jaunt.DurationBetweenEffects > 0f) | |
| continue; | |
| var endEffect = Spawn(jaunt.JauntEndEffect, | |
| _transform.GetMapCoordinates(uid, xform), | |
| rotation: _transform.GetWorldRotation(xform)); | |
| _audio.PlayEntity(jaunt.JauntEndSound, Filter.Pvs(endEffect), endEffect, true); | |
| jaunt.JauntEndEffectEntity = endEffect; | |
| if (!TryComp(endEffect, out TrailComponent? trail)) | |
| continue; | |
| trail.RenderedEntity = polymorphed.Parent; | |
| Dirty(endEffect, trail); | |
| } | |
| } | |
| public override void Update(float frameTime) | |
| { | |
| base.Update(frameTime); | |
| var trailQuery = GetEntityQuery<TrailComponent>(); | |
| var query = EntityQueryEnumerator<WizardJauntComponent, PolymorphedEntityComponent, TransformComponent>(); | |
| while (query.MoveNext(out var uid, out var jaunt, out var polymorphed, out var xform)) | |
| { | |
| if (jaunt.JauntEndEffectEntity is {} existingEndEffect) | |
| { | |
| _transform.SetMapCoordinates(existingEndEffect, _transform.GetMapCoordinates(xform)); | |
| continue; | |
| } | |
| jaunt.DurationBetweenEffects -= frameTime; | |
| if (jaunt.DurationBetweenEffects > 0f) | |
| continue; | |
| var endEffect = Spawn(jaunt.JauntEndEffect, | |
| _transform.GetMapCoordinates(uid, xform), | |
| rotation: _transform.GetWorldRotation(xform)); | |
| _audio.PlayEntity(jaunt.JauntEndSound, Filter.Pvs(endEffect), endEffect, true); | |
| jaunt.JauntEndEffectEntity = endEffect; | |
| if (!TryComp(endEffect, out TrailComponent? trail)) | |
| continue; | |
| trail.RenderedEntity = polymorphed.Parent; | |
| Dirty(endEffect, trail); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Build & Test Debug
[error] 52-52: CS0128: A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Actions: Build & Test Map Renderer
[error] 52-52: CS0128: A local variable or function named 'endEffect' is already defined in this scope.
🪛 GitHub Actions: Test Packaging
[error] 52-52: CS0128: A local variable or function named 'endEffect' is already defined in this scope.
🪛 GitHub Actions: YAML Linter
[error] 52-52: CS0128: A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Check: build (ubuntu-latest)
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Check: Test Packaging
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Check: YAML Linter
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 56-56:
Use of unassigned local variable 'endEffect'
[failure] 52-52:
A local variable or function named 'endEffect' is already defined in this scope
🤖 Prompt for AI Agents
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs lines 32-66: the
variable name endEffect is declared twice in the same scope (once by the pattern
match "is {} endEffect" and again with "var endEffect = Spawn(...)"), causing
CS0128; fix by renaming one of the variables (e.g., rename the pattern-matched
variable to existingEndEffect or the spawned variable to spawnedEndEffect) or
avoid redeclaring by assigning to jaunt.JauntEndEffectEntity when present; after
renaming/adjusting, update subsequent uses (setting map coordinates, trail
assignment, Dirty call) to the new identifier, and ensure that
JauntEndEffectEntity is cleared/removed elsewhere as noted.
| if (_mobState.IsIncapacitated(ev.Target) || HasComp<ZombieComponent>(ev.Target)) | ||
| { | ||
| _popup.PopupClient(Loc.GetString("spell-fail-mindswap-dead"), ev.Performer, ev.Performer); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Пропущен ev.Handled = true перед ранним возвратом
При проверке на IsIncapacitated или ZombieComponent код возвращается без установки ev.Handled = true. Это может привести к повторной обработке события другими обработчиками. Для консистентности с блоком выше (строки 544-548) следует установить флаг:
if (_mobState.IsIncapacitated(ev.Target) || HasComp<ZombieComponent>(ev.Target))
{
_popup.PopupClient(Loc.GetString("spell-fail-mindswap-dead"), ev.Performer, ev.Performer);
+ ev.Handled = true;
return;
}🤖 Prompt for AI Agents
In Content.Shared/Magic/SharedMagicSystem.cs around lines 550 to 554, the early
return when the target is incapacitated or a zombie omits setting ev.Handled =
true; add ev.Handled = true just before the return to match the earlier check
(lines ~544-548) so the event is marked handled and other handlers won't
reprocess it.
| if (blockers.Any(x => CheckMindswapBlocker(x.Item1, x.Item2))) | ||
| return; |
There was a problem hiding this comment.
Пропущен ev.Handled = true в блоке blockers
Аналогично предыдущему замечанию — при срабатывании блокировщика (строка 570-571) событие не помечается как обработанное:
if (blockers.Any(x => CheckMindswapBlocker(x.Item1, x.Item2)))
+ {
+ ev.Handled = true;
return;
+ }Также учтите предыдущий комментарий от Spatison о рефакторинге этого блока через handled event pattern.
🤖 Prompt for AI Agents
In Content.Shared/Magic/SharedMagicSystem.cs around lines 570-571, the code
returns when a blockers.Any check succeeds but does not mark the event as
handled; update the branch so that when a blockers blocker is found you set
ev.Handled = true before returning, and (per Spatison’s prior note) refactor
this block to the handled-event pattern: determine the blocker, set ev.Handled =
true and then return, or replace the lambda with a method that returns a boolean
indicating handled and sets ev.Handled accordingly to keep the control flow
explicit and testable.
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
Co-authored-by: Spatison <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs (1)
32-63: Критическая ошибка компиляции: дублирование имени переменнойendEffectблокирует сборку.В методе
UpdateпеременнаяendEffectобъявлена дважды в одной области видимости:
- Строка 39: паттерн
is {} endEffectобъявляет переменную- Строка 50:
var endEffect = Spawn(...)пытается объявить переменную с тем же именемЭто вызывает ошибку компилятора CS0128, которая блокирует все сборки проекта (Debug, Release, Map Renderer, Packaging).
Примените этот diff для исправления конфликта имён:
var query = EntityQueryEnumerator<WizardJauntComponent, PolymorphedEntityComponent, TransformComponent>(); while (query.MoveNext(out var uid, out var jaunt, out var polymorphed, out var xform)) { - if (jaunt.JauntEndEffectEntity is {} endEffect) + if (jaunt.JauntEndEffectEntity is {} existingEndEffect) { - _transform.SetMapCoordinates(endEffect, _transform.GetMapCoordinates(xform)); + _transform.SetMapCoordinates(existingEndEffect, _transform.GetMapCoordinates(xform)); continue; } jaunt.DurationBetweenEffects -= frameTime; if (jaunt.DurationBetweenEffects > 0f) continue; var endEffect = Spawn(jaunt.JauntEndEffect, _transform.GetMapCoordinates(uid, xform), rotation: _transform.GetWorldRotation(xform)); _audio.PlayEntity(jaunt.JauntEndSound, Filter.Pvs(endEffect), endEffect, true); jaunt.JauntEndEffectEntity = endEffect; if (!TryComp(endEffect, out TrailComponent? trail)) continue; trail.RenderedEntity = polymorphed.Parent; Dirty(endEffect, trail); }Content.Shared/Magic/SharedMagicSystem.cs (3)
494-497: Потенциальный NullReferenceException при отсутствии Capacity.В строке 495 проверяется только
basicAmmoComp.Count != null, но затем безусловно используетсяCapacity!.Value. ЕслиCapacity == null, произойдёт исключение.Примените паттерн-матчинг для проверки обоих свойств:
- if (TryComp<BasicEntityAmmoProviderComponent>(wand, out var basicAmmoComp) && basicAmmoComp.Count != null) - _gunSystem.UpdateBasicEntityAmmoCount(wand.Value, basicAmmoComp.Capacity!.Value, basicAmmoComp); // WD edit - recharge to max + if (TryComp<BasicEntityAmmoProviderComponent>(wand, out var basicAmmoComp) && + basicAmmoComp is { Count: not null, Capacity: not null }) + { + _gunSystem.UpdateBasicEntityAmmoCount(wand.Value, basicAmmoComp.Capacity.Value, basicAmmoComp); + } else if (TryComp<LimitedChargesComponent>(wand, out var charges)) - _charges.SetCharges(wand.Value, charges.MaxCharges, charges.MaxCharges); // WD edit - recharge to max + _charges.SetCharges(wand.Value, charges.MaxCharges, charges.MaxCharges);
550-554: Пропущена установкаev.Handled = trueперед возвратом.При проверке на
IsIncapacitatedилиZombieComponentкод возвращается без установкиev.Handled = true. Это может привести к повторной обработке события другими обработчиками.if (_mobState.IsIncapacitated(ev.Target) || HasComp<ZombieComponent>(ev.Target)) { _popup.PopupClient(Loc.GetString("spell-fail-mindswap-dead"), ev.Performer, ev.Performer); + ev.Handled = true; return; }
556-572: Пропущена установкаev.Handled = trueи закомментированный код.При срабатывании блокировщика (строка 570) событие не помечается как обработанное. Кроме того, присутствует закомментированный код, который следует либо удалить, либо раскомментировать при необходимости.
if (blockers.Any(x => CheckMindswapBlocker(x.Item1, x.Item2))) + { + ev.Handled = true; return; + }Примечание: Spatison ранее предложил рефакторинг этого блока с использованием handled event pattern для улучшения архитектуры и тестируемости.
🧹 Nitpick comments (1)
Content.Shared/Magic/SharedMagicSystem.cs (1)
616-688: Логика трансфера фракций: архитектурное замечание.Вложенная функция
TransferFactionsсодержит сложную логику работы с фракциями (72 строки). Spatison ранее предложил переместить эту логику в систему фракций для улучшения архитектуры и переиспользования.Рассмотрите возможность:
- Перенести логику трансфера фракций в
NpcFactionSystemкак публичный методTransferWizardFactions(EntityUid source, EntityUid target)- Упростить текущий код до вызова:
_faction.TransferWizardFactions(ev.Performer, ev.Target)Это улучшит тестируемость, переиспользование и соответствие принципу единственной ответственности.
По замечанию из retrieved learnings (Spatison, PR 815, XenomorphQueenSystem).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Content.Server/Power/EntitySystems/BatterySystem.cs(1 hunks)Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs(1 hunks)Content.Shared/Magic/SharedMagicSystem.cs(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Content.Server/Power/EntitySystems/BatterySystem.cs
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/ru-RU/_white/cards/danger.ftl:1-1
Timestamp: 2025-10-22T20:31:06.235Z
Learning: В PR по EvacPod для WWhiteDreamProject/wwdpublic: в ru-RU локализациях допустим осознанный сленг сообщества SS13 (напр., «дистры»), если он не ломает отображение и используется последовательно; не блокировать PR, предлагать необязательный follow-up (глоссарий/проверка консистентности).
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/en-US/_white/cards/hobby.ftl:5-5
Timestamp: 2025-10-22T20:13:31.389Z
Learning: В PR по EvacPod для репозитория WWhiteDreamProject/wwdpublic: если опечатка в ключе локализации (например, card-ep_hobby_mashrooms) не влияет на отображение и используется последовательно, автор предпочитает не блокировать PR; вместо этого предлагать необязательный follow-up issue.
📚 Learning: 2025-08-09T13:01:08.902Z
Learnt from: RedFoxIV
Repo: WWhiteDreamProject/wwdpublic PR: 776
File: Content.Shared/_White/Guns/SharedGunFluxSystem.cs:0-0
Timestamp: 2025-08-09T13:01:08.902Z
Learning: В системе SharedGunFluxSystem существует инвариант: метод OnGunShot вызывается только после успешного выстрела, который возможен только при наличии flux core (проверяется в OnAttemptShoot). Использование DebugTools.Assert в OnGunShot — намеренное решение для обнаружения нарушения этого инварианта в debug-сборках.
Applied to files:
Content.Shared/Magic/SharedMagicSystem.cs
📚 Learning: 2024-12-17T19:29:23.754Z
Learnt from: Spatison
Repo: WWhiteDreamProject/wwdpublic PR: 169
File: Content.Server/WhiteDream/BloodCult/Items/ShadowShacklesAura/ShadowShacklesAuraSystem.cs:30-40
Timestamp: 2024-12-17T19:29:23.754Z
Learning: In the method `OnMeleeHit` in `ShadowShacklesAuraSystem.cs`, it's not necessary to check `args.HitEntities` for `null` before calling `Any()`, because the `if` operator handles `null` values appropriately.
Applied to files:
Content.Shared/Magic/SharedMagicSystem.cs
📚 Learning: 2025-08-28T20:09:07.328Z
Learnt from: Spatison
Repo: WWhiteDreamProject/wwdpublic PR: 815
File: Content.Server/_White/Xenomorphs/Queen/XenomorphQueenSystem.cs:42-47
Timestamp: 2025-08-28T20:09:07.328Z
Learning: В XenomorphQueenSystem при тогgle выключении промоушена (удалении существующего promotion item) не нужно помечать событие как обработанное (args.Handled = true), поскольку это приведет к трате плазмы при отключении, что нежелательно с точки зрения игрового дизайна.
Applied to files:
Content.Shared/Magic/SharedMagicSystem.cs
📚 Learning: 2025-08-06T11:28:07.018Z
Learnt from: Spatison
Repo: WWhiteDreamProject/wwdpublic PR: 759
File: Content.Server/_White/Xenomorphs/FaceHugger/FaceHuggerSystem.cs:23-31
Timestamp: 2025-08-06T11:28:07.018Z
Learning: В системе FaceHuggerSystem метод OnGotEquipped проверяет состояние самого лицехвата (uid) через _mobState.IsAlive(uid), а не жертвы (args.Equipee). Это корректно, так как мертвый лицехват не должен быть способен к заражению.
Applied to files:
Content.Shared/Magic/SharedMagicSystem.cs
📚 Learning: 2025-09-24T18:18:06.673Z
Learnt from: RedFoxIV
Repo: WWhiteDreamProject/wwdpublic PR: 849
File: Content.Client/_White/UserInterface/WindowTracker.cs:1-5
Timestamp: 2025-09-24T18:18:06.673Z
Learning: The WWhiteDreamProject codebase uses GlobalUsings.cs files to provide global using directives. IoCManager is available throughout Content.Client without explicit using Robust.Shared.IoC; directive due to global usings configuration. Code analysis should account for global usings when checking for missing using statements.
Applied to files:
Content.Shared/Magic/SharedMagicSystem.cs
🧬 Code graph analysis (1)
Content.Shared/Magic/SharedMagicSystem.cs (2)
Content.Server/Ghost/GhostSystem.cs (2)
EntityUid(436-441)EntityUid(460-523)Content.Server/_Goobstation/Changeling/ChangelingSystem.cs (2)
IsIncapacitated(285-292)ChangelingComponent(492-509)
🪛 GitHub Actions: Build & Test Debug
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[error] 50-50: CS0128: A local variable or function named 'endEffect' is already defined in this scope.
🪛 GitHub Actions: Build & Test Map Renderer
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[error] 50-50: CS0128: A local variable or function named 'endEffect' is already defined in this scope.
🪛 GitHub Actions: Test Packaging
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[error] 50-50: CS0128: A local variable or function named 'endEffect' is already defined in this scope. Build failed due to duplicate local variable in WizardJauntSystem.cs (line 50).
🪛 GitHub Actions: YAML Linter
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[error] 50-50: CS0128: A local variable or function named 'endEffect' is already defined in this scope.
🪛 GitHub Check: build (ubuntu-latest)
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[failure] 54-54:
Use of unassigned local variable 'endEffect'
[failure] 50-50:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 54-54:
Use of unassigned local variable 'endEffect'
[failure] 50-50:
A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Check: Test Packaging
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[failure] 54-54:
Use of unassigned local variable 'endEffect'
[failure] 50-50:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 54-54:
Use of unassigned local variable 'endEffect'
[failure] 50-50:
A local variable or function named 'endEffect' is already defined in this scope
🪛 GitHub Check: YAML Linter
Content.Server/_White/Wizard/Systems/WizardJauntSystem.cs
[failure] 54-54:
Use of unassigned local variable 'endEffect'
[failure] 50-50:
A local variable or function named 'endEffect' is already defined in this scope
[failure] 54-54:
Use of unassigned local variable 'endEffect'
[failure] 50-50:
A local variable or function named 'endEffect' is already defined in this scope
🔇 Additional comments (4)
Content.Shared/Magic/SharedMagicSystem.cs (4)
1-1: Новые импорты для поддержки расширенной магической системы.Добавленные using-директивы (System.Linq, Wizard, Changeling, Ghost, Mobs, NPC, Revolutionary, Zombies) необходимы для новой функциональности заклинаний и проверок mindswap. Изменения корректны.
Also applies to: 4-4, 8-8, 15-15, 25-28, 31-31, 38-38
83-87: Добавлены новые зависимости для WD-функциональности.Новые системные зависимости (MobStateSystem, NpcFactionSystem, SharedSpellsSystem) корректно инжектируются и используются в расширенной логике заклинаний.
543-548: Корректная проверка запрета заклинаний касания.Проверка через
IsTouchSpellDeniedвыполнена правильно с установкойev.Handled = trueперед возвратом. Логика соответствует best practices.
141-141: Изменение видимости метода обоснованно: требуется для интеграции систем.Метод
PassesSpellPrerequisitesиспользуется системойSharedSpellsSystem(строки 46, 108 вSharedSpellsSystem.cs) через внедрённую зависимость для проверки предпосылок заклинаний. Публичность метода необходима для взаимодействия между системами Wizard.
| private void TransferComponent(Type type, EntityUid a, EntityUid b) | ||
| { | ||
| var aHasComp = HasComp(a, type); | ||
| var bHasComp = HasComp(b, type); | ||
|
|
||
| if (aHasComp && bHasComp) | ||
| return; | ||
|
|
||
| var comp = Factory.GetComponent(type); | ||
| if (aHasComp) | ||
| { | ||
| AddComp(b, comp); | ||
| RemCompDeferred(a, type); | ||
| } | ||
| else if (bHasComp) | ||
| { | ||
| AddComp(a, comp); | ||
| RemCompDeferred(b, type); | ||
| } | ||
| } |
There was a problem hiding this comment.
Метод TransferComponent не копирует состояние компонента.
Метод создаёт новый пустой компонент через Factory.GetComponent(type) и добавляет его к целевой сущности, но не копирует данные из исходного компонента. Это может привести к потере состояния для компонентов типа RevolutionaryComponent, WizardComponent и других.
Рассмотрите использование сериализации для копирования состояния компонента (аналогично методу AddComponents в строках 393-405) или убедитесь, что переносимые компоненты не содержат важного состояния.
private void TransferComponent(Type type, EntityUid a, EntityUid b)
{
var aHasComp = HasComp(a, type);
var bHasComp = HasComp(b, type);
if (aHasComp && bHasComp)
return;
- var comp = Factory.GetComponent(type);
if (aHasComp)
{
+ var sourceComp = EntityManager.GetComponent(a, type);
+ var comp = (Component)_compFact.GetComponent(type.Name);
+ var temp = (object)comp;
+ _seriMan.CopyTo(sourceComp, ref temp);
- AddComp(b, comp);
+ EntityManager.AddComponent(b, (Component)temp!);
RemCompDeferred(a, type);
}
else if (bHasComp)
{
+ var sourceComp = EntityManager.GetComponent(b, type);
+ var comp = (Component)_compFact.GetComponent(type.Name);
+ var temp = (object)comp;
+ _seriMan.CopyTo(sourceComp, ref temp);
- AddComp(a, comp);
+ EntityManager.AddComponent(a, (Component)temp!);
RemCompDeferred(b, type);
}
}Committable suggestion skipped: line range outside the PR's diff.
Co-authored-by: Spatison <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
Content.Shared/_White/Wizard/SharedSpellsSystem.cs (5)
22-22: Рассмотрите перемещение в namespaceContent.Shared._White.Spells.Как уже было отмечено в предыдущем ревью, организация по функциональности (Spells) может быть предпочтительнее, чем по роли (Wizard), особенно если система будет использоваться для различных типов заклинаний.
27-39: Форматирование зависимостей уже обсуждалось.Предыдущее ревью предложило конкретное форматирование для этих зависимостей. Обратите внимание на смешение модификаторов доступа (protected vs private) - убедитесь, что это осознанное решение для разграничения использования в производных классах.
41-45: Инициализация выглядит корректно.Подписки на события уже отформатированы согласно предыдущим замечаниям.
147-148: Уберите лишние пустые строки.Две пустые строки перед
#endregionне нужны.
109-115: Рассмотрите видимость метода и базовое поведение.Метод
OnPolymorphявляетсяprivate, что не позволяет переопределить его в производных классах. Предыдущее ревью предлагало сделать егоprotected virtual.Также обратите внимание: базовая реализация
Polymorph(строки 158-161) всегда возвращаетtrue, что означает, чтоev.Handledвсегда будет установлен вtrue, даже если полиморф фактически не произошёл. Убедитесь, что это желаемое поведение.🔎 Предлагаемое изменение согласно предыдущему ревью
-private void OnPolymorph(PolymorphSpellEvent ev) +protected virtual void OnPolymorph(PolymorphSpellEvent ev) { if (ev.Handled || !_magic.PassesSpellPrerequisites(ev.Action, ev.Performer)) return; - ev.Handled = Polymorph(ev); + ev.Handled = true; + Polymorph(ev); }
🧹 Nitpick comments (1)
Content.Shared/_White/Wizard/SharedSpellsSystem.cs (1)
131-145: Метод RechargeAllSpells работает корректно, но можно улучшить читаемость.Логика корректна, но LINQ-выражение на строках 134-136 можно упростить для лучшей читаемости.
🔎 Предлагаемое упрощение
private bool RechargeAllSpells(EntityUid uid, EntityUid? except = null) { var magicQuery = GetEntityQuery<MagicComponent>(); - var ents = except != null - ? Actions.GetActions(uid).Where(x => x.Id != except.Value && magicQuery.HasComp((EntityUid)x.Id)) - : Actions.GetActions(uid).Where(x => magicQuery.HasComp(x.Id)); + var actions = Actions.GetActions(uid) + .Where(x => x.Id != except && magicQuery.HasComp(x.Id)); + var hasSpells = false; - foreach (var (ent, _) in ents) + foreach (var (ent, _) in actions) { hasSpells = true; Actions.SetCooldown(ent, TimeSpan.Zero); } return hasSpells; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Content.Shared/_White/Wizard/SharedSpellsSystem.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/ru-RU/_white/cards/danger.ftl:1-1
Timestamp: 2025-10-22T20:31:06.235Z
Learning: В PR по EvacPod для WWhiteDreamProject/wwdpublic: в ru-RU локализациях допустим осознанный сленг сообщества SS13 (напр., «дистры»), если он не ломает отображение и используется последовательно; не блокировать PR, предлагать необязательный follow-up (глоссарий/проверка консистентности).
Learnt from: Lachklen
Repo: WWhiteDreamProject/wwdpublic PR: 863
File: Resources/Locale/en-US/_white/cards/hobby.ftl:5-5
Timestamp: 2025-10-22T20:13:31.389Z
Learning: В PR по EvacPod для репозитория WWhiteDreamProject/wwdpublic: если опечатка в ключе локализации (например, card-ep_hobby_mashrooms) не влияет на отображение и используется последовательно, автор предпочитает не блокировать PR; вместо этого предлагать необязательный follow-up issue.
📚 Learning: 2025-08-27T23:57:37.337Z
Learnt from: kekoven1
Repo: WWhiteDreamProject/wwdpublic PR: 813
File: Resources/Prototypes/Psionics/PsionicPowerPool.yml:1-18
Timestamp: 2025-08-27T23:57:37.337Z
Learning: ClonePower и другие WWDP-специфичные способности находятся в папке Resources/Prototypes/_White/, а не в основной папке Resources/Prototypes/Psionics/.
Applied to files:
Content.Shared/_White/Wizard/SharedSpellsSystem.cs
📚 Learning: 2024-12-09T16:50:36.059Z
Learnt from: Spatison
Repo: WWhiteDreamProject/wwdpublic PR: 147
File: Content.Server/_White/Atmos/Reactions/NitriumDecompositionReaction.cs:7-7
Timestamp: 2024-12-09T16:50:36.059Z
Learning: В проекте используется пространство имен `Content.Server._White` с подчёркиванием, чтобы облегчить поиск файлов, уникальных для данного форка.
Applied to files:
Content.Shared/_White/Wizard/SharedSpellsSystem.cs
🧬 Code graph analysis (1)
Content.Shared/_White/Wizard/SharedSpellsSystem.cs (2)
Content.Shared/Actions/SharedActionsSystem.cs (1)
SharedActionsSystem(26-1225)Content.Shared/Magic/SharedMagicSystem.cs (2)
SharedMagicSystem(58-744)PassesSpellPrerequisites(141-146)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML map schema validator
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML Linter
- GitHub Check: Test Packaging
🔇 Additional comments (1)
Content.Shared/_White/Wizard/SharedSpellsSystem.cs (1)
119-119: Хороший выбор абстрактного метода.Объявление
CreateChargeEffectкак абстрактного метода правильно делегирует реализацию визуальных эффектов производным классам (клиентским/серверным).
| private void OnCharge(ChargeMagicEvent ev) | ||
| { | ||
| if (ev.Handled || !_magic.PassesSpellPrerequisites(ev.Action, ev.Performer)) | ||
| return; | ||
|
|
||
| ev.Handled = true; | ||
|
|
||
| var raysEv = new ChargeSpellRaysEffectEvent(GetNetEntity(ev.Performer)); | ||
| CreateChargeEffect(ev.Performer, raysEv); | ||
|
|
||
| if (TryComp<PullerComponent>(ev.Performer, out var puller) && HasComp<PullableComponent>(puller.Pulling) && | ||
| RechargePerson(puller.Pulling.Value)) | ||
| return; | ||
|
|
||
| if (TryComp(ev.Performer, out CarryingComponent? carrying) && RechargePerson(carrying.Carried)) | ||
| return; | ||
|
|
||
| if (!TryComp(ev.Performer, out HandsComponent? hands)) | ||
| return; | ||
|
|
||
| foreach (var item in Hands.EnumerateHeld(ev.Performer, hands)) | ||
| { | ||
| if (Tag.HasAnyTag(item, ev.RechargeTags)) | ||
| { | ||
| if (TryComp<LimitedChargesComponent>(item, out var limitedCharges)) | ||
| { | ||
| _charges.SetCharges(item, limitedCharges.MaxCharges, limitedCharges.MaxCharges); | ||
| PopupCharged(item, ev.Performer); | ||
| break; | ||
| } | ||
|
|
||
| if (TryComp<BasicEntityAmmoProviderComponent>(item, out var basicAmmoComp) && | ||
| basicAmmoComp is { Count: not null, Capacity: not null } && | ||
| basicAmmoComp.Count < basicAmmoComp.Capacity) | ||
| { | ||
| _gunSystem.UpdateBasicEntityAmmoCount(item, basicAmmoComp.Capacity.Value, basicAmmoComp); | ||
| PopupCharged(item, ev.Performer); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (ChargeItem(item, ev)) | ||
| break; | ||
| } | ||
|
|
||
| return; | ||
|
|
||
| bool RechargePerson(EntityUid uid) | ||
| { | ||
| if (RechargeAllSpells(uid)) | ||
| { | ||
| PopupCharged(uid, ev.Performer, false); | ||
| _popup.PopupEntity(Loc.GetString("spell-charge-spells-charged-pulled"), uid, uid, PopupType.Medium); | ||
| ev.Handled = true; | ||
| return true; | ||
| } | ||
|
|
||
| _popup.PopupEntity(Loc.GetString("spell-charge-no-spells-to-charge-pulled"), uid, uid, PopupType.Medium); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Проблема с обработкой события и потоком выполнения.
Обнаружено несколько проблем:
-
Строка 52:
ev.Handled = trueустанавливается до того, как что-либо фактически заряжено. Если ни один предмет не будет заряжен, событие всё равно будет помечено как обработанное. -
Строка 100: Повторная установка
ev.Handled = trueв функцииRechargePersonизбыточна, так как значение уже установлено на строке 52. -
Строка 92: Лишний оператор
return;в конце метода. -
Логика зарядки: Если пользователь не тянет/не несёт никого, или ничего не держит в руках, событие всё равно помечается как обработанное (строка 52), но никакой обратной связи пользователю не предоставляется.
Рекомендация: Устанавливайте ev.Handled = true только после успешной зарядки чего-либо. Можно использовать локальную переменную для отслеживания успеха и установить ev.Handled в конце метода.
🔎 Предлагаемое исправление
private void OnCharge(ChargeMagicEvent ev)
{
if (ev.Handled || !_magic.PassesSpellPrerequisites(ev.Action, ev.Performer))
return;
- ev.Handled = true;
+ var charged = false;
var raysEv = new ChargeSpellRaysEffectEvent(GetNetEntity(ev.Performer));
CreateChargeEffect(ev.Performer, raysEv);
if (TryComp<PullerComponent>(ev.Performer, out var puller) && HasComp<PullableComponent>(puller.Pulling) &&
RechargePerson(puller.Pulling.Value))
+ {
+ ev.Handled = true;
return;
+ }
if (TryComp(ev.Performer, out CarryingComponent? carrying) && RechargePerson(carrying.Carried))
+ {
+ ev.Handled = true;
return;
+ }
if (!TryComp(ev.Performer, out HandsComponent? hands))
return;
foreach (var item in Hands.EnumerateHeld(ev.Performer, hands))
{
if (Tag.HasAnyTag(item, ev.RechargeTags))
{
if (TryComp<LimitedChargesComponent>(item, out var limitedCharges))
{
_charges.SetCharges(item, limitedCharges.MaxCharges, limitedCharges.MaxCharges);
PopupCharged(item, ev.Performer);
+ charged = true;
break;
}
if (TryComp<BasicEntityAmmoProviderComponent>(item, out var basicAmmoComp) &&
basicAmmoComp is { Count: not null, Capacity: not null } &&
basicAmmoComp.Count < basicAmmoComp.Capacity)
{
_gunSystem.UpdateBasicEntityAmmoCount(item, basicAmmoComp.Capacity.Value, basicAmmoComp);
PopupCharged(item, ev.Performer);
+ charged = true;
break;
}
}
if (ChargeItem(item, ev))
+ {
+ charged = true;
break;
+ }
}
- return;
+ ev.Handled = charged;
bool RechargePerson(EntityUid uid)
{
if (RechargeAllSpells(uid))
{
PopupCharged(uid, ev.Performer, false);
_popup.PopupEntity(Loc.GetString("spell-charge-spells-pulled"), uid, uid, PopupType.Medium);
- ev.Handled = true;
return true;
}
_popup.PopupEntity(Loc.GetString("spell-charge-no-spells-to-charge-pulled"), uid, uid, PopupType.Medium);
return false;
}
}🤖 Prompt for AI Agents
In Content.Shared/_White/Wizard/SharedSpellsSystem.cs around lines 47 to 107,
ev.Handled is set prematurely (line ~52) and redundantly inside RechargePerson,
and there's an unnecessary return at the end; change flow to track whether any
charge action succeeded with a local bool (e.g., charged=false), remove the
early ev.Handled=true, set charged=true whenever a charge or PopupCharged call
occurs (including when RechargePerson succeeds), remove the redundant ev.Handled
assignment inside RechargePerson, delete the trailing return; at the end of
OnCharge set ev.Handled = charged and ensure appropriate popups still fire for
success/failure.
| protected virtual bool ChargeItem(EntityUid uid, ChargeMagicEvent ev) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| protected virtual bool Polymorph(PolymorphSpellEvent ev) | ||
| { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Возвращаемые значения по умолчанию вводят в заблуждение.
Методы ChargeItem и Polymorph возвращают true в базовой реализации, что подразумевает успех, хотя на самом деле ничего не делают. Это может привести к путанице:
- ChargeItem (строка 155): Возвращает
true, хотя ничего не зарядил. Должен возвращатьfalse, чтобы вызывающий код знал, что зарядка не произошла. - Polymorph (строка 160): То же самое - возвращает
trueбез выполнения полиморфа.
🔎 Предлагаемое исправление
protected virtual bool ChargeItem(EntityUid uid, ChargeMagicEvent ev)
{
- return true;
+ return false;
}
protected virtual bool Polymorph(PolymorphSpellEvent ev)
{
- return true;
+ return false;
}🤖 Prompt for AI Agents
In Content.Shared/_White/Wizard/SharedSpellsSystem.cs around lines 153 to 161,
the base implementations of ChargeItem and Polymorph misleadingly return true
despite doing nothing; change their default return value to false so callers
know the action did not succeed (leave methods virtual and signatures unchanged
so derived classes can override and return true when they actually perform the
work).
| [Serializable, NetSerializable] | ||
| public sealed class ChargeSpellRaysEffectEvent(NetEntity uid) : EntityEventArgs | ||
| { | ||
| public NetEntity Uid = uid; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Используйте readonly поле или свойство вместо публичного изменяемого поля.
Публичное изменяемое поле Uid на строке 176 нарушает соглашения C#. Поскольку значение инициализируется из конструктора и не должно изменяться, сделайте поле readonly или преобразуйте в свойство.
🔎 Предлагаемые варианты исправления
Вариант 1: Readonly поле
[Serializable, NetSerializable]
public sealed class ChargeSpellRaysEffectEvent(NetEntity uid) : EntityEventArgs
{
- public NetEntity Uid = uid;
+ public readonly NetEntity Uid = uid;
}Вариант 2: Свойство с init-only setter (предпочтительно)
[Serializable, NetSerializable]
-public sealed class ChargeSpellRaysEffectEvent(NetEntity uid) : EntityEventArgs
+public sealed class ChargeSpellRaysEffectEvent(NetEntity Uid) : EntityEventArgs
{
- public NetEntity Uid = uid;
}Или:
[Serializable, NetSerializable]
public sealed class ChargeSpellRaysEffectEvent(NetEntity uid) : EntityEventArgs
{
- public NetEntity Uid = uid;
+ public NetEntity Uid { get; init; } = uid;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [Serializable, NetSerializable] | |
| public sealed class ChargeSpellRaysEffectEvent(NetEntity uid) : EntityEventArgs | |
| { | |
| public NetEntity Uid = uid; | |
| } | |
| [Serializable, NetSerializable] | |
| public sealed class ChargeSpellRaysEffectEvent(NetEntity uid) : EntityEventArgs | |
| { | |
| public readonly NetEntity Uid = uid; | |
| } |
🤖 Prompt for AI Agents
In Content.Shared/_White/Wizard/SharedSpellsSystem.cs around lines 173 to 177,
the public mutable field `Uid` should not be publicly writable; change it to an
immutable member by either making the field `readonly` or (preferred) converting
it to a public property with an init-only setter (e.g., public NetEntity Uid {
get; init; }) and set it from the existing constructor; ensure the constructor
assigns the value to the new readonly field or init property and keep the
NetSerializable/Serializable attributes unchanged.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
7 participants
Описание PR
Очевидно, для работы этого PR необходим #872
В этом PR я перенес всякие системки с губа, что облегчит в дальнейшем перенос новых заклинаний. Также за одно захватил оттуда всю одежду, шейдеры. Тестом перенесенного всего стало заклинание "Заряд" и их система полиморфа. Ну и всякие фишечки, перечисленные в чл
Медиа
Список
Изменения
🆑