Weekly Mode(Еженедельный режим)#660
Open
MusicalGOODTV wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
size/Medium
Contributor
|
Warning Review limit reached
More reviews will be available in 44 minutes and 50 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (78)
WalkthroughPR вводит Imperial Weekly Mode. Добавлены shared-модели и сетевые контракты для weekly cargo, lathe, research, role aliases, snapshots и runtime state. Реализованы серверные map/storage API и admin-команды для управления кампаниями, snapshot, ролями, технологиями, рецептами и cargo. Игровые системы обновлены для weekly access, forced roles, job display names, weekly cargo orders, weekly lathe recipes и weekly research unlocks. Клиентский UI обновлён для lobby, cargo, lathe и research. Добавлены интеграционные и unit-тесты, а также документация для администрирования. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
t (1)
1-62:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winФайл
tпохоже не предназначен для коммита в репозиторий.Этот файл содержит вывод
git diff --statи имеет нехарактерное имяt, что указывает на временный/служебный файл, случайно включённый в коммит. Подобные файлы засоряют репозиторий и не несут функциональной нагрузки.Рекомендуется:
- Удалить файл из коммита
- Добавить
tв.gitignore, если это часто используемый scratch-файл🤖 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 `@t` around lines 1 - 62, The file `t` is a temporary scratch file containing git diff output that was accidentally committed to the repository and should not be part of the codebase. Remove this file from the commit by deleting it from the working directory, then add the filename `t` to the `.gitignore` file to prevent similar temporary files from being accidentally included in future commits.Content.Client/Cargo/UI/CargoShuttleMenu.xaml.cs (1)
38-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winНесоответствие источника имени продукта между UI.
В
CargoShuttleMenuдля non-weekly продуктов используетсяproduct.NameизEntityPrototype(строка 53), тогда как вCargoConsoleMenu.PopulateOrdersиспользуетсяproductProto.NameизCargoProductPrototype(строка 322).Это может привести к отображению разных имён для одного и того же товара в разных интерфейсах.
🐛 Предлагаемое исправление
else { if (!protoManager.Resolve<CargoProductPrototype>(order.Product, out var productProto)) continue; var product = protoManager.Index<EntityPrototype>(productProto.Product); - productName = product.Name; + productName = productProto.Name; icon = sprites.Frame0(product); }🤖 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.Client/Cargo/UI/CargoShuttleMenu.xaml.cs` around lines 38 - 75, The product name source is inconsistent between CargoShuttleMenu and CargoConsoleMenu. In the non-weekly products branch where you resolve the CargoProductPrototype, replace the line that sets productName to use product.Name with productProto.Name instead. This ensures both UI components display the same product name from the CargoProductPrototype source, maintaining consistency across the interface.Content.Server/GameTicking/GameTicker.RoundFlow.cs (1)
573-594:⚠️ Potential issue | 🔴 CriticalИсправьте потенциальный InvalidOperationException при получении антага роли.
Существует несогласованность между
MindIsAntagonist()иMindGetAllRoleInfo():
MindIsAntagonist()проверяет флагcomp.Antagдля всех ролей в контейнере без валидацииMindGetAllRoleInfo()добавляет в результат только валидные роли (когда прототип успешно загружается)Если роль имеет
comp.Antag == trueно содержит невалидный прототип,MindIsAntagonist()вернётtrue, ноMindGetAllRoleInfo()не включит эту роль в результат. Тогда вызовroles.First(role => role.Antagonist)на строке 593 выброситInvalidOperationException.Предлагаемое исправление:
Role = antag - ? roles.First(role => role.Antagonist).Name + ? roles.FirstOrDefault(role => role.Antagonist)?.Name ?? roleDisplay : roleDisplay,🤖 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/GameTicking/GameTicker.RoundFlow.cs` around lines 573 - 594, The code has a potential InvalidOperationException on line 593 where it calls roles.First(role => role.Antagonist) when antag is true. The issue is that MindIsAntagonist() may return true even when the antagonist role has an invalid prototype, but MindGetAllRoleInfo() filters out invalid roles, leaving no antagonist role in the results for First() to find. Fix this by using FirstOrDefault() instead of First() when querying for the antagonist role, and provide a fallback display name using the null-coalescing operator (similar to how firstRole.Name is handled) to ensure a valid role name is always displayed even if the antagonist role is not found.Content.Shared/Lathe/LatheComponent.cs (1)
82-91:⚠️ Potential issue | 🟠 MajorДобавьте атрибут
[AutoNetworkedField]для полейCurrentRecipeиCurrentRecipeIsWeekly.Поля помечены только
[ViewVariables]без[DataField]и[AutoNetworkedField], хотя компонент имеет[NetworkedComponent, AutoGenerateComponentState]. Это создаёт проблему синхронизации, так как:
LatheSystem.cs(строки 260-261, 268-269) устанавливает значения этих полей при запуске производстваWeeklyModeSystem.cs(строки 5461-5462) читает эти значения для проверки еженедельного рецепта- Без
[AutoNetworkedField]клиенты не получат обновления состоянияСледуйте паттерну других полей компонента (например,
UseCardIdиDefaultProductionAmount), которые используют оба атрибута:[DataField, AutoNetworkedField].🤖 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/Lathe/LatheComponent.cs` around lines 82 - 91, The fields CurrentRecipe and CurrentRecipeIsWeekly in LatheComponent lack the required [AutoNetworkedField] attribute needed for network synchronization. Since the component is marked with [NetworkedComponent, AutoGenerateComponentState] and these fields are modified by LatheSystem and read by WeeklyModeSystem, add both [DataField] and [AutoNetworkedField] attributes to CurrentRecipe and CurrentRecipeIsWeekly to follow the pattern established by other networked fields in this component like UseCardId and DefaultProductionAmount, ensuring state changes are properly synchronized to all clients.
🤖 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.Client/Cargo/UI/CargoConsoleMenu.xaml.cs`:
- Around line 200-218: The method TryGetProductDisplayData is inefficiently
rebuilding the entire product list on each call by invoking
BuildProductDisplayData(). Instead of rebuilding the list every time, cache the
result of BuildProductDisplayData() as a class-level field and reuse it. Modify
TryGetProductDisplayData to iterate over the cached list instead of calling
BuildProductDisplayData(). Implement an InvalidateProductCache() method that
clears this cache, and ensure this method is called whenever ProductCatalogue or
WeeklyProductCatalogue is modified to keep the cache in sync with changes to the
underlying data.
In `@Content.Client/LateJoin/LateJoinGui.cs`:
- Around line 239-240: The JobButton's localized role name becomes stale when
JobNameOverrides is updated while the LateJoinGui window is open. Currently, the
JobsAvailableUpdated method only calls RefreshLabel() to update slot counts but
does not update the JobLocalisedName property of the JobButton objects. Modify
the JobsAvailableUpdated method to also update each JobButton's JobLocalisedName
property with the current job display name obtained from
_gameTicker.GetJobDisplayName(prototype) for each job, ensuring role name
changes are reflected immediately without requiring a full UI rebuild through
RebuildUI().
In `@Content.Client/Lathe/UI/LatheMenu.xaml.cs`:
- Around line 328-372: The material discount parameter in the
GenerateTooltipText method (where SharedLatheSystem.AdjustMaterial is called) is
hardcoded to true, while regular recipes use prototype.ApplyMaterialDiscount for
flexible configuration. Determine whether WeeklyLatheRecipeData should include
an ApplyMaterialDiscount field to make discount behavior configurable like
regular recipes. If yes, add this boolean field to WeeklyLatheRecipeData and
replace the hardcoded true parameter with the value from the recipe object in
both GenerateTooltipText and CanProduceWeeklyRecipe methods.
- Around line 482-501: The SetQueueInfo method uses FirstOrDefault to search for
a WeeklyRecipes item by recipeId, but when no match is found, it returns a
default record struct with RecipeId = string.Empty instead of a null value. This
causes empty UI controls to be displayed instead of explicitly handling the
missing recipe case. After the FirstOrDefault call that assigns the recipe
variable (around line 492), add a check to verify that recipe.RecipeId !=
string.Empty before proceeding to add the child control and set the name label.
If the recipe is not found, handle it appropriately (such as logging an error or
returning early). Apply the same defensive check pattern to the
GetQueuedRecipeDisplayControl method at line 559 where FirstOrDefault is used in
a similar way.
In `@Content.Server/Cargo/Systems/CargoSystem.Orders.cs`:
- Around line 838-856: The GetAvailableWeeklyProducts method contains a
hardcoded check for the "market" group that limits flexibility for weekly cargo
product filtering. Add a clarifying comment above the line that checks if
(!markets.Contains("market")) to explain why weekly products are restricted to
only the "market" group and not other market-group variations. This will
document the intended design decision and help future maintainers understand the
constraint.
In `@Content.Server/WeeklyMode/Storage/WeeklyModeStore.cs`:
- Around line 452-468: The GetDirectorySize method is inefficiently loading
entire files into memory via ReadAllBytes just to obtain their size, which
wastes memory and stresses the garbage collector especially for large snapshot
files like station.yml. Replace the inefficient ReadAllBytes(path).Length call
with a more efficient approach using the Open method to get the file stream
length directly. Use Open with FileMode.Open, FileAccess.Read, and
FileShare.Read parameters, wrap it in a using statement to ensure proper
resource cleanup, and access the Stream.Length property instead. This will
retrieve the file size without loading the entire file contents into memory.
In `@Content.Shared/Cargo/CargoOrderData.cs`:
- Line 30: The WeeklyProduct field is a value type that will contain default
values when IsWeeklyProduct is false, which can lead to accidental access to
incorrect data. Change the WeeklyProduct field declaration to use a nullable
type (WeeklyCargoProductData?) to explicitly represent when the data is not
available, and update the constructor and any code that sets or accesses
WeeklyProduct to properly handle the nullable type, ensuring null is assigned
when IsWeeklyProduct is false and a proper instance is assigned otherwise.
In `@Content.Shared/Research/Components/TechnologyDatabaseComponent.cs`:
- Line 40: The `WeeklyAllowedTechnologies` field uses
`List<ProtoId<TechnologyPrototype>>` while `WeeklyUnlockedTechnologies` uses
`List<string>`, creating a type inconsistency that can lead to errors when
working with these fields. Unify both fields to use the same type throughout the
TechnologyDatabaseComponent class by changing one of them to match the other,
then update all code that consumes these fields to work with the unified type
consistently.
In `@Content.Shared/WeeklyMode/WeeklyModeModels.cs`:
- Line 165: The CompatibleWithBuild property in the WeeklyModeModels.cs file has
an incorrect default value of true, which doesn't accurately represent that
version compatibility has not yet been verified. Change the default value
initialization of the CompatibleWithBuild property from true to false to
explicitly indicate that compatibility checking is pending and has not been
performed.
In `@Content.Tests/Server/WeeklyMode/WeeklyModeStoreTest.cs`:
- Around line 892-932: The parameter mapName in the SerializedMapData method is
checked for null using an if statement but lacks a nullable annotation, which
should be added for clarity and compliance with C# nullable reference types.
Change the mapName parameter declaration from string to string? to explicitly
indicate that it can accept null values, making the code more consistent with
the null check that follows.
In `@Docs/administration/weekly-mode.md`:
- Around line 113-118: The documentation for the wm.rollback confirmation token
flow in the section describing the command syntax (wm.rollback <setId>
<snapshotId> <token>) is missing information about the token's lifetime. Add a
note immediately after the code block that explicitly states the confirmation
token is valid for 2 minutes and that after this period the rollback request
must be initiated again. This aligns the documentation with the actual
server-side token expiration behavior.
---
Outside diff comments:
In `@Content.Client/Cargo/UI/CargoShuttleMenu.xaml.cs`:
- Around line 38-75: The product name source is inconsistent between
CargoShuttleMenu and CargoConsoleMenu. In the non-weekly products branch where
you resolve the CargoProductPrototype, replace the line that sets productName to
use product.Name with productProto.Name instead. This ensures both UI components
display the same product name from the CargoProductPrototype source, maintaining
consistency across the interface.
In `@Content.Server/GameTicking/GameTicker.RoundFlow.cs`:
- Around line 573-594: The code has a potential InvalidOperationException on
line 593 where it calls roles.First(role => role.Antagonist) when antag is true.
The issue is that MindIsAntagonist() may return true even when the antagonist
role has an invalid prototype, but MindGetAllRoleInfo() filters out invalid
roles, leaving no antagonist role in the results for First() to find. Fix this
by using FirstOrDefault() instead of First() when querying for the antagonist
role, and provide a fallback display name using the null-coalescing operator
(similar to how firstRole.Name is handled) to ensure a valid role name is always
displayed even if the antagonist role is not found.
In `@Content.Shared/Lathe/LatheComponent.cs`:
- Around line 82-91: The fields CurrentRecipe and CurrentRecipeIsWeekly in
LatheComponent lack the required [AutoNetworkedField] attribute needed for
network synchronization. Since the component is marked with [NetworkedComponent,
AutoGenerateComponentState] and these fields are modified by LatheSystem and
read by WeeklyModeSystem, add both [DataField] and [AutoNetworkedField]
attributes to CurrentRecipe and CurrentRecipeIsWeekly to follow the pattern
established by other networked fields in this component like UseCardId and
DefaultProductionAmount, ensuring state changes are properly synchronized to all
clients.
In `@t`:
- Around line 1-62: The file `t` is a temporary scratch file containing git diff
output that was accidentally committed to the repository and should not be part
of the codebase. Remove this file from the commit by deleting it from the
working directory, then add the filename `t` to the `.gitignore` file to prevent
similar temporary files from being accidentally included in future commits.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 34ba3750-28e0-4b36-a496-c411b367bbcb
📒 Files selected for processing (64)
Content.Client/Cargo/BUI/CargoOrderConsoleBoundUserInterface.csContent.Client/Cargo/UI/CargoConsoleMenu.xaml.csContent.Client/Cargo/UI/CargoProductRow.xaml.csContent.Client/Cargo/UI/CargoShuttleMenu.xaml.csContent.Client/GameTicking/Managers/ClientGameTicker.csContent.Client/LateJoin/LateJoinGui.csContent.Client/Lathe/UI/LatheBoundUserInterface.csContent.Client/Lathe/UI/LatheMenu.xaml.csContent.Client/Lathe/UI/RecipeControl.xaml.csContent.Client/Lobby/UI/CharacterPickerButton.xaml.csContent.Client/Lobby/UI/CharacterSetupGui.xaml.csContent.Client/Lobby/UI/HumanoidProfileEditor.Roles.csContent.Client/Lobby/UI/HumanoidProfileEditor.xaml.csContent.Client/Research/UI/MiniTechnologyCardControl.xaml.csContent.Client/Research/UI/ResearchConsoleMenu.xaml.csContent.Client/Research/UI/TechnologyCardControl.xaml.csContent.IntegrationTests/Tests/WeeklyMode/WeeklyModeLiveEditingTest.csContent.IntegrationTests/Tests/WeeklyMode/WeeklyModePersistenceTest.csContent.Server/Administration/Commands/WeeklyMode/WeeklyModeCommands.csContent.Server/Cargo/Systems/CargoSystem.Orders.csContent.Server/Cargo/Systems/CargoSystem.csContent.Server/CrewManifest/CrewManifestSystem.csContent.Server/GameTicking/Commands/JoinGameCommand.csContent.Server/GameTicking/GameTicker.GamePreset.csContent.Server/GameTicking/GameTicker.Lobby.csContent.Server/GameTicking/GameTicker.RoundFlow.csContent.Server/GameTicking/GameTicker.Spawning.csContent.Server/GameTicking/GameTicker.csContent.Server/Lathe/LatheSystem.csContent.Server/Maps/GameMapManager.csContent.Server/Maps/IGameMapManager.csContent.Server/Research/Systems/ResearchSystem.Console.csContent.Server/Research/Systems/ResearchSystem.Technology.csContent.Server/Roles/Jobs/JobSystem.csContent.Server/Station/Systems/StationJobsSystem.Roundstart.csContent.Server/Station/Systems/StationJobsSystem.csContent.Server/Station/Systems/StationSpawningSystem.csContent.Server/StationRecords/Systems/StationRecordsSystem.csContent.Server/WeeklyMode/Storage/WeeklyModeStore.csContent.Server/WeeklyMode/Systems/WeeklyModeSystem.csContent.Shared/Actions/ActionGrantSystem.csContent.Shared/CCVar/CCVars.WeeklyMode.csContent.Shared/Cargo/BUI/CargoConsoleInterfaceState.csContent.Shared/Cargo/CargoOrderData.csContent.Shared/Cargo/WeeklyCargoProductData.csContent.Shared/GameTicking/SharedGameTicker.csContent.Shared/Lathe/LatheComponent.csContent.Shared/Lathe/LatheMessages.csContent.Shared/Lathe/WeeklyLatheRecipeData.csContent.Shared/Light/EntitySystems/SharedLightReplacerSystem.csContent.Shared/Light/EntitySystems/SharedPoweredLightSystem.csContent.Shared/Maps/GameMapPrototype.csContent.Shared/Research/Components/TechnologyDatabaseComponent.csContent.Shared/Research/Systems/SharedResearchSystem.csContent.Shared/Research/WeeklyTechnologyData.csContent.Shared/Storage/EntitySystems/BinSystem.csContent.Shared/VendingMachines/SharedVendingMachineSystem.csContent.Shared/VendingMachines/VendingMachineComponent.csContent.Shared/WeeklyMode/WeeklyModeModels.csContent.Tests/Server/WeeklyMode/WeeklyModeStoreTest.csDocs/administration/weekly-mode.mdResources/Prototypes/Imperial/Other/ImperialMedkits/ImperialMedkits.ymlt
Comment on lines
+200
to
+218
| // Imperial Weekly Mode | ||
| public bool TryGetProductDisplayData(string productId, out string name, out string description, out int cost) | ||
| { | ||
| foreach (var product in BuildProductDisplayData()) | ||
| { | ||
| if (!string.Equals(product.ProductId, productId, StringComparison.Ordinal)) | ||
| continue; | ||
|
|
||
| name = product.Name; | ||
| description = product.Description; | ||
| cost = product.Cost; | ||
| return true; | ||
| } | ||
|
|
||
| name = string.Empty; | ||
| description = string.Empty; | ||
| cost = 0; | ||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Повторное построение списка продуктов при каждом вызове.
TryGetProductDisplayData вызывает BuildProductDisplayData(), который полностью перестраивает список продуктов при каждом обращении. При частых вызовах (например, при обновлении UI) это может создавать лишнюю нагрузку.
Рекомендуется кешировать результат BuildProductDisplayData() и инвалидировать кеш при изменении каталогов.
♻️ Предлагаемое решение
+ private List<ProductDisplayData>? _cachedProductDisplayData;
+
+ private List<ProductDisplayData> GetCachedProductDisplayData()
+ {
+ return _cachedProductDisplayData ??= BuildProductDisplayData();
+ }
+
+ public void InvalidateProductCache()
+ {
+ _cachedProductDisplayData = null;
+ }
+
public bool TryGetProductDisplayData(string productId, out string name, out string description, out int cost)
{
- foreach (var product in BuildProductDisplayData())
+ foreach (var product in GetCachedProductDisplayData())
{Вызывайте InvalidateProductCache() при изменении ProductCatalogue или WeeklyProductCatalogue.
🤖 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.Client/Cargo/UI/CargoConsoleMenu.xaml.cs` around lines 200 - 218, The
method TryGetProductDisplayData is inefficiently rebuilding the entire product
list on each call by invoking BuildProductDisplayData(). Instead of rebuilding
the list every time, cache the result of BuildProductDisplayData() as a
class-level field and reuse it. Modify TryGetProductDisplayData to iterate over
the cached list instead of calling BuildProductDisplayData(). Implement an
InvalidateProductCache() method that clears this cache, and ensure this method
is called whenever ProductCatalogue or WeeklyProductCatalogue is modified to
keep the cache in sync with changes to the underlying data.
| // Imperial Weekly Mode | ||
| [AutoNetworkedField] | ||
| [DataField] | ||
| public List<ProtoId<TechnologyPrototype>> WeeklyAllowedTechnologies = new(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Поиск использования WeeklyUnlockedTechnologies и WeeklyAllowedTechnologies для проверки паттерна использования
rg -nP '\b(WeeklyUnlockedTechnologies|WeeklyAllowedTechnologies)\b' --type=cs -C3Repository: imperial-space/SS14-public
Length of output: 17242
Несоответствие типов подтверждено: требуется унификация API для Weekly-технологий.
WeeklyAllowedTechnologies использует List<ProtoId<TechnologyPrototype>>, а WeeklyUnlockedTechnologies использует List<string>.
Это несоответствие существует и создаёт риск ошибок: первый список содержит строго типизированные идентификаторы прототипов (используемые с PrototypeManager.TryIndex()), второй — raw строковые ID. При работе с этими полями легко спутать типы, особенно при добавлении новых функций или рефакторинге.
Рекомендуется привести оба поля к единому типу: либо оба List<ProtoId<TechnologyPrototype>>, либо оба List<string> в зависимости от общего подхода в компоненте.
🤖 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/Research/Components/TechnologyDatabaseComponent.cs` at line
40, The `WeeklyAllowedTechnologies` field uses
`List<ProtoId<TechnologyPrototype>>` while `WeeklyUnlockedTechnologies` uses
`List<string>`, creating a type inconsistency that can lead to errors when
working with these fields. Unify both fields to use the same type throughout the
TechnologyDatabaseComponent class by changing one of them to match the other,
then update all code that consumes these fields to work with the unified type
consistently.
The-Fiafi
requested changes
Jun 20, 2026
The-Fiafi
left a comment
The-Fiafi
left a comment
Contributor
There was a problem hiding this comment.
Добавьте комментарии на каждую измененную строчку
Если вы удаляете строчку, то комментируйте ее
Если вы изменяете несколько строчек, то оберните изменения в комментарии // Imperial Space Start/End
Не изменяйте код официальных разработчиков, вынесите изменения по partial классам в папку _SystemsSupplementation
Все новые файлы должны храниться в подпапках Imperial
Ваш код очень сильно вмешивается в большинство систем, это будет очень сложно апстримить, рассмотрите возможность вынесение логики в статичные классы или создания параллельных систем
| @@ -0,0 +1,61 @@ | |||
| .../BUI/CargoOrderConsoleBoundUserInterface.cs | 37 [32m+[m[31m-[m | |||
Contributor
Author
There was a problem hiding this comment.
Это случайно добавленный временный файл, к Weekly Mode он не относится. Удалю.
| @@ -0,0 +1,96 @@ | |||
| [35mResources/Locale/ru-RU/Imperial/Medical/ImperialMedkits.ftl[m[36m:[m[32m39[m[36m:[ment-[1;31mMedkitOxygenImperialFilled[m = { ent-MedkitOxygenImperial } | |||
Contributor
Author
There was a problem hiding this comment.
Это тоже случайный служебный файл с повреждённым именем. Удалю.
|
|
||
| _orderMenu.Description.SetMessage(description); | ||
| _product = row.Product; | ||
| _productId = row.ProductId; |
|
|
||
| _menu.PopulateProducts(); | ||
| _menu.PopulateCategories(); | ||
| _menu.PopulateProducts(); |
| _menu.PopulateProducts(); | ||
| _menu.PopulateOrders(orders); | ||
| _menu.PopulateAccountActions(); | ||
| RefreshSelectedProduct(); |
| @@ -672,9 +806,6 @@ private bool FulfillOrder(CargoOrderData order, ProtoId<CargoAccountPrototype> a | |||
| _slots.TryInsert(item, label.LabelSlot, printed, null); | |||
| } | |||
| } | |||
Contributor
There was a problem hiding this comment.
Комментарии, все удаленные строчки должны быть закомментированы
| namespace Content.Server.Cargo.Systems; | ||
|
|
||
| // Imperial Weekly Mode | ||
| public readonly record struct WeeklyCargoCatalogChangedEvent; |
Contributor
There was a problem hiding this comment.
Вынести в отдельную папку
| var entry = new CrewManifestEntry(record.Name, record.JobTitle, record.JobIcon, record.JobPrototype); | ||
|
|
||
| _prototypeManager.TryIndex(record.JobPrototype, out JobPrototype? job); | ||
| // Imperial Weekly Mode |
Contributor
There was a problem hiding this comment.
Комментируйте удаленные строчки
| { | ||
| shell.WriteLine($"{jobPrototype.LocalizedName} has no available slots."); | ||
| // Imperial Weekly Mode | ||
| var weeklyMode = _entManager.System<WeeklyModeSystem>(); |
Contributor
There was a problem hiding this comment.
Комментируйте удаленные строчки
| Role = antag | ||
| ? roles.First(role => role.Antagonist).Name | ||
| : roles.FirstOrDefault().Name ?? Loc.GetString("game-ticker-unknown-role"), | ||
| : roleDisplay, |
Contributor
There was a problem hiding this comment.
Комментируйте удаленные строчки
Contributor
Author
There was a problem hiding this comment.
Окей. Я завтра всё исправлю и отвечу.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Content.Server/Cargo/Systems/CargoSystem.Orders.cs (2)
217-217:⚠️ Potential issue | 🟠 Major | ⚡ Quick winПредотвратите переполнение при расчёте стоимости заказа.
costсчитается вint(productCost * order.OrderQuantity) без проверок. При больших weekly-стоимостях это переполняется и может привести к некорректному списанию/зачислению средств.💡 Вариант исправления
- var cost = productCost * order.OrderQuantity; + if (productCost < 0) + { + ConsolePopup(args.Actor, Loc.GetString("cargo-console-invalid-product")); + PlayDenySound(uid, component); + return; + } + + var totalCost = (long) productCost * order.OrderQuantity; + if (totalCost > int.MaxValue) + { + ConsolePopup(args.Actor, Loc.GetString("cargo-console-invalid-product")); + PlayDenySound(uid, component); + return; + } + + var cost = (int) totalCost;🤖 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/Cargo/Systems/CargoSystem.Orders.cs` at line 217, The cost calculation in CargoSystem.Orders.cs at the line `var cost = productCost * order.OrderQuantity` performs integer multiplication without overflow protection, which can cause incorrect financial calculations when multiplying large values. Change the calculation to use `long` instead of `int` by either casting the operands to `long` (e.g., `(long)productCost * order.OrderQuantity`) or by using a checked expression block to detect overflow conditions. Ensure the `cost` variable is declared or cast appropriately to accommodate the larger numeric range and prevent silent overflow errors.
266-268:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winЛоги не отражают weekly-продукт в заказах.
Для weekly-заказов в лог пишется
product:{order.Product}/product:{data.Product}, но это поле не заполняется в weekly-конструкторе. В результате теряется идентификатор товара в аудит-логах.💡 Вариант исправления
- _adminLogger.Add(LogType.Action, + var approvedProductLog = order.IsWeeklyProduct + ? order.WeeklyProduct?.ProductId ?? "<weekly-unknown>" + : order.Product.Id; + _adminLogger.Add(LogType.Action, LogImpact.Low, - $"{ToPrettyString(player):user} approved order [orderId:{order.OrderId}, quantity:{order.OrderQuantity}, product:{order.Product}, requester:{order.Requester}, reason:{order.Reason}] on account {order.Account} with balance at {accountBalance}"); + $"{ToPrettyString(player):user} approved order [orderId:{order.OrderId}, quantity:{order.OrderQuantity}, product:{approvedProductLog}, requester:{order.Requester}, reason:{order.Reason}] on account {order.Account} with balance at {accountBalance}");- _adminLogger.Add(LogType.Action, + var addedProductLog = data.IsWeeklyProduct + ? data.WeeklyProduct?.ProductId ?? "<weekly-unknown>" + : data.Product.Id; + _adminLogger.Add(LogType.Action, LogImpact.Low, - $"{ToPrettyString(player):user} added order [orderId:{data.OrderId}, quantity:{data.OrderQuantity}, product:{data.Product}, requester:{data.Requester}, reason:{data.Reason}]"); + $"{ToPrettyString(player):user} added order [orderId:{data.OrderId}, quantity:{data.OrderQuantity}, product:{addedProductLog}, requester:{data.Requester}, reason:{data.Reason}]");Also applies to: 429-431
🤖 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/Cargo/Systems/CargoSystem.Orders.cs` around lines 266 - 268, The audit logs for order approvals reference order.Product in their log messages, but for weekly orders this field is not being populated during order creation, resulting in missing product identifiers in the logs. Ensure that when weekly orders are constructed, the Product field is properly initialized with the actual product identifier so that the logging statements in the approval methods (around the locations where LogType.Action is called with orderId, quantity, product, requester, and reason fields) will have valid product information to include in the audit trail.Content.Tests/Server/Imperial/WeeklyMode/WeeklyModeStoreTest.cs (1)
1046-1049:⚠️ Potential issue | 🟠 MajorДобавьте поддержку переименования директорий в тестовый
IWritableDirProvider.
File.Moveне может переименовывать директории. ВWeeklyModeStore(строки 595, 599, 609) вызываетсяRenameдля snapshot-путей (tempPath, finalPath, backupPath), которые являются директориями. Текущая тестовая реализация упадёт при попытке переместить директорию, что блокирует тесты rollback/restore сценариев.Распределите логику
RenameмеждуDirectory.Moveдля директорий иFile.Moveдля файлов:Предлагаемое решение
public void Rename(ResPath oldPath, ResPath newPath) { - File.Move(GetFullPath(oldPath), GetFullPath(newPath)); + var oldFullPath = GetFullPath(oldPath); + var newFullPath = GetFullPath(newPath); + + if (Directory.Exists(oldFullPath)) + Directory.Move(oldFullPath, newFullPath); + else + File.Move(oldFullPath, newFullPath); }🤖 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.Tests/Server/Imperial/WeeklyMode/WeeklyModeStoreTest.cs` around lines 1046 - 1049, The Rename method in the test IWritableDirProvider implementation currently uses only File.Move, which cannot handle directory renaming. Since WeeklyModeStore calls Rename for directory paths (tempPath, finalPath, backupPath), the test will fail when attempting to move directories. Modify the Rename method to check whether the path being moved is a directory or a file using Directory.Exists and File.Exists respectively, then use Directory.Move for directories and File.Move for files to properly handle both cases.
🤖 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.Client/LateJoin/LateJoinGui.cs`:
- Around line 316-323: The issue is in the matchingJobButton.Disabled assignment
where the |= operator causes the button to become permanently disabled once
Amount reaches 0, since |= can only set the value to true but never back to
false. Replace the line containing matchingJobButton.Disabled |=
matchingJobButton.Amount == 0; with a direct assignment that recalculates the
Disabled state based on the current Amount value and any requirement
constraints, ensuring the button can be re-enabled when slots become available
again. The Disabled state should be determined by evaluating whether Amount == 0
along with any other logical requirements, not by accumulating the previous
state.
In
`@Content.Server/Imperial/WeeklyMode/_SystemsSupplementation/LatheSystem.WeeklyMode.cs`:
- Around line 24-30: The issue is that materials are deducted immediately when
queuing a weekly recipe (in GetAdjustedWeeklyAmount and TryChangeMaterialAmount
calls), but only the RecipeId is stored in the LatheRecipeBatch. If an admin
modifies or deletes the recipe before the batch completes via
WeeklyRecipesChangedEvent, the refund logic in LatheSystem will fail because the
original recipe data no longer exists, potentially blocking the lathe. Fix this
by capturing and storing a snapshot of the deducted materials and recipe cost
information in the LatheRecipeBatch constructor at the time of queuing, so that
even if the recipe is later modified or deleted, the system can properly refund
or reconcile the original deducted amounts. Alternatively, add a handler for
WeeklyRecipesChangedEvent that atomically processes refunds and reconciliation
for all affected weekly job batches currently in the queue before the recipe
changes take effect.
In
`@Content.Server/Imperial/WeeklyMode/_SystemsSupplementation/ResearchSystem.WeeklyMode.cs`:
- Around line 24-36: The code updates WeeklyTechnologies and
WeeklyUnlockedTechnologies but does not recalculate the recipe unlocks that
result from these technologies. Store the result of previousWeeklyTechnologies
at the beginning of the method and after updating the weekly technologies lists,
use the previousWeeklyTechnologies to determine which recipes were previously
unlocked. Remove any recipe IDs that were only provided by removed or modified
weekly technologies and are no longer covered by the updated normal or weekly
unlocks. Then re-add the recipe IDs from the preserved
WeeklyUnlockedTechnologies to ensure the recipe unlock list stays consistent
with the current weekly technology state. Apply the same logic to the related
code around lines 57-73 that handles similar technology updates.
In
`@Content.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Roles.cs`:
- Around line 340-361: The Execute method (and similar async void command
handlers) contains an await call to _weekly.ResolveForcedRoleTargetAsync() which
can throw unhandled exceptions such as HttpRequestException and
TaskCanceledException from underlying HTTP operations, causing crashes in the
synchronous context. Wrap the await _weekly.ResolveForcedRoleTargetAsync() call
and any other await operations in a try-catch block, and in the catch block use
shell.WriteError() to convert these exceptions into managed error messages that
are properly reported to the console instead of becoming unhandled exceptions.
In `@Content.Shared/Imperial/WeeklyMode/WeeklyModeModels.cs`:
- Line 168: The CompatibleWithBuild property in WeeklyModeModels.cs is missing
its safe default value and currently defaults to false, causing new
WeeklySnapshotMetadata instances to be marked as incompatible without any actual
validation. Either restore the default value by adding = true to the
CompatibleWithBuild property declaration, or locate all creation paths of
WeeklySnapshotMetadata objects and explicitly set CompatibleWithBuild to true
during initialization to ensure new snapshots are properly marked as compatible
by default.
---
Outside diff comments:
In `@Content.Server/Cargo/Systems/CargoSystem.Orders.cs`:
- Line 217: The cost calculation in CargoSystem.Orders.cs at the line `var cost
= productCost * order.OrderQuantity` performs integer multiplication without
overflow protection, which can cause incorrect financial calculations when
multiplying large values. Change the calculation to use `long` instead of `int`
by either casting the operands to `long` (e.g., `(long)productCost *
order.OrderQuantity`) or by using a checked expression block to detect overflow
conditions. Ensure the `cost` variable is declared or cast appropriately to
accommodate the larger numeric range and prevent silent overflow errors.
- Around line 266-268: The audit logs for order approvals reference
order.Product in their log messages, but for weekly orders this field is not
being populated during order creation, resulting in missing product identifiers
in the logs. Ensure that when weekly orders are constructed, the Product field
is properly initialized with the actual product identifier so that the logging
statements in the approval methods (around the locations where LogType.Action is
called with orderId, quantity, product, requester, and reason fields) will have
valid product information to include in the audit trail.
In `@Content.Tests/Server/Imperial/WeeklyMode/WeeklyModeStoreTest.cs`:
- Around line 1046-1049: The Rename method in the test IWritableDirProvider
implementation currently uses only File.Move, which cannot handle directory
renaming. Since WeeklyModeStore calls Rename for directory paths (tempPath,
finalPath, backupPath), the test will fail when attempting to move directories.
Modify the Rename method to check whether the path being moved is a directory or
a file using Directory.Exists and File.Exists respectively, then use
Directory.Move for directories and File.Move for files to properly handle both
cases.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: eadd86c0-0105-4021-988a-ba122158c3e2
📒 Files selected for processing (45)
Content.Client/Cargo/BUI/CargoOrderConsoleBoundUserInterface.csContent.Client/Cargo/UI/CargoConsoleMenu.xaml.csContent.Client/Cargo/UI/CargoShuttleMenu.xaml.csContent.Client/Imperial/WeeklyMode/Cargo/CargoConsoleMenu.WeeklyMode.csContent.Client/Imperial/WeeklyMode/Cargo/CargoProductRow.WeeklyMode.csContent.Client/Imperial/WeeklyMode/Lathe/LatheMenu.WeeklyMode.csContent.Client/Imperial/WeeklyMode/Lathe/RecipeControl.WeeklyMode.csContent.Client/Imperial/WeeklyMode/Research/MiniTechnologyCardControl.WeeklyMode.csContent.Client/Imperial/WeeklyMode/Research/ResearchConsoleMenu.WeeklyMode.csContent.Client/Imperial/WeeklyMode/Research/TechnologyCardControl.WeeklyMode.csContent.Client/LateJoin/LateJoinGui.csContent.Client/Lathe/UI/LatheMenu.xaml.csContent.Client/Lathe/UI/RecipeControl.xaml.csContent.Client/Research/UI/MiniTechnologyCardControl.xaml.csContent.Client/Research/UI/ResearchConsoleMenu.xaml.csContent.IntegrationTests/Tests/Imperial/WeeklyMode/WeeklyModeLiveEditingTest.csContent.IntegrationTests/Tests/Imperial/WeeklyMode/WeeklyModePersistenceTest.csContent.Server/Cargo/Systems/CargoSystem.Orders.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Access.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Campaign.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Cargo.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Config.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Gamerules.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Persistence.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Recipes.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Research.csContent.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Roles.csContent.Server/Imperial/WeeklyMode/Storage/WeeklyModeStore.csContent.Server/Imperial/WeeklyMode/Systems/WeeklyModeSystem.csContent.Server/Imperial/WeeklyMode/_SystemsSupplementation/CargoSystem.WeeklyMode.csContent.Server/Imperial/WeeklyMode/_SystemsSupplementation/LatheSystem.WeeklyMode.csContent.Server/Imperial/WeeklyMode/_SystemsSupplementation/ResearchSystem.WeeklyMode.csContent.Server/Lathe/LatheSystem.csContent.Server/Research/Systems/ResearchSystem.Console.csContent.Server/Research/Systems/ResearchSystem.Technology.csContent.Shared/Cargo/CargoOrderData.csContent.Shared/Imperial/WeeklyMode/CCVar/CCVars.WeeklyMode.csContent.Shared/Imperial/WeeklyMode/Cargo/WeeklyCargoProductData.csContent.Shared/Imperial/WeeklyMode/Lathe/WeeklyLatheRecipeData.csContent.Shared/Imperial/WeeklyMode/Research/WeeklyTechnologyData.csContent.Shared/Imperial/WeeklyMode/WeeklyModeModels.csContent.Shared/Imperial/WeeklyMode/_SystemsSupplementation/SharedResearchSystem.WeeklyMode.csContent.Shared/Research/Systems/SharedResearchSystem.csContent.Tests/Server/Imperial/WeeklyMode/WeeklyModeStoreTest.csDocs/administration/weekly-mode.md
💤 Files with no reviewable changes (5)
- Content.Shared/Imperial/WeeklyMode/CCVar/CCVars.WeeklyMode.cs
- Content.Shared/Imperial/WeeklyMode/Research/WeeklyTechnologyData.cs
- Content.Server/Research/Systems/ResearchSystem.Console.cs
- Content.Server/Research/Systems/ResearchSystem.Technology.cs
- Content.Shared/Imperial/WeeklyMode/Cargo/WeeklyCargoProductData.cs
Comment on lines
+316
to
323
| // Imperial Weekly Mode Start | ||
| var jobName = _gameTicker.GetJobDisplayName(existingJobEntry.Key, _prototypeManager); | ||
| if (matchingJobButton.Amount != updatedJobValue || | ||
| !string.Equals(matchingJobButton.JobLocalisedName, jobName, StringComparison.Ordinal)) | ||
| { | ||
| matchingJobButton.RefreshLabel(updatedJobValue); | ||
| matchingJobButton.RefreshLabel(updatedJobValue, jobName); | ||
| matchingJobButton.Disabled |= matchingJobButton.Amount == 0; | ||
| } |
Contributor
There was a problem hiding this comment.
Кнопка профессии может остаться навсегда отключённой после временного отсутствия слотов.
В Line 322 оператор |= делает Disabled необратимым для этого пути: после Amount == 0 кнопка не включится, когда слот снова станет доступен. Нужен пересчёт Disabled из источников истины (ограничения требований + текущий Amount), а не накопление старого состояния.
💡 Вариант правки
- matchingJobButton.Disabled |= matchingJobButton.Amount == 0;
+ matchingJobButton.Disabled = matchingJobButton.RequirementsLocked || matchingJobButton.Amount == 0; sealed class JobButton : ContainerButton
{
+ public bool RequirementsLocked { get; set; }
...
}
if (!_jobRequirements.IsAllowed(...))
{
jobButton.Disabled = true;
+ jobButton.RequirementsLocked = true;
...
}🤖 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.Client/LateJoin/LateJoinGui.cs` around lines 316 - 323, The issue is
in the matchingJobButton.Disabled assignment where the |= operator causes the
button to become permanently disabled once Amount reaches 0, since |= can only
set the value to true but never back to false. Replace the line containing
matchingJobButton.Disabled |= matchingJobButton.Amount == 0; with a direct
assignment that recalculates the Disabled state based on the current Amount
value and any requirement constraints, ensuring the button can be re-enabled
when slots become available again. The Disabled state should be determined by
evaluating whether Amount == 0 along with any other logical requirements, not by
accumulating the previous state.
Comment on lines
+24
to
+30
| foreach (var (mat, amount) in GetAdjustedWeeklyAmount(component, recipe)) | ||
| _materialStorage.TryChangeMaterialAmount(uid, mat, -amount * quantity); | ||
|
|
||
| if (component.Queue.Last is { } node && node.ValueRef.IsWeekly && node.ValueRef.Recipe == recipe.RecipeId) | ||
| node.ValueRef.ItemsRequested += quantity; | ||
| else | ||
| component.Queue.AddLast(new LatheRecipeBatch(recipe.RecipeId, true, 0, quantity)); |
Contributor
There was a problem hiding this comment.
Сохраняйте стоимость queued weekly-рецептов до live-edit.
Материалы списываются на Line 24-Line 25, но в очередь попадает только RecipeId; при WeeklyRecipesChangedEvent очередь не сверяется. Если админ изменит или удалит рецепт до завершения/удаления batch-а, refund в основном LatheSystem будет искать уже новую конфигурацию или ничего не вернёт, а недоступный batch может заблокировать lathe. Сохраняйте snapshot стоимости/результата в batch/current job или перед применением live-edit атомарно refund/reconcile все затронутые weekly jobs.
Also applies to: 35-43
🤖 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/Imperial/WeeklyMode/_SystemsSupplementation/LatheSystem.WeeklyMode.cs`
around lines 24 - 30, The issue is that materials are deducted immediately when
queuing a weekly recipe (in GetAdjustedWeeklyAmount and TryChangeMaterialAmount
calls), but only the RecipeId is stored in the LatheRecipeBatch. If an admin
modifies or deletes the recipe before the batch completes via
WeeklyRecipesChangedEvent, the refund logic in LatheSystem will fail because the
original recipe data no longer exists, potentially blocking the lathe. Fix this
by capturing and storing a snapshot of the deducted materials and recipe cost
information in the LatheRecipeBatch constructor at the time of queuing, so that
even if the recipe is later modified or deleted, the system can properly refund
or reconcile the original deducted amounts. Alternatively, add a handler for
WeeklyRecipesChangedEvent that atomically processes refunds and reconciliation
for all affected weekly job batches currently in the queue before the recipe
changes take effect.
Comment on lines
+340
to
+361
| public override async void Execute(IConsoleShell shell, string argStr, string[] args) | ||
| { | ||
| if (args.Length is < 3 or > 4 || | ||
| args.Length == 4 && args[3] != "--bypass-playtime") | ||
| { | ||
| shell.WriteError(Help); | ||
| return; | ||
| } | ||
|
|
||
| var target = await _weekly.ResolveForcedRoleTargetAsync(args[1]); | ||
| if (!target.Success) | ||
| { | ||
| shell.WriteError(target.Message); | ||
| return; | ||
| } | ||
|
|
||
| var createdBy = shell.Player?.Name ?? "server console"; | ||
| if (_weekly.TryForceRole(args[0], target.UserId, target.LastKnownCKey, args[2], args.Length == 4, createdBy, out var message)) | ||
| shell.WriteLine(message); | ||
| else | ||
| shell.WriteError(message); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверка наличия async void обработчиков и отсутствия локальной защиты try/catch в файле команд ролей.
rg -nP 'public override async void Execute\s*\(' Content.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Roles.cs -A40 -B2
rg -nP '\btry\s*\{' Content.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Roles.csRepository: imperial-space/SS14-public
Length of output: 5966
🏁 Script executed:
# Check the base class definition and Execute method signature
fd -e cs | xargs rg -l 'class LocalizedEntityCommands' | head -5Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Search for other Execute method implementations in command handlers
rg 'public override.*Execute\s*\(' Content.Server -A 2 | head -100Repository: imperial-space/SS14-public
Length of output: 7295
🏁 Script executed:
# Check if ResolveForcedRoleTargetAsync throws or returns results
rg 'ResolveForcedRoleTargetAsync' Content.Server -B 2 -A 10 | head -50Repository: imperial-space/SS14-public
Length of output: 5507
🏁 Script executed:
# Find the base class for these commands
fd -e cs -path '*Content.Shared*' -o -path '*Shared*' | xargs rg -l 'class.*EntityCommand' | head -10Repository: imperial-space/SS14-public
Length of output: 773
🏁 Script executed:
# Check specifically for LocalizedEntityCommands
rg 'class LocalizedEntityCommands' Content.Shared -A 10Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Look for how Whitelist commands (which also use async void) handle exceptions
cat -n Content.Server/Whitelist/WhitelistCommands.cs | head -150Repository: imperial-space/SS14-public
Length of output: 5861
🏁 Script executed:
# Find LocalizedCommands base class definition
fd -e cs | xargs rg 'class LocalizedCommands' | head -20Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Search for LocalizedEntityCommands base class
fd -e cs | xargs rg 'class LocalizedEntityCommands' | head -20Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Check if there's any try-catch in async void command handlers across the codebase
rg 'async void Execute' Content.Server -A 30 | rg -B 3 'try\s*\{'Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Search in Content.Shared for the base classes
fd -e cs Content.Shared | xargs rg 'class LocalizedCommands|class LocalizedEntityCommands' -B 2 -A 5Repository: imperial-space/SS14-public
Length of output: 54
🏁 Script executed:
# Look for IConsoleCommand interface to understand the Execute method contract
fd -e cs | xargs rg 'interface.*IConsoleCommand|public.*Execute.*IConsoleShell' | grep -i interface | head -10Repository: imperial-space/SS14-public
Length of output: 489
🏁 Script executed:
# Check how exceptions are typically handled in async void Execute methods
rg 'async void Execute' -A 40 | rg -B 5 -A 5 'catch|try' | head -80Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Search in all shared code for command interfaces
rg 'Execute.*IConsoleShell' --type cs -B 5 | grep 'interface\|public.*Execute' | head -30Repository: imperial-space/SS14-public
Length of output: 4107
🏁 Script executed:
# Check Robust.Shared.Console namespace for IConsoleCommand
fd -e cs | xargs rg 'namespace.*Console' | head -5Repository: imperial-space/SS14-public
Length of output: 588
🏁 Script executed:
# Look at the actual inheritance pattern - what do these command classes inherit from?
rg 'class.*Command.*:' Content.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Roles.cs | head -10Repository: imperial-space/SS14-public
Length of output: 754
🏁 Script executed:
# Search for where LocalizedEntityCommands is defined
rg 'LocalizedEntityCommands' --type cs | head -20Repository: imperial-space/SS14-public
Length of output: 2444
🏁 Script executed:
# Find LocalizedEntityCommands definition
fd -e cs | xargs rg 'class LocalizedEntityCommands' -B 2 -A 20Repository: imperial-space/SS14-public
Length of output: 54
🏁 Script executed:
# Check if Execute method signature in base class is async void or void
fd -e cs | xargs rg 'public.*Execute.*IConsoleShell' -B 5 | grep -A 5 'LocalizedEntityCommands\|interface'Repository: imperial-space/SS14-public
Length of output: 3692
🏁 Script executed:
# Look for any exception handling pattern documentation or comments in async void commands
rg 'async void Execute' -A 50 | rg -B 3 -A 3 'exception|error|throw|catch' | head -100Repository: imperial-space/SS14-public
Length of output: 4852
🏁 Script executed:
# Search with Python to find the actual LocalizedEntityCommands class definition
python3 << 'PY'
import subprocess
result = subprocess.run(['rg', 'class LocalizedEntityCommands', '-A', '15'],
capture_output=True, text=True)
print("STDOUT:", result.stdout)
print("STDERR:", result.stderr)
PYRepository: imperial-space/SS14-public
Length of output: 88
🏁 Script executed:
# Check if LocalizedEntityCommands is in Robust library or Content.Shared
fd -e cs | xargs rg 'LocalizedEntityCommands' | cut -d: -f1 | sort -uRepository: imperial-space/SS14-public
Length of output: 5804
🏁 Script executed:
# Look for the actual Execute method signature in base class or interface
fd -e cs | xargs rg 'public.*Execute.*IConsoleShell.*string' | head -30Repository: imperial-space/SS14-public
Length of output: 4116
🏁 Script executed:
# Check the actual ResolveForcedRoleTargetAsync implementation more carefully
rg 'public async Task<.*ResolveForcedRoleTargetAsync' Content.Server/Imperial/WeeklyMode/Systems/WeeklyModeSystem.cs -A 30Repository: imperial-space/SS14-public
Length of output: 1411
🏁 Script executed:
# Check if there are any throw statements in ResolveForcedRoleTargetAsync
rg 'ResolveForcedRoleTargetAsync' -A 20 | rg 'throw' -B 3Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Look at how other async void commands in Whitelist handle the pattern
rg 'async void Execute' Content.Server/Whitelist/WhitelistCommands.cs -A 30 | head -100Repository: imperial-space/SS14-public
Length of output: 3137
🏁 Script executed:
# Check if _playerLocator methods can throw exceptions
rg 'interface.*IPlayerLocator|class.*PlayerLocator' -A 50 | rg 'LookupIdAsync|LookupIdByNameAsync' -B 2 -A 10 | head -80Repository: imperial-space/SS14-public
Length of output: 4863
🏁 Script executed:
# Look for exception throwing in PlayerLocator implementation
fd -e cs | xargs rg 'LookupIdAsync|LookupIdByNameAsync' -B 3 -A 15 | grep -A 10 'throws\|throw ' | head -60Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Check what methods the _playerLocator can call that might throw
rg 'GetPlayerRecordByUserName|GetPlayerRecordByUserId' Content.Server/Administration/PlayerLocator.cs -A 20 | head -60Repository: imperial-space/SS14-public
Length of output: 1973
🏁 Script executed:
# Look for any actual exception throwing in database calls
rg 'GetPlayerRecordByUserName|GetPlayerRecordByUserId' -A 10 | rg 'throw|exception' -B 3 -A 3Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Final check: Do other async void commands use try-catch?
rg 'async void Execute' -A 50 | rg -c 'try\s*\{'Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Check if any other async void commands use try-catch patterns
rg 'async void Execute' Content.Server -A 80 -B 2 | grep -A 20 'try {' | head -40Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Look at DirtyCommand which also has async void
cat -n Content.Server/Administration/Commands/DirtyCommand.cs | head -60Repository: imperial-space/SS14-public
Length of output: 1640
🏁 Script executed:
# Check if _httpClient.GetAsync() can throw and whether that's wrapped
rg 'HttpClient.*GetAsync' Content.Server/Administration/PlayerLocator.cs -B 5 -A 10Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Look for the HandleAuthServerResponse to see if exceptions are handled
rg 'HandleAuthServerResponse' Content.Server/Administration/PlayerLocator.cs -A 30Repository: imperial-space/SS14-public
Length of output: 2330
🏁 Script executed:
# Verify the base class method signature for Execute in LocalizedEntityCommands
find . -name "*.cs" -type f -exec grep -l "abstract.*Execute\|interface.*Execute" {} \; | head -10Repository: imperial-space/SS14-public
Length of output: 126
🏁 Script executed:
# Try to find the base class definition in Robust library or shared code more carefully
rg 'public.*Execute.*IConsoleShell' -B 10 | grep -A 10 'abstract\|interface' | head -40Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Check if there's a try-catch handler at the framework level for async void commands
rg 'Execute.*async void' -B 5 -A 50 | rg 'try|catch|exception' | head -20Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Examine the full PlayerLocator to understand all possible exception sources
cat -n Content.Server/Administration/PlayerLocator.cs | sed -n '1,150p'Repository: imperial-space/SS14-public
Length of output: 7621
🏁 Script executed:
# Final verification: check what the actual interface or abstract base requires
rg 'class LocalizedEntityCommands|class LocalizedCommands' -A 30 | head -100Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Look for the exact Execute method signature requirement
fd -e cs | xargs rg 'public.*void Execute.*IConsoleShell' | grep -E 'abstract|interface' -B 2 -A 2 | head -30Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Check the imports at the top of the WeeklyModeCommands.Roles.cs file to see where LocalizedEntityCommands comes from
head -30 Content.Server/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Roles.csRepository: imperial-space/SS14-public
Length of output: 1109
🏁 Script executed:
# Try to find where LocalizedEntityCommands is actually defined using a broader search
find . -name "*.cs" -type f -exec grep -l "class LocalizedEntityCommands" {} \;Repository: imperial-space/SS14-public
Length of output: 52
🏁 Script executed:
# Check if Robust.Shared has the base definition
fd -e cs | xargs rg 'class.*Commands.*public.*Execute' | grep Robust | head -10Repository: imperial-space/SS14-public
Length of output: 52
Добавьте обработку исключений к async void методам на строках 340, 385, 467, 495.
Методы используют async void и выполняют вызовы через await _weekly.ResolveForcedRoleTargetAsync(), который транзитивно вызывает HTTP-операции (HttpClient.GetAsync()) через _playerLocator. Исключения из этих операций (HttpRequestException, TaskCanceledException и т.д.) не будут перехвачены текущим кодом и станут необработанными в синхронном контексте. Оберните каждый вызов await локальным try-catch блоком для преобразования потенциальных исключений в управляемые ошибки через shell.WriteError().
🤖 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/Imperial/WeeklyMode/Administration/Commands/WeeklyModeCommands.Roles.cs`
around lines 340 - 361, The Execute method (and similar async void command
handlers) contains an await call to _weekly.ResolveForcedRoleTargetAsync() which
can throw unhandled exceptions such as HttpRequestException and
TaskCanceledException from underlying HTTP operations, causing crashes in the
synchronous context. Wrap the await _weekly.ResolveForcedRoleTargetAsync() call
and any other await operations in a try-catch block, and in the catch block use
shell.WriteError() to convert these exceptions into managed error messages that
are properly reported to the console instead of becoming unhandled exceptions.
| public string Notes { get; set; } = string.Empty; | ||
| public string CreatedBy { get; set; } = string.Empty; | ||
| public long BundleSizeBytes { get; set; } | ||
| public bool CompatibleWithBuild { get; set; } |
Contributor
There was a problem hiding this comment.
Потерян безопасный дефолт для CompatibleWithBuild.
На Line 168 свойство теперь по умолчанию false. В пути создания WeeklySnapshotMetadata это поле явно не заполняется, поэтому новые снапшоты будут помечаться как несовместимые без реальной проверки. Верните дефолт = true или устанавливайте значение явно при создании метаданных.
💡 Предложение правки
- public bool CompatibleWithBuild { get; set; }
+ public bool CompatibleWithBuild { get; set; } = true;🤖 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/Imperial/WeeklyMode/WeeklyModeModels.cs` at line 168, The
CompatibleWithBuild property in WeeklyModeModels.cs is missing its safe default
value and currently defaults to false, causing new WeeklySnapshotMetadata
instances to be marked as incompatible without any actual validation. Either
restore the default value by adding = true to the CompatibleWithBuild property
declaration, or locate all creation paths of WeeklySnapshotMetadata objects and
explicitly set CompatibleWithBuild to true during initialization to ensure new
snapshots are properly marked as compatible by default.
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.
О ПР`е
Тип: feat
Изменения: Ивентовый контент
Технические детали
Назначение
Добавлен Weekly Mode для проведения долгосрочных ивентовых кампаний с сохранением состояния мира между запусками сервера.
Режим активируется только для выбранной кампании. Вне активной Weekly-кампании используются стандартные игровые системы.
Кампании
Сохранение мира
snapshot.Автосохранение
Роли
Гарантированные роли
Ограничение по игровому времени
Животные и роботы
Киборги
mind,actorи связи с игроком не переносятся в сохранение.Исследования
Снабжение
Рецепты
Игровые правила
Настройка во время игры
Завершение режима
Изоляция
Тестирование
Content.Serverвыполнена успешно.Content.Clientвыполнена успешно.Content.Tests: 402 теста пройдено, 1 пропущен.git diff --check upstream/develop...HEADвыполнен без ошибок.Ручная проверка
snapshot.Buckle,StrapиJoint.Со списком команд и примерами их использования можно ознакомиться здесь:
https://docs.google.com/spreadsheets/d/1Vj1ug122eEULwSHbS9QCfifdHZDNpPkkQ3x6cw0hoek/edit?usp=sharing
Изменения кода официальных разработчиков
latejoinи распределение профессий добавлены кампанийные ограничения доступа, лимиты ролей, переименования профессий и гарантированные назначения игроков.Summary by CodeRabbit
Release Notes
New Features