Conversation
|
Warning Review limit reachedNext included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
✨ 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 |
|
Потом заревьювю это |
|
Трахнул бы |
| private void RaiseSunriseMarkingsUpdated(EntityUid target) | ||
| { | ||
| var ev = new SunriseMarkingsUpdatedEvent(); | ||
| RaiseLocalEvent(target, ref ev); | ||
| } |
There was a problem hiding this comment.
Метод на 2 строчки используемый 1 раз. Не вижу в нем смысла, используй этот код напрямую
| private bool TryGetMoodState( | ||
| Entity<MoodVisualsComponent> ent, | ||
| AppearanceComponent appearance, | ||
| out string? state) |
| if (HasComp<PentagramComponent>(ent) && HasComp<WingToggleComponent>(ent)) | ||
| return false; | ||
|
|
||
| return !AppearanceSystem.TryGetData<MoodThreshold>(ent, MoodVisuals.CurrentMoodThreshold, out var moodThreshold, appearance) ? ent.Comp.VisibleWithoutMood : ent.Comp.MoodStates.TryGetValue(moodThreshold, out state); |
There was a problem hiding this comment.
Лучше инвертировать условие и поменять местами аргументы + сделать многострочным.
Лишнее логическое усложнение в виде ! - хуже для читаемости
| return; | ||
|
|
||
| if (ShouldHideMoodVisuals(ent)) | ||
| Entity<SpriteComponent> spriteEnt = (ent, sprite); |
There was a problem hiding this comment.
Поменять тип на простой var, этого достаточно и смотрится проще
There was a problem hiding this comment.
Заменю вот таким образом и уберу лишнее AsNullable
var spriteEnt = new Entity<SpriteComponent?>(ent, sprite);
There was a problem hiding this comment.
Заменю вот таким образом и уберу лишнее AsNullable
var spriteEnt = new Entity<SpriteComponent?>(ent, sprite);
Достаточно просто var spriteEnt = (ent, sprite);
| /// <summary> | ||
| /// Должны ли настроенные markings отображаться до появления данных о настроении. | ||
| /// </summary> | ||
| [DataField] | ||
| public SpriteSpecifier? Sprite; | ||
| public bool VisibleWithoutMood; | ||
|
|
There was a problem hiding this comment.
Нейминг и комментарий расходятся.
Нейминг означает видны ли ВООБЩЕ эффекты без настроения. В комментарии же написано "до появления настроения", что не равнозначно отсутствию проверок на наличие настроения
Нужно уточнить и поменять
| [RegisterComponent] | ||
| public sealed partial class MoodVisualsComponent : Component | ||
| { |
There was a problem hiding this comment.
Тут кстати компонент не дублируется на клиент, если будет добавлен на с клиента.
Нужно исправить или переместить чисто на клиент
| (collection.TryResolveType<ISharedSponsorsManager>(out _) && | ||
| speciesPrototype.SponsorOnly && | ||
| !sponsorPrototypes.Contains(Species.Id))) // Sunrise-Edit |
There was a problem hiding this comment.
Комментарий должен стоять на уровне добавленной строчки, а не в конце.
Переместить наверх
There was a problem hiding this comment.
Как правильно?
Первый вариант:
// Sunrise-Edit
(collection.TryResolveType<ISharedSponsorsManager>(out _) &&
speciesPrototype.SponsorOnly &&
!sponsorPrototypes.Contains(Species.Id)))
Второй вариант:
// Sunrise-Edit-Start
(collection.TryResolveType<ISharedSponsorsManager>(out _) &&
speciesPrototype.SponsorOnly &&
!sponsorPrototypes.Contains(Species.Id)))
// Sunrise-Edit-End
There was a problem hiding this comment.
Второй можно, но я имел ввиду такое
(collection.TryResolveType<ISharedSponsorsManager>(out _) && // Sunrise-Edit
speciesPrototype.SponsorOnly &&
!sponsorPrototypes.Contains(Species.Id)))
Готовим изменения к ревьюПривет! Здесь видно, что осталось сделать перед проверкой человеком. Пролистай страницу ПР вниз до блока проверок: там видны тесты и их результаты. Галочки в этом списке обновляются автоматически.
Warning GitHub не разрешит слить ПР, пока есть конфликты. Обнови свою ветку из целевой, открой отмеченные как конфликтующие файлы в IDE, выбери правильные изменения, создай коммит и отправь его.
Показать обязательные проверки
Как найти список ошибок тестов
Когда все пункты выполнены, бот сам переведёт ПР из черновика в готовое состояние. Обновление иногда занимает несколько минут. |
makura-auto-draft
Bot
added
the
🍄 автодрафт: нужны исправления
3 participants
Краткое описание
Добавлен выбор внешности для милир между нимбом и различными видами рогов (Роги от аркан)
Важно -> специально сделано так, чтобы нельзя было выбрать рога + нимб.
Сделал небольшой cleanup в
MoodVisualizerSystem.cs. Полный cleanup в этом ПРе делать не буду.ДОПОЛНИТЕЛЬНО:
Также исправлено сохранение персонажей спонсорских рас в сборках без подключённой системы спонсоров. Невозможно было сохранить персонажа на спонсорской расе на тестовом сервере. Исправил проверком есть ли спонсоркий менеджер.
Ссылка на багрепорт/Предложение
https://discord.com/channels/1499823476360613898/1512507715925053553
Медиа
Скриншоты выбора нимба и рогов
Changelog
🆑 Orvex07
Summary by CodeRabbit
Новые возможности
Исправления
Локализация