Conversation
github-actions
bot
added
Статус: Untriaged
Статус: Требует рассмотрения
size/M
Изменения: Аудио
and removed
Статус: Требует рассмотрения
Статус: Untriaged
labels
Чтобы включить этот плейлист надо прописать: "setlobbymusic EventMusicForLobby" |
📝 WalkthroughWalkthroughДобавлена серверная консольная команда для смены коллекции музыки лобби с ограничением «раз за раунд» и валидацией; клиентская аудиосистема подписывается на CVar изменения коллекции и прекращает лобби‑музыку при выходе из состояния лобби; добавлены локализации, прототип коллекции и файл атрибуций аудио. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Администратор
participant Console as Консоль
participant Command as SetLobbyMusicCommand
participant CVar as CCVars
participant Client as Клиент
participant Audio as AudioSystem
Admin->>Console: setlobbymusic <collection>
Console->>Command: Execute(argStr, args)
Command->>Command: Проверка: уже использовано в раунде?
alt Уже использовано
Command-->>Console: Вывести "уже использовано"
else Не использовано
Command->>Command: Валидировать аргументы и найти прототип
alt Прототип не найден / неверный аргумент
Command-->>Console: Ошибка "коллекция не найдена" / "неверные аргументы"
else Прототип найден
Command->>CVar: Установить CCVars.LobbyMusicCollection
CVar->>Client: Уведомление об изменении CVar
Client->>Audio: LobbyMusicCollectionChanged()
Audio->>Audio: Если не в LobbyState -> EndLobbyMusic() иначе (планируется) обновить музыку
Command-->>Console: Сообщение об успехе
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Content.Server/_OpenSpace/Administration/Commands/SetLobbyMusic.cs (1)
24-24: Статическое поле_alreadyUsedможет быть заменено на экземплярное.
EntitySystemв этом движке — синглтон, поэтомуstaticработает, но семантически правильнее использовать обычное поле экземпляра (private bool _alreadyUsed;). Это также упростит тестирование и соответствует паттерну других команд.Предлагаемое изменение
- private static bool _alreadyUsed; + private bool _alreadyUsed;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/_OpenSpace/Administration/Commands/SetLobbyMusic.cs` at line 24, Поле _alreadyUsed объявлено static; нужно заменить его на экземплярное поле в классе SetLobbyMusic (private bool _alreadyUsed;) и обновить все места, которые к нему обращаются, чтобы использовать экземпляр команды вместо статического доступа; проверьте методы/свойства в классе SetLobbyMusic и любые вызовы типа SetLobbyMusic._alreadyUsed и измените их на this._alreadyUsed или просто _alreadyUsed, удалите/обновите статические зависимости, и убедитесь, что семантика сброса/инициализации сохраняется при создании нового экземпляра (например, при необходимости инициализируйте в конструкторе или методе и очистите при завершении).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Client/Audio/ContentAudioSystem.LobbyMusic.cs`:
- Around line 129-141: The if in LobbyMusicCollectionChanged is missing braces
so only EndLobbyMusic() is conditional while return always runs; wrap the two
statements (EndLobbyMusic() and return;) in braces so the return executes only
when _state.CurrentState is not LobbyState, and ensure the subsequent commented
block (the StartLobbyMusic() code) remains reachable when the condition is
false.
In `@Resources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/attributions.yml`:
- Around line 1-34: В attributions.yml для каждой записи (например запись с
files: ["1.-Credits-Song-for-My-Death.ogg"], "2.-Space-Cruise-_Title_.ogg" и
т.д.) замените значение поля license на корректный SPDX-идентификатор (например
CC-BY-NC-SA-4.0 или CC-BY-NC-SA-3.0) вместо "CC BY-NC-SA"; примените правку ко
всем секциям (включая записи для "3.-Deus-Ex_-UNATCO.ogg", "4.-Beacon-Beach.ogg"
и т.д.) и дополнительно проверьте совместимость лицензий с политикой репозитория
(если NC запрещён, пометить или исключить трек).
In `@Resources/Prototypes/SoundCollections/lobby.yml`:
- Around line 20-28: В коллекции soundCollection с id EventMusicForLobby в
lobby.yml файлы не соответствуют записям в attributions.yml: атрибуции ссылаются
на 6.-Resurrections.ogg и 7.-Neon-Requiem.ogg, тогда как в files указан
6.-Neon-Requiem.ogg и отсутствует файл №7; исправьте это либо добавив в files
недостающий файл 6.-Resurrections.ogg (и при необходимости 7.-Neon-Requiem.ogg)
в список under id EventMusicForLobby, либо отредактируйте attributions.yml,
чтобы имена и номера треков точно совпадали с фактическими файлами в files.
---
Nitpick comments:
In `@Content.Server/_OpenSpace/Administration/Commands/SetLobbyMusic.cs`:
- Line 24: Поле _alreadyUsed объявлено static; нужно заменить его на
экземплярное поле в классе SetLobbyMusic (private bool _alreadyUsed;) и обновить
все места, которые к нему обращаются, чтобы использовать экземпляр команды
вместо статического доступа; проверьте методы/свойства в классе SetLobbyMusic и
любые вызовы типа SetLobbyMusic._alreadyUsed и измените их на this._alreadyUsed
или просто _alreadyUsed, удалите/обновите статические зависимости, и убедитесь,
что семантика сброса/инициализации сохраняется при создании нового экземпляра
(например, при необходимости инициализируйте в конструкторе или методе и
очистите при завершении).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f53a790-bd5d-4fad-97c0-862f46b77eb0
⛔ Files ignored due to path filters (6)
Resources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/1.-Credits-Song-for-My-Death.oggis excluded by!**/*.oggResources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/2.-Space-Cruise-_Title_.oggis excluded by!**/*.oggResources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/3.-Deus-Ex_-UNATCO.oggis excluded by!**/*.oggResources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/4.-Beacon-Beach.oggis excluded by!**/*.oggResources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/5.-All-Seeying-Eyes.oggis excluded by!**/*.oggResources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/6.-Neon-Requiem.oggis excluded by!**/*.ogg
📒 Files selected for processing (6)
Content.Client/Audio/ContentAudioSystem.LobbyMusic.csContent.Server/_OpenSpace/Administration/Commands/SetLobbyMusic.csResources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/attributions.ymlResources/Locale/en-US/_OpenSpace/commands/set-lobby-music-command.ftlResources/Locale/ru-RU/_OpenSpace/commands/set-lobby-music-command.ftlResources/Prototypes/SoundCollections/lobby.yml
Resources/Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/attributions.yml
Show resolved
Hide resolved
| - type: soundCollection | ||
| id: EventMusicForLobby | ||
| files: | ||
| - /Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/1.-Credits-Song-for-My-Death.ogg | ||
| - /Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/2.-Space-Cruise-_Title_.ogg | ||
| - /Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/3.-Deus-Ex_-UNATCO.ogg | ||
| - /Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/4.-Beacon-Beach.ogg | ||
| - /Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/5.-All-Seeying-Eyes.ogg | ||
| - /Audio/_OpenSpace/TEMP_FOR_EVENTS/ESphagnum/Lobby/6.-Neon-Requiem.ogg |
There was a problem hiding this comment.
Несоответствие между файлами коллекции и атрибуций.
В attributions.yml указаны файлы 6.-Resurrections.ogg и 7.-Neon-Requiem.ogg, но в этой коллекции файл №6 — это 6.-Neon-Requiem.ogg, а файл №7 отсутствует вовсе.
Необходимо синхронизировать:
- Либо добавить недостающий файл
6.-Resurrections.oggв коллекцию - Либо исправить атрибуции, чтобы они соответствовали фактическим файлам
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resources/Prototypes/SoundCollections/lobby.yml` around lines 20 - 28, В
коллекции soundCollection с id EventMusicForLobby в lobby.yml файлы не
соответствуют записям в attributions.yml: атрибуции ссылаются на
6.-Resurrections.ogg и 7.-Neon-Requiem.ogg, тогда как в files указан
6.-Neon-Requiem.ogg и отсутствует файл №7; исправьте это либо добавив в files
недостающий файл 6.-Resurrections.ogg (и при необходимости 7.-Neon-Requiem.ogg)
в список under id EventMusicForLobby, либо отредактируйте attributions.yml,
чтобы имена и номера треков точно совпадали с фактическими файлами в files.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd "attributions.yml" --exec cat {}
Length of output: 138806
@ESphagnum, вы правы — мой предыдущий комментарий был ошибочным. Проверив реальное содержимое attributions.yml, я вижу, что все 6 файлов в нём точно соответствуют файлам в коллекции EventMusicForLobby:
| lobby.yml | attributions.yml |
|---|---|
1.-Credits-Song-for-My-Death.ogg |
✅ |
2.-Space-Cruise-_Title_.ogg |
✅ |
3.-Deus-Ex_-UNATCO.ogg |
✅ |
4.-Beacon-Beach.ogg |
✅ |
5.-All-Seeying-Eyes.ogg |
✅ |
6.-Neon-Requiem.ogg |
✅ |
Несоответствия нет. Приношу извинения за ложноположительный комментарий.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Content.Client/Audio/ContentAudioSystem.LobbyMusic.cs (1)
130-140:⚠️ Potential issue | 🟠 MajorСмена коллекции в лобби сейчас фактически не применяется.
На Line 136-139 логика перезапуска закомментирована, поэтому при
LobbyStateметод становится no-op. С учетомContent.Server/_OpenSpace/Administration/Commands/SetLobbyMusic.cs(Line 64-68), где команда делает только_cfg.SetCVar(...), изменение коллекции не приводит к немедленному обновлению лобби-музыки на клиенте.Предлагаемое исправление
private void LobbyMusicCollectionChanged() { - if (_state.CurrentState is not LobbyState) { + if (_state.CurrentState is not LobbyState) + { EndLobbyMusic(); return; } - // if (_configManager.GetCVar(CCVars.LobbyMusicEnabled)) - // { - // StartLobbyMusic(); - // } + + if (_configManager.GetCVar(CCVars.LobbyMusicEnabled)) + { + EndLobbyMusic(); + StartLobbyMusic(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Client/Audio/ContentAudioSystem.LobbyMusic.cs` around lines 130 - 140, Смена коллекции сейчас игнорируется потому что внутри LobbyMusicCollectionChanged() закомментирован вызов перезапуска; нужно восстановить логику: в методе LobbyMusicCollectionChanged() проверить флаг _configManager.GetCVar(CCVars.LobbyMusicEnabled) и при true вызвать StartLobbyMusic(), иначе вызвать EndLobbyMusic(); оставьте предварительную проверку состояния (_state.CurrentState is not LobbyState) как есть, чтобы перезапуск происходил только в LobbyState — используйте существующие методы StartLobbyMusic и EndLobbyMusic для применения изменения сразу после SetCVar.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Server/GameTicking/Commands/ForceMapCommand.cs`:
- Line 20: Проблема: свойство Command в классе ForceMapCommand было
переименовано на "setgamemap", что ломает существующие вызовы и тесты; исправьте
это, вернув обратно идентификатор "forcemap" в ForceMapCommand.Command либо
синхронно обновите все зависимые места — тесты (класс ForceMapTest в
интеграционных тестах) и строку помощи (ресурс commands/force-map-command.ftl) —
чтобы использовать новое имя "setgamemap"; для минимального безразрывного
исправления просто изменить public override string Command => "forcemap" в
ForceMapCommand, либо если выбираете миграцию, обновите ForceMapTest и
соответствующий FTL help-файл в том же PR.
---
Duplicate comments:
In `@Content.Client/Audio/ContentAudioSystem.LobbyMusic.cs`:
- Around line 130-140: Смена коллекции сейчас игнорируется потому что внутри
LobbyMusicCollectionChanged() закомментирован вызов перезапуска; нужно
восстановить логику: в методе LobbyMusicCollectionChanged() проверить флаг
_configManager.GetCVar(CCVars.LobbyMusicEnabled) и при true вызвать
StartLobbyMusic(), иначе вызвать EndLobbyMusic(); оставьте предварительную
проверку состояния (_state.CurrentState is not LobbyState) как есть, чтобы
перезапуск происходил только в LobbyState — используйте существующие методы
StartLobbyMusic и EndLobbyMusic для применения изменения сразу после SetCVar.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb7e9b3d-a5b1-420a-9650-85cd3834fa4f
📒 Files selected for processing (2)
Content.Client/Audio/ContentAudioSystem.LobbyMusic.csContent.Server/GameTicking/Commands/ForceMapCommand.cs
| [Dependency] private readonly IPrototypeManager _prototypeManager = default!; | ||
|
|
||
| public override string Command => "forcemap"; | ||
| public override string Command => "setgamemap"; // OpenSpace-Edit |
There was a problem hiding this comment.
Переименование команды сделано неатомарно и ломает существующие вызовы.
На Line 20 идентификатор сменён на setgamemap, но связанные потребители не обновлены: интеграционный тест Content.IntegrationTests/Tests/Commands/ForceMapTest.cs (Line 62/67/72/77) и help-текст Resources/Locale/en-US/commands/force-map-command.ftl (Line 2) всё ещё используют forcemap. Это создаёт регрессию CLI и падение тестов. Либо верните forcemap, либо в этом же PR синхронно обновите тесты/локаль и явно оформите миграцию.
Вариант минимального исправления (без breaking change)
- public override string Command => "setgamemap"; // OpenSpace-Edit
+ public override string Command => "forcemap";📝 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 string Command => "setgamemap"; // OpenSpace-Edit | |
| public override string Command => "forcemap"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/GameTicking/Commands/ForceMapCommand.cs` at line 20, Проблема:
свойство Command в классе ForceMapCommand было переименовано на "setgamemap",
что ломает существующие вызовы и тесты; исправьте это, вернув обратно
идентификатор "forcemap" в ForceMapCommand.Command либо синхронно обновите все
зависимые места — тесты (класс ForceMapTest в интеграционных тестах) и строку
помощи (ресурс commands/force-map-command.ftl) — чтобы использовать новое имя
"setgamemap"; для минимального безразрывного исправления просто изменить public
override string Command => "forcemap" в ForceMapCommand, либо если выбираете
миграцию, обновите ForceMapTest и соответствующий FTL help-файл в том же PR.
There was a problem hiding this comment.
@ESphagnum сделай как кролик просит, обнови в тестах и локале
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Ещё пройдись по советам кролика. В целом это то что я нашёл. Это всё не критично, но не желательно. Код должен быть чистым, иначе могут быть конфликты, а нам это не нужно совсем.
Вот кстати линтер починить надо бы
5
Error: /home/runner/work/open-space/open-space/Content.Server/_OpenSpace/Administration/Commands/SetLobbyMusic.cs(21,19): error CS0108: 'SetLobbyMusicCommand.Description' hides inherited member 'EntitySystem.Description(EntityUid, MetaDataComponent?)'. Use the new keyword if hiding was intended. [/home/runner/work/open-space/open-space/Content.Server/Content.Server.csproj]
Error: /home/runner/work/open-space/open-space/Content.Server/_OpenSpace/Administration/Commands/SetLobbyMusic.cs(53,30): error CS0246: The type or namespace name 'SoundCollectionPrototype' could not be found (are you missing a using directive or an assembly reference?) [/home/runner/work/open-space/open-space/Content.Server/Content.Server.csproj]
| using Robust.Shared.Utility; | ||
|
|
||
|
|
||
| namespace Content.Client.Audio; |
| // if (_configManager.GetCVar(CCVars.LobbyMusicEnabled)) | ||
| // { | ||
| // StartLobbyMusic(); | ||
| // } |
There was a problem hiding this comment.
Если в комменте, то зачем оставлять? Или ты в будущем хочешь переработать метод?
| [Dependency] private readonly IPrototypeManager _prototypeManager = default!; | ||
|
|
||
| public override string Command => "forcemap"; | ||
| public override string Command => "setgamemap"; // OpenSpace-Edit |
There was a problem hiding this comment.
Лучше откати. В будущем возможны конфликты если оффы будут куда-то впихивать forcemap, в те же тесты например.
2 participants
Краткое описание
setlobbymusic
Почему мы должны добавить это?
Для ивентёров и сезонных событий
Медиа (Видео/Скриншоты)
Проверочный пункт
Changelog
Summary by CodeRabbit
Новые функции
Документация / Локализация