душ#4537
Open
liegend wants to merge 9 commits into
Open
Conversation
liegend
requested review from
KaiserMaus,
ThereDrD0 and
kanopus952
as code owners
Contributor
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughДобавлена новая игровая сущность «душ»: shared-компонент ChangesРеализация душа
Sequence DiagramsequenceDiagram
participant Player as Игрок
participant ShowerSys as ShowerSystem
participant Component as ShowerComponent
participant Appearance as AppearanceSystem
participant Puddle as PuddleSystem
Player->>ShowerSys: InteractHandEvent (клик)
ShowerSys->>Component: IsActive = !IsActive
ShowerSys->>Appearance: SetData(ShowerVisuals.Active, IsActive)
loop Update() каждые 5 секунд
ShowerSys->>Component: EntityQuery активных
ShowerSys->>Puddle: TrySpillAt(Water solution)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
added
🖌️ Спрайты 🖌️
📚 Изменения: Локализация 📚
🛠️ Есть C# 🛠️
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs`:
- Around line 36-54: The Update() method is spawning unlimited puddles every
UpdateInterval without any checks or limits. First, optimize the query by
caching the EntityQuery<ShowerComponent> as a field in Initialize() instead of
creating a new enumerator every Update call. Second, implement a mechanism to
prevent unlimited puddle accumulation: either track the current puddle reference
in ShowerComponent and reuse it instead of spawning new ones each interval,
check for existing puddles at the spawn coordinates using EntityLookupSystem
before spawning, or enforce a maximum puddle count per shower. Finally, verify
that the PuddleTemporary prototype has an auto-evaporation timeout that is
shorter than the UpdateInterval to ensure old puddles are cleaned up
automatically rather than accumulating indefinitely.
- Around line 10-13: The field `_currentPuddle` is currently defined at the
system level in ShowerSystem.cs, causing all shower entities on the map to share
the same puddle reference. This breaks logic when multiple showers exist. Move
`_currentPuddle` from being a system-level field to a per-component field in
ShowerComponent, so each individual shower entity can track its own puddle
independently. Update all references to `_currentPuddle` in ShowerSystem to
access it through the specific shower component being processed rather than from
the system instance.
- Around line 27-32: The puddle spawning logic in the shower interaction handler
executes unconditionally every time the shower is toggled, creating a new puddle
even when the shower is being turned off. Wrap the puddle spawning code (the if
block checking _currentPuddle) with an additional condition that verifies
ent.Comp.IsActive is true before attempting to spawn or recreate the puddle.
This ensures the puddle is only created when the shower is actually turned on,
not during shutdown interactions.
In `@Content.Shared/_Sunrise/Misc/ShowerComponent.cs`:
- Line 4: The namespaces in both files do not match their directory structures,
violating .NET naming conventions. In
Content.Shared/_Sunrise/Misc/ShowerComponent.cs at line 4, update the namespace
declaration from Content.Sunrise.Shared.Shower to Content.Shared._Sunrise.Misc.
In Content.Server/_Sunrise/Misc/ShowerSystem.cs at line 4, update the namespace
declaration from Content.Sunrise.Server.Shower to Content.Server._Sunrise.Misc.
This ensures that namespace declarations reflect the actual folder hierarchy for
consistency and better code navigation.
- Around line 9-10: The public field IsActive in ShowerComponent is missing XML
documentation. Add an XML comment block (using ///) above the IsActive field
declaration that describes what this field represents and its purpose. The
documentation should clearly explain to other developers what the IsActive field
controls and how it is used within the shower component.
In `@Resources/Locale/ru-RU/_prototypes/_sunrise/structures/interrogator.ftl`:
- Around line 7-8: The localization key `ent-Shower` does not match the
prototype ID `BaseShower` defined in the interrogator.yml file, which means the
localization will not be applied to the entity. Change the localization key from
`ent-Shower` to `ent-BaseShower` to align with the prototype ID, following the
convention that localization keys must use the pattern `ent-<PrototypeId>` where
PrototypeId is the exact ID of the prototype entity.
In `@Resources/Prototypes/_Sunrise/Entities/Effects/puddle.yml`:
- Around line 48-59: The PuddleWater entity prototype is missing the mandatory
English fallback fields `name` and `description` that are required by the YAML
prototype coding guidelines. Add both `name` and `description` fields to the
PuddleWater prototype with appropriate English values that describe this water
puddle entity. Even though the parent prototype PuddleTemporary may have these
fields, the PuddleWater entity must define its own specific values to serve as
fallbacks when localization is unavailable.
In
`@Resources/Textures/_Sunrise/Structures/Specific/Anomalies/clown.rsi/meta.json`:
- Around line 13-19: The delays field for the anomclown object in the meta.json
file has an extra level of nesting. Remove one layer of square brackets from the
delays array so that instead of having 3 levels of nesting (outer brackets
wrapping 4 inner arrays), you have 2 levels of nesting with 4 directional delay
arrays directly under delays. This should match the structure used in other
4-directional objects like conveyor.rsi.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 94be01ab-c46a-4d95-8e07-b6a02c9eeda7
⛔ Files ignored due to path filters (3)
Resources/Audio/_Sunrise/Machines/opened_shower.oggis excluded by!**/*.oggResources/Textures/_Sunrise/Structures/Specific/shower.rsi/icon.pngis excluded by!**/*.pngResources/Textures/_Sunrise/Structures/Specific/shower.rsi/water.pngis excluded by!**/*.png
📒 Files selected for processing (11)
Content.Server/_Sunrise/Misc/ShowerSystem.csContent.Shared/_Sunrise/Misc/ShowerComponent.csResources/Locale/ru-RU/_prototypes/_sunrise/structures/interrogator.ftlResources/Locale/ru-RU/_strings/_sunrise/construction/graphs/wallhorn.ftlResources/Prototypes/_Sunrise/Entities/Effects/puddle.ymlResources/Prototypes/_Sunrise/Recipes/Construction/Graphs/structures/girder.ymlResources/Prototypes/_Sunrise/Recipes/Construction/Graphs/structures/showers.ymlResources/Prototypes/_Sunrise/Recipes/Construction/wallhorn.ymlResources/Prototypes/_Sunrise/Structures/interrogator.ymlResources/Textures/_Sunrise/Structures/Specific/Anomalies/clown.rsi/meta.jsonResources/Textures/_Sunrise/Structures/Specific/shower.rsi/meta.json
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Server/_Sunrise/Misc/ShowerSystem.cs (1)
20-34: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winРазделите обработчик взаимодействия по схеме
OnEvent -> TryDo -> CanDo -> Do.Сейчас
OnInteractHandсодержит и проверки, и мутацию состояния, и сайд-эффекты. По правилу interaction-flow обработчик должен быть роутером:OnInteractHand→TryToggle→CanToggle(без сайд-эффектов) →DoToggle.As per coding guidelines, для
InteractHandEventтребуется строгая схемаOnEvent -> TryDo -> CanDo -> Doс минимальным обработчиком события и мутациями только вDo.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs` around lines 20 - 34, Refactor the OnInteractHand method in the ShowerComponent handler to follow the strict interaction-flow pattern: OnInteractHand should be a minimal router that calls TryToggle, which should check preconditions and call DoToggle if allowed. Create a CanToggle method that validates conditions without side-effects (checking if the puddle exists or is deleted), a TryToggle method that calls CanToggle and returns true/false to indicate whether the operation should proceed, and a DoToggle method that performs all state mutations (toggling IsActive, setting appearance data, spawning/managing the puddle, and marking dirty). Move all the logic from the current OnInteractHand (args.Handled check, state mutations, puddle management, and dirty call) into these appropriately named methods following the OnEvent -> TryDo -> CanDo -> Do pattern.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs`:
- Around line 20-34: Refactor the OnInteractHand method in the ShowerComponent
handler to follow the strict interaction-flow pattern: OnInteractHand should be
a minimal router that calls TryToggle, which should check preconditions and call
DoToggle if allowed. Create a CanToggle method that validates conditions without
side-effects (checking if the puddle exists or is deleted), a TryToggle method
that calls CanToggle and returns true/false to indicate whether the operation
should proceed, and a DoToggle method that performs all state mutations
(toggling IsActive, setting appearance data, spawning/managing the puddle, and
marking dirty). Move all the logic from the current OnInteractHand (args.Handled
check, state mutations, puddle management, and dirty call) into these
appropriately named methods following the OnEvent -> TryDo -> CanDo -> Do
pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7c38147b-3177-4ecd-af6f-3eae164126ff
📒 Files selected for processing (4)
Content.Server/_Sunrise/Misc/ShowerSystem.csContent.Shared/_Sunrise/Misc/ShowerComponent.csResources/Locale/ru-RU/_prototypes/_sunrise/structures/interrogator.ftlResources/Prototypes/_Sunrise/Entities/Effects/puddle.yml
💤 Files with no reviewable changes (1)
- Resources/Prototypes/_Sunrise/Entities/Effects/puddle.yml
Contributor
Author
|
@coderabbitai что не так? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get review comments on PR `#4537`
gh pr view 4537 --repo space-sunrise/sunrise-station --json reviews,comments
echo "---REVIEW_COMMENTS---"
gh api repos/space-sunrise/sunrise-station/pulls/4537/comments --jq '.[] | {user: .user.login, body: .body, path: .path, line: .line}'
echo "---ISSUE_COMMENTS---"
gh api repos/space-sunrise/sunrise-station/issues/4537/comments --jq '.[] | {user: .user.login, body: .body}'Length of output: 95541
❌ Критические ошибки (блокируют мёрж)1. Неверный синтаксис YAML-компонентов 2. Отсутствуют английские FTL-файлы
🔴 Серьёзные баги в логике3. 4. Неограниченный спавн луж в 5. Лужа спавнится при ВЫКЛЮЧЕНИИ душа ( 🟠 Другие замечания6. Namespace не соответствует директориям
7. Лишний уровень вложенности в 8. Отсутствуют ✅ Уже исправлено
🐰 (´･ω･`)？ |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Content.Server/_Sunrise/Misc/ShowerSystem.cs (2)
26-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winНужно разделить обработчик на Try/Can/Do и добавить явную валидацию перед переключением.
Сейчас
OnInteractHandсразу меняетIsActive. Для server-side интеракта это обходит обязательный потокOnEvent -> TryDo -> CanDo -> Doи усложняет контроль авторизации/дистанции/доступности действия в одном месте.Предлагаемый минимальный рефакторинг
private void OnInteractHand(Entity<ShowerComponent> ent, ref InteractHandEvent args) { if (args.Handled) return; - args.Handled = true; - - ent.Comp.IsActive = !ent.Comp.IsActive; - _appearance.SetData(ent.Owner, ShowerVisuals.Active, ent.Comp.IsActive); - - Dirty(ent); + if (!TryToggleShower(ent, args)) + return; + + args.Handled = true; } + +private bool TryToggleShower(Entity<ShowerComponent> ent, InteractHandEvent args) +{ + if (!CanToggleShower(ent, args)) + return false; + + DoToggleShower(ent); + return true; +} + +private bool CanToggleShower(Entity<ShowerComponent> ent, InteractHandEvent args) +{ + // TODO: централизованные проверки доступа/дистанции/блокировок по стандарту проекта + return true; +} + +private void DoToggleShower(Entity<ShowerComponent> ent) +{ + ent.Comp.IsActive = !ent.Comp.IsActive; + _appearance.SetData(ent.Owner, ShowerVisuals.Active, ent.Comp.IsActive); + Dirty(ent); +}As per coding guidelines, для
*System.csобязателен interaction flowOnEvent -> TryDo -> CanDo -> Do, а client→server события должны валидироваться до выполнения действия.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs` around lines 26 - 37, The OnInteractHand method in ShowerSystem.cs currently bypasses the required server-side interaction flow pattern. Refactor it to follow the mandatory OnEvent -> TryDo -> CanDo -> Do pattern: create a separate TryDo method that handles the validation chain, a CanDo method that validates prerequisites (authorization, distance, availability), and a Do method that actually toggles the IsActive state and updates appearance via SetData and Dirty. Have OnInteractHand call the TryDo method instead of directly modifying IsActive, ensuring all authorization and distance validation happens before the action is executed.Source: Coding guidelines
43-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winАккумулятор теряет интервалы при больших
frameTimeи занижает частоту пролива.Сейчас вычитание интервала происходит один раз. Если
frameTime/лаг даст накопление >UpdateInterval, часть тиков пролива теряется. Лучше обрабатывать в цикле.Точечное исправление
- _accumulator += frameTime; - if (_accumulator < UpdateInterval) - return; - - _accumulator -= UpdateInterval; - - var query = EntityQueryEnumerator<ShowerComponent>(); - while (query.MoveNext(out var uid, out var shower)) - { - if (!shower.IsActive) - continue; - - SpillWater((uid, shower)); - } + _accumulator += frameTime; + while (_accumulator >= UpdateInterval) + { + _accumulator -= UpdateInterval; + + var query = EntityQueryEnumerator<ShowerComponent>(); + while (query.MoveNext(out var uid, out var shower)) + { + if (!shower.IsActive) + continue; + + SpillWater((uid, shower)); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs` around lines 43 - 48, The accumulator logic in ShowerSystem.cs only processes a single update per frame even when large frameTime values cause the accumulator to exceed UpdateInterval by multiple intervals, resulting in missed shower ticks during lag or frame drops. Replace the if statement that checks if accumulator is less than UpdateInterval and subtracts UpdateInterval once, with a while loop that continues to subtract UpdateInterval and process updates as long as the accumulator is greater than or equal to UpdateInterval. This ensures all updates are processed when the accumulated time spans multiple update intervals.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs`:
- Around line 14-20: The member declaration order in the ShowerSystem class
violates the required layout standard. The runtime state field _accumulator is
currently declared before the const and static readonly members, but it must be
moved to appear after the constants/static section. Reorder the members so that
all const declarations (UpdateInterval, ShowerFillDuration, ShowerFillLimit,
ShowerReagent) and static readonly declarations (ShowerSpillAmount) come first,
followed by the _accumulator runtime state field.
---
Outside diff comments:
In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs`:
- Around line 26-37: The OnInteractHand method in ShowerSystem.cs currently
bypasses the required server-side interaction flow pattern. Refactor it to
follow the mandatory OnEvent -> TryDo -> CanDo -> Do pattern: create a separate
TryDo method that handles the validation chain, a CanDo method that validates
prerequisites (authorization, distance, availability), and a Do method that
actually toggles the IsActive state and updates appearance via SetData and
Dirty. Have OnInteractHand call the TryDo method instead of directly modifying
IsActive, ensuring all authorization and distance validation happens before the
action is executed.
- Around line 43-48: The accumulator logic in ShowerSystem.cs only processes a
single update per frame even when large frameTime values cause the accumulator
to exceed UpdateInterval by multiple intervals, resulting in missed shower ticks
during lag or frame drops. Replace the if statement that checks if accumulator
is less than UpdateInterval and subtracts UpdateInterval once, with a while loop
that continues to subtract UpdateInterval and process updates as long as the
accumulator is greater than or equal to UpdateInterval. This ensures all updates
are processed when the accumulated time spans multiple update intervals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d9eeeb95-31a9-46f8-8fda-4a21addaafd5
📒 Files selected for processing (1)
Content.Server/_Sunrise/Misc/ShowerSystem.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Shared/_Sunrise/Misc/ShowerComponent.cs (1)
4-4:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winКритическая проблема: неймспейсы не соответствуют физической структуре директорий, что вызывает сбой сборки.
Все три объявления неймспейсов/импортов нарушают конвенцию .NET о соответствии неймспейса физическому пути файла. Это приводит к ошибкам компиляции CS0234 во всех CI pipeline.
Content.Shared/_Sunrise/Misc/ShowerComponent.cs#L4-L4: Изменитеnamespace Content._Sunrise.Shared.Shower;наnamespace Content.Shared._Sunrise.Misc;Content.Server/_Sunrise/Misc/ShowerSystem.cs#L1-L1: Изменитеusing Content.Shared._Sunrise.Shower;наusing Content.Shared._Sunrise.Misc;Content.Server/_Sunrise/Misc/ShowerSystem.cs#L8-L8: Изменитеnamespace Content.Server._Sunrise.Shower;наnamespace Content.Server._Sunrise.Misc;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Shared/_Sunrise/Misc/ShowerComponent.cs` at line 4, The namespace declarations do not match the physical directory structure, causing compilation errors. In Content.Shared/_Sunrise/Misc/ShowerComponent.cs at line 4, change the namespace declaration from `namespace Content._Sunrise.Shared.Shower;` to `namespace Content.Shared._Sunrise.Misc;` to match the file path. In Content.Server/_Sunrise/Misc/ShowerSystem.cs at line 1, update the using statement from `using Content.Shared._Sunrise.Shower;` to `using Content.Shared._Sunrise.Misc;` to reference the correct namespace. In Content.Server/_Sunrise/Misc/ShowerSystem.cs at line 8, change the namespace declaration from `namespace Content.Server._Sunrise.Shower;` to `namespace Content.Server._Sunrise.Misc;` to align with the file's physical location.
♻️ Duplicate comments (2)
Content.Server/_Sunrise/Misc/ShowerSystem.cs (1)
8-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winНеймспейс системы не соответствует структуре директорий.
Аналогично компоненту, неймспейс должен быть
Content.Server._Sunrise.Misc, чтобы соответствовать физическому путиContent.Server/_Sunrise/Misc/.🔧 Предлагаемое исправление
-namespace Content.Server._Sunrise.Shower; +namespace Content.Server._Sunrise.Misc;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs` at line 8, The namespace declaration in ShowerSystem.cs does not match the directory structure. Change the namespace from `Content.Server._Sunrise.Shower` to `Content.Server._Sunrise.Misc` to align with the actual file path location at Content.Server/_Sunrise/Misc/.Content.Shared/_Sunrise/Misc/ShowerComponent.cs (1)
12-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winОтсутствует XML-документация для новых публичных полей.
Согласно coding guidelines, каждое публичное поле компонента должно иметь XML-комментарий с описанием назначения. Это упрощает понимание кода другими разработчиками.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Shared/_Sunrise/Misc/ShowerComponent.cs` around lines 12 - 16, The public fields Accumulator and CurrentPuddle in the ShowerComponent class are missing XML documentation comments required by coding guidelines. Add XML documentation comments (using ///) above each field to describe their purpose: Accumulator should document what it accumulates and CurrentPuddle should document what entity reference it maintains. These comments will improve code comprehension for other developers reviewing the component.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs`:
- Line 64: The TrySpillAt method returns a UID for the created or updated puddle
through its out parameter, but the current code ignores this result using out _.
Additionally, the CurrentPuddle field was added to ShowerComponent but is never
set, making it dead code. Either remove the unused CurrentPuddle field from
ShowerComponent if puddle tracking is not needed, or modify the TrySpillAt call
to assign the returned UID to the CurrentPuddle field by changing out _ to out
ent.Comp.CurrentPuddle if tracking is required. Choose the approach that aligns
with the system's design intent for puddle management.
- Line 14: The dependency injection field `_solution` of type
`SharedSolutionContainerSystem` is declared but never used anywhere in the
ShowerSystem class. The code creates a Solution instance directly through the
constructor instead of using this injected system. Remove the unused `_solution`
field dependency declaration to eliminate unnecessary DI overhead and dead code.
In `@Content.Shared/_Sunrise/Misc/ShowerComponent.cs`:
- Around line 12-16: The fields Accumulator and CurrentPuddle in the
ShowerComponent class are incorrectly marked with [DataField]. The Accumulator
field is a runtime counter that changes during system operation and should not
be serialized from YAML configuration, so remove its [DataField] attribute. The
CurrentPuddle field is a temporary cached reference that is not used anywhere in
ShowerSystem.cs, making it dead code, so remove this field entirely along with
its [DataField] attribute. Only keep fields with [DataField] that represent
actual configuration coming from YAML prototypes.
---
Outside diff comments:
In `@Content.Shared/_Sunrise/Misc/ShowerComponent.cs`:
- Line 4: The namespace declarations do not match the physical directory
structure, causing compilation errors. In
Content.Shared/_Sunrise/Misc/ShowerComponent.cs at line 4, change the namespace
declaration from `namespace Content._Sunrise.Shared.Shower;` to `namespace
Content.Shared._Sunrise.Misc;` to match the file path. In
Content.Server/_Sunrise/Misc/ShowerSystem.cs at line 1, update the using
statement from `using Content.Shared._Sunrise.Shower;` to `using
Content.Shared._Sunrise.Misc;` to reference the correct namespace. In
Content.Server/_Sunrise/Misc/ShowerSystem.cs at line 8, change the namespace
declaration from `namespace Content.Server._Sunrise.Shower;` to `namespace
Content.Server._Sunrise.Misc;` to align with the file's physical location.
---
Duplicate comments:
In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs`:
- Line 8: The namespace declaration in ShowerSystem.cs does not match the
directory structure. Change the namespace from `Content.Server._Sunrise.Shower`
to `Content.Server._Sunrise.Misc` to align with the actual file path location at
Content.Server/_Sunrise/Misc/.
In `@Content.Shared/_Sunrise/Misc/ShowerComponent.cs`:
- Around line 12-16: The public fields Accumulator and CurrentPuddle in the
ShowerComponent class are missing XML documentation comments required by coding
guidelines. Add XML documentation comments (using ///) above each field to
describe their purpose: Accumulator should document what it accumulates and
CurrentPuddle should document what entity reference it maintains. These comments
will improve code comprehension for other developers reviewing the component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8cee12a7-96c7-466d-9466-216293be86eb
📒 Files selected for processing (2)
Content.Server/_Sunrise/Misc/ShowerSystem.csContent.Shared/_Sunrise/Misc/ShowerComponent.cs
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Content.Server/_Sunrise/Misc/ShowerSystem.cs (2)
26-37: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winРазделите обработчик по цепочке
OnEvent -> TryDo -> CanDo -> Do.Сейчас на Line 26-37 логика переключения душа выполняется прямо в обработчике события. Это ломает обязательный interaction-flow и затрудняет добавление валидаций (доступ, дистанция, состояние) без разрастания
OnInteractHand.
As per coding guidelines: enforce the interaction rule from.agents/rules/ss14-interaction-flow.md:OnEvent -> TryDo -> CanDo -> Do.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs` around lines 26 - 37, The OnInteractHand method contains all the shower toggle logic directly in the event handler, violating the required interaction-flow pattern of OnEvent -> TryDo -> CanDo -> Do. Refactor by: keeping OnInteractHand as the event handler that only sets args.Handled and delegates to a new TryDo method, creating a CanDo method to validate preconditions (access, distance, state), and creating a Do method that contains the actual toggle logic currently in OnInteractHand (toggling ent.Comp.IsActive, calling _appearance.SetData with ShowerVisuals.Active, resetting ent.Comp.Accumulator to 0f, and calling Dirty). This separation will allow validation logic to be added without expanding OnInteractHand and will enforce the mandatory interaction-flow pattern.Source: Coding guidelines
50-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winПотеря накопленного времени при большом
frameTimeвUpdate().На Line 50-55 за тик выполняется только один пролив, даже если накопилось несколько интервалов (
frameTime/лаг >UpdateInterval). В итоге душ «недоливает» воду и состояние расходится с ожидаемым темпом.🛠️ Предлагаемое исправление
- shower.Accumulator += frameTime; - if (shower.Accumulator < UpdateInterval) - continue; - - shower.Accumulator -= UpdateInterval; - SpillWater((uid, shower)); - Dirty(uid, shower); + shower.Accumulator += frameTime; + var spilled = false; + while (shower.Accumulator >= UpdateInterval) + { + shower.Accumulator -= UpdateInterval; + SpillWater((uid, shower)); + spilled = true; + } + + if (spilled) + Dirty(uid, shower);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs` around lines 50 - 55, The shower system in the ShowerSystem.cs Update method only processes one water spillage per frame using an if condition, which causes accumulated time intervals to be lost during large frameTime values (lag spikes). Replace the if statement at line 50-52 with a while loop that continues executing SpillWater for as many complete UpdateInterval periods as have accumulated in shower.Accumulator, ensuring that all pending water spillages are processed in a single frame rather than discarding excess accumulated time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Content.Server/_Sunrise/Misc/ShowerSystem.cs`:
- Around line 26-37: The OnInteractHand method contains all the shower toggle
logic directly in the event handler, violating the required interaction-flow
pattern of OnEvent -> TryDo -> CanDo -> Do. Refactor by: keeping OnInteractHand
as the event handler that only sets args.Handled and delegates to a new TryDo
method, creating a CanDo method to validate preconditions (access, distance,
state), and creating a Do method that contains the actual toggle logic currently
in OnInteractHand (toggling ent.Comp.IsActive, calling _appearance.SetData with
ShowerVisuals.Active, resetting ent.Comp.Accumulator to 0f, and calling Dirty).
This separation will allow validation logic to be added without expanding
OnInteractHand and will enforce the mandatory interaction-flow pattern.
- Around line 50-55: The shower system in the ShowerSystem.cs Update method only
processes one water spillage per frame using an if condition, which causes
accumulated time intervals to be lost during large frameTime values (lag
spikes). Replace the if statement at line 50-52 with a while loop that continues
executing SpillWater for as many complete UpdateInterval periods as have
accumulated in shower.Accumulator, ensuring that all pending water spillages are
processed in a single frame rather than discarding excess accumulated time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 826788b3-58f8-4c60-bc7d-57e9149b5c58
📒 Files selected for processing (2)
Content.Server/_Sunrise/Misc/ShowerSystem.csContent.Shared/_Sunrise/Misc/ShowerComponent.cs
Contributor
Author
|
чо ты мне мозги ебешь блять заяц, все работает |
Contributor
|
оно сжигает слаймолюдов? Если нет, то должно. |
Contributor
Author
не |
Contributor
Я считаю что Душ должен сжигать слаймолдов как огнетушитель. Это логичная реакция. |
Contributor
Author
|
@coderabbitai что не так |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Остальное — стиль/неймспейсы, некритично для работы. (´・_・`) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Краткое описание | Short description
добавлен душ
Ссылка на багрепорт/Предложение | Related Issue/Bug Report
https://discord.com/channels/1499823476360613898/1502944114289934447
Медиа (Видео/Скриншоты) | Media (Video/Screenshots)
2026-06-15.15-42-41.mp4
🆑 4_ydo
Summary by CodeRabbit
Выпуск
BaseShower.PuddleWater, строительные рецепт и граф сборки для душа, включая новые локализационные строки, и обновлены метаданные спрайтаshower.anomclown(структура задержек).