Draft
Conversation
Contributor
📝 WalkthroughWalkthroughДобавлена новая система знаний о персонажах для игры, позволяющая игрокам постепенно изучать информацию об SCP, локациях и терминах через речь, чтение, радио и осмотры. Система скрывает личности неизвестных SCP, показывает подсказки при наличии знаний и отслеживает прогресс через несколько каналов приобретения. Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant Client
participant ExamineUI
participant Server
participant KnowledgeSystem
participant ChatSystem
Player->>Client: Говорит фразу с термином SCP
Client->>Server: SendEntitySpeak()
Server->>KnowledgeSystem: AnalyzeRecognitionText()
KnowledgeSystem->>KnowledgeSystem: Распознать SCP термины
KnowledgeSystem-->>Server: ScpKnowledgeTextAnalysis
Server->>ChatSystem: SendInVoiceRange() с анализом
ChatSystem->>KnowledgeSystem: GetScpKnowledgeWrappedMessage()
KnowledgeSystem-->>ChatSystem: Выделенный текст
ChatSystem->>Player: Сообщение с подсветкой неизвестных термин
Server->>KnowledgeSystem: RaiseScpKnowledgeSpeakEvent()
KnowledgeSystem->>KnowledgeSystem: TryGrantKnowledgeProgress() для слушателей
Player->>Client: Осматривает SCP-173
Client->>Client: ScpKnowledgeSystem.TryGetPredictedExamineData()
Client->>ExamineUI: OpenTooltip() с предсказанными данными
ExamineUI-->>Player: Скрытое имя если знание не получено
Client->>Server: ExamineInfoRequest
Server->>KnowledgeSystem: TryGrantExamineKnowledge()
Server->>KnowledgeSystem: TryGetKnownExamineNameOverride()
KnowledgeSystem->>KnowledgeSystem: Обновить состояние знаний
Server->>Server: ExamineInfoResponseMessage с nameOverride
Server-->>Client: Сообщение осмотра
Client->>ExamineUI: OpenTooltip() с реальным именем
ExamineUI-->>Player: Раскрытые детали SCP
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
coderabbitai
bot
added
🪟 Интерфейс 🪟
📦 YAML | Прототип 📦
📚 Изменения: Локализация 📚
labels
![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: 19
🤖 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/Examine/ExamineSystem.cs`:
- Around line 436-441: Avoid always allocating predictedVerbs by changing the
call pattern with _scpKnowledge.AddPredictedExamineVerbs: either modify
AddPredictedExamineVerbs to return a bool and an out List<Verb> (or return
IEnumerable<Verb>/nullable list) so the list is only created when there are
verbs, or add a pre-check API on _scpKnowledge (e.g. HasPredictedExamineVerbs)
and only instantiate predictedVerbs when that check returns true; update the
call site in ExamineSystem (predictedVerbs variable and the
AddPredictedExamineVerbs invocation) accordingly so you no longer allocate an
empty List<Verb> unconditionally.
In `@Content.Server/_Scp/Knowledge/ScpKnowledgeSpeechHeardEvent.cs`:
- Around line 4-7: ScpKnowledgeSpeechHeardEvent declares a mutable EntityUid[]
Listeners inside a readonly record struct; change its type to
ImmutableArray<EntityUid> to make it immutable, add the
System.Collections.Immutable using where needed, and update all call
sites/creators of ScpKnowledgeSpeechHeardEvent (places that construct the
record) to pass an ImmutableArray via ImmutableArray.Create(...) or
.ToImmutableArray() so existing foreach/read-only usage continues to work
without copying.
In `@Content.Server/_Scp/Knowledge/ScpKnowledgeSystem.cs`:
- Around line 26-32: The reusable buffers (_matchedKnowledgeBuffer,
_matchedKnowledgeIdsBuffer, _recognitionMatchesBuffer) are fields and get
cleared at the start of CollectKnowledgeMatches, which loses data if
CollectKnowledgeMatches is ever re-entered (e.g., via nested calls or event
handlers). Fix by making these buffers local to CollectKnowledgeMatches (or use
a push/pop stack-pool) instead of shared fields so each invocation has its own
collections; update CollectKnowledgeMatches to declare and use local
List<CachedKnowledgeMatch>, HashSet<ProtoId<ScpKnowledgePrototype>>, and
List<ScpKnowledgeTextMatch> and ensure AnalyzeRecognitionText still accepts/uses
them or is adapted to receive them as parameters.
In `@Content.Server/_Scp/Knowledge/ScpKnowledgeSystem.Presentation.cs`:
- Line 22: Поле `_highlightRanges` повторно используется и очищается в методе
`BuildHighlightedText`, что ломает данные при вложенных/рекурсивных вызовах;
замените его на локальную коллекцию внутри `BuildHighlightedText` (например,
создать `var highlightRanges = new List<TextHighlightRange>()` в начале метода)
и используйте эту локальную переменную во всех местах метода вместо
`_highlightRanges`, либо, если вы хотите оптимизировать аллокации, примените
`ArrayPool<TextHighlightRange>` или `stackalloc` для небольших буферов; удалите
или прекратите использование `_highlightRanges` поле в коде, чтобы предотвратить
будущие гонки/повреждение данных.
In `@Content.Server/_Scp/Knowledge/ScpKnowledgeTextAnalysis.cs`:
- Around line 16-18: В конструкторе ScpKnowledgeTextAnalysis снимите защитный
снимок входных коллекций вместо хранения внешних ссылок: создайте новые
коллекции из параметров matches и matchedKnowledgeIds (например new
List<ScpKnowledgeAnalyzedMatch>(matches).AsReadOnly() или
ImmutableArray/ToArray()) и присвойте их свойствам Matches и
MatchedKnowledgeIds; если TokenizedText представляет изменяемый объект —
выполните защитное копирование/клонирование при присвоении TokenizedText, чтобы
экземпляр был действительно неизменяемым.
In `@Content.Server/Chat/Systems/ChatSystem.cs`:
- Around line 677-685: The code eagerly allocates clearListeners,
obfuscatedListeners and calls
CreateScpKnowledgeTextAnalysis(message)/CreateScpKnowledgeTextAnalysis(obfuscatedMessage)
before checking for whisper recipients; change this to lazily initialize them
only when a listener is actually found — e.g. remove the initial List<EntityUid>
and analysis creations and instead create the lists and call
CreateScpKnowledgeTextAnalysis(...) at the first point where you detect a
listener (use null or Optional-like variables to test prior to creation), then
reuse those instances for subsequent listeners so no allocations occur if there
are no recipients.
In `@Content.Server/Radio/EntitySystems/RadioSystem.cs`:
- Line 2: Replace all fork-edit markers that use the "Fire" prefix with the
repository-standard "Sunrise" markers: change single-line comments like "// Fire
added - ..." to use "Sunrise-Edit" (e.g. "// Sunrise-Edit added - ...") and
replace multi-line markers "Fire ... start/end" with "Sunrise edit start" /
"Sunrise edit end". Specifically update the comment on the using directive
referencing Content.Server._Scp.Knowledge and the other occurrences around the
blocks noted (the markers at the locations corresponding to lines 39 and 76-82)
so they follow the exact "Sunrise-Edit" / "Sunrise edit start/end" format used
across the repo.
In `@Content.Shared/_Scp/Knowledge/ScpKnowledgePaperReadEvent.cs`:
- Around line 3-8: Change the event type ScpKnowledgePaperReadEvent from a class
to a readonly struct: replace "public sealed class ScpKnowledgePaperReadEvent :
EntityEventArgs" with "public readonly struct ScpKnowledgePaperReadEvent"
(remove the inheritance from EntityEventArgs), keep the same constructor
signature (ScpKnowledgePaperReadEvent(EntityUid paper, EntityUid user, string
content)) and the readonly fields Paper, User and Content so existing call sites
still work while eliminating GC pressure.
In `@Content.Shared/_Scp/Knowledge/ScpKnowledgeText.cs`:
- Around line 210-230: AddUniqueMatch and AddUniqueIndex perform linear searches
per insert causing O(n^2) behavior; switch to using HashSet to get O(1)
membership checks: replace the List-based uniqueness checks in AddUniqueMatch
and AddUniqueIndex with HashSet<ScpKnowledgeTextMatch> and HashSet<int>
respectively (either by changing the methods to accept a HashSet or by
maintaining a companion HashSet inside the class), ensure ScpKnowledgeTextMatch
implements proper Equals/GetHashCode (or provide an IEqualityComparer) before
using it in a HashSet, and update all callers to pass or maintain the HashSet
while still adding to any existing List if order or duplication-free list
semantics are required.
- Around line 80-82: Replace the manual array allocation/copy with the built-in
ToArray() call: instead of creating firstTokensArray via new
string[firstTokens.Count] and firstTokens.CopyTo(...), call
firstTokens.ToArray() and pass that result into the ScpKnowledgeCompiledPattern
constructor (keep pattern and root as-is); update the code around firstTokens,
firstTokensArray, and ScpKnowledgeCompiledPattern accordingly.
- Around line 8-11: NormalizeRecognitionText currently calls
TokenizeRecognitionText which performs full tokenization and allocations just to
return NormalizedText; change this by adding a lightweight normalization path:
either implement a new helper (e.g., NormalizeRecognitionTextLight or
NormalizeRecognitionOnly) that performs the same string clean-up steps as
TokenizeRecognitionText but without creating Token objects, or add a boolean
parameter to TokenizeRecognitionText (e.g., bool produceTokens) to skip token
list creation and return only the normalized string; then update
NormalizeRecognitionText to call the lightweight path so callers avoid
unnecessary allocations.
- Around line 497-533: MinTokenCount and CanMatchEmpty currently recompute over
_children on every access; compute and cache these once in the type's
constructor and have the MinTokenCount and CanMatchEmpty properties return the
cached fields instead. Add private readonly backing fields (e.g. _minTokenCount,
_canMatchEmpty) initialized by iterating _children in the constructor (reusing
logic in the current getters), keep CollectFirstTokens as-is since it needs to
traverse at runtime, and remove the loop logic from the property getters so they
simply return the cached values.
- Around line 569-578: GetRemainingMinTokenCount is recalculated on every
recursive MatchChild call causing O(N²); precompute suffix sums once and use
them during recursion. Add a private int[] (e.g., _suffixMinTokenCount) and
populate it once before the first MatchChild invocation by walking _children
from the end and storing cumulative MinTokenCount; then change
GetRemainingMinTokenCount to return the precomputed value from
_suffixMinTokenCount[childIndex] (or 0 if out of range) so MatchChild no longer
accumulates remaining tokens repeatedly.
- Around line 585-618: MinTokenCount and CanMatchEmpty in
ScpKnowledgeAlternativePatternNode are recomputed on every access; compute and
store their values once when the node is constructed (e.g., add private readonly
fields like _cachedMinTokenCount and _cachedCanMatchEmpty and initialize them
inside the constructor by iterating _options), then change the MinTokenCount and
CanMatchEmpty properties to simply return these cached fields; ensure the
constructor logic mirrors the current loops (using option.MinTokenCount and
option.CanMatchEmpty) so behavior is unchanged.
In `@Content.Shared/Examine/ExamineSystemMessages.cs`:
- Around line 39-42: Replace the repository fork marker "Fire" with
"Sunrise-Edit" in the C# edits for the ExamineInfoResponseMessage type: update
the comment on the field NameOverride and the inline comment on the constructor
parameter nameOverride (and any other "Fire" markers within the
ExamineInfoResponseMessage scope) so they read "Sunrise-Edit" instead of "Fire"
to match repository guidelines.
In `@Content.Shared/Paper/PaperComponent.cs`:
- Around line 16-19: Replace the incorrect "Fire" fork markers around the
upstream edits with the repository-standard "Sunrise" markers: locate the added
blocks in PaperComponent (the KnowledgeAuthorRanges field and the two other
edited blocks around the regions referencing PaperKnowledgeAuthorRange / the
authorship tracking code) and change comments like "// Fire added start - ..."
and "// Fire added end" to the prescribed Sunrise markers (use the
"Sunrise-Edit" / "Sunrise edit start" and "Sunrise edit end" style as your repo
requires) for all three blocks (the one with KnowledgeAuthorRanges and the
blocks at the other two edit regions).
In `@Content.Shared/Paper/PaperSystem.cs`:
- Around line 409-427: The current LCS diff allocates a full 2D array
lcs[oldMiddleLength + 1, newMiddleLength + 1], which can be memory-heavy;
replace it with a linear-memory DP using two 1D arrays (e.g., prev and curr of
length newMiddleLength + 1) and update the nested loops in the block that
computes lcs to compute curr[newIndex] from prev[newIndex-1], prev[newIndex],
and curr[newIndex-1], then swap prev and curr each oldIndex iteration; keep the
existing maxExactDiffArea check and the variables oldMiddleLength,
newMiddleLength, oldMiddleStart, newMiddleStart and oldContent/newContent
unchanged so behavior stays the same but memory usage drops to
O(newMiddleLength).
In `@Resources/Prototypes/_Scp/Roles/Jobs/Administration/director.yml`:
- Around line 36-82: The added ScpKnowledge block (the "type: ScpKnowledge"
entry and its "knownKnowledge" list) must be marked with the Sunrise fork
marker; insert the required YAML comment "# Sunrise-Edit" immediately above (or
directly next to) the modified "type: ScpKnowledge" block so the entire added
knownKnowledge list is clearly annotated as a Sunrise edit; update the "type:
ScpKnowledge" / "knownKnowledge" section to include that marker consistent with
other Sunrise edits.
In
`@Resources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_commandant.yml`:
- Around line 70-89: This block duplicates the ScpKnowledge knownKnowledge list
across multiple job prototypes; extract the repeated list into a single reusable
template or parent (e.g., create a shared prototype or include file containing
the ScpKnowledge entry and the knownKnowledge array) and replace the inline list
in heavy_containment_zone_commandant.yml (the type: ScpKnowledge /
knownKnowledge section) with a reference/inheritance to that shared template so
all jobs consume the same source of truth and avoid future divergence.
🪄 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: b470a472-c039-4d88-9678-4d38901d8cdf
📒 Files selected for processing (77)
Content.Client/Examine/ExamineSystem.csContent.Client/Paper/UI/PaperBoundUserInterface.csContent.Client/Paper/UI/PaperWindow.xaml.csContent.Client/_Scp/Knowledge/ScpKnowledgeSystem.csContent.IntegrationTests/Tests/_Scp/ScpKnowledgeSystemTest.csContent.IntegrationTests/_Sunrise/DummyMidiManager.csContent.Server/Chat/Systems/ChatSystem.csContent.Server/Examine/ExamineSystem.csContent.Server/Radio/EntitySystems/HeadsetSystem.csContent.Server/Radio/EntitySystems/RadioSystem.csContent.Server/_Scp/Knowledge/ChatSystem.ScpKnowledge.csContent.Server/_Scp/Knowledge/HeadsetSystem.ScpKnowledge.csContent.Server/_Scp/Knowledge/ScpKnowledgeChatMessage.csContent.Server/_Scp/Knowledge/ScpKnowledgeSpeechHeardEvent.csContent.Server/_Scp/Knowledge/ScpKnowledgeSystem.Acquisition.csContent.Server/_Scp/Knowledge/ScpKnowledgeSystem.Presentation.csContent.Server/_Scp/Knowledge/ScpKnowledgeSystem.csContent.Server/_Scp/Knowledge/ScpKnowledgeTextAnalysis.csContent.Server/_Sunrise/GameTicking/Events/RoundEndMusicSelectionEvent.csContent.Server/_Sunrise/GameTicking/Events/RoundLobbyReadyEvent.csContent.Shared/Examine/ExamineSystemMessages.csContent.Shared/Paper/PaperComponent.csContent.Shared/Paper/PaperSystem.csContent.Shared/_Scp/Knowledge/Components/ScpKnowledgeAcquisitionComponent.csContent.Shared/_Scp/Knowledge/Components/ScpKnowledgeComponent.csContent.Shared/_Scp/Knowledge/ScpKnowledgeEvents.csContent.Shared/_Scp/Knowledge/ScpKnowledgeLogic.csContent.Shared/_Scp/Knowledge/ScpKnowledgePaperReadEvent.csContent.Shared/_Scp/Knowledge/ScpKnowledgePrototype.csContent.Shared/_Scp/Knowledge/ScpKnowledgeText.csContent.Tests/Shared/_Scp/Knowledge/ScpKnowledgeTextTest.csResources/Locale/en-US/_strings/_scp/knowledge.ftlResources/Locale/ru-RU/_strings/_scp/knowledge.ftlResources/Prototypes/Entities/Mobs/Species/human.ymlResources/Prototypes/_Scp/Knowledge/knowledge.ymlResources/Prototypes/_Scp/Roles/Jobs/Administration/chief_technical_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/Administration/director.ymlResources/Prototypes/_Scp/Roles/Jobs/Administration/head_of_medical_sector.ymlResources/Prototypes/_Scp/Roles/Jobs/Administration/personnel_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/Administration/scientific_service_director.ymlResources/Prototypes/_Scp/Roles/Jobs/Administration/security_commander.ymlResources/Prototypes/_Scp/Roles/Jobs/Administration/senior_logistics_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/external_administrative_zone_commandant.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/external_administrative_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/field_medical_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/junior_external_administrative_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/CommandantSquad/senior_external_administrative_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/EngineeringTechnicalService/electrical_technical_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/EngineeringTechnicalService/general_technical_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/EngineeringTechnicalService/lead_technical_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/EngineeringTechnicalService/technical_trainee.ymlResources/Prototypes/_Scp/Roles/Jobs/EthicsCommittee/ethics_committee_representative.ymlResources/Prototypes/_Scp/Roles/Jobs/FoodAndSanitationService/botanist_phytogeneticist.ymlResources/Prototypes/_Scp/Roles/Jobs/FoodAndSanitationService/gastronomic_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/FoodAndSanitationService/service_personnel.ymlResources/Prototypes/_Scp/Roles/Jobs/InternalSecurity/internal_security_operative.ymlResources/Prototypes/_Scp/Roles/Jobs/InternalSecurity/mental_control_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/InternalTribunalService/judicial_inspector.ymlResources/Prototypes/_Scp/Roles/Jobs/InternalTribunalService/tribunal_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/LogisticsService/logistics_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_botanist.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_cook.ymlResources/Prototypes/_Scp/Roles/Jobs/LowAccessPersonnel/class_d_janitor.ymlResources/Prototypes/_Scp/Roles/Jobs/MedicalService/hospital_intern.ymlResources/Prototypes/_Scp/Roles/Jobs/MedicalService/medical_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/MedicalService/pharmaceutical_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/MedicalService/psychotherapist.ymlResources/Prototypes/_Scp/Roles/Jobs/MedicalService/senior_medical_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/ScientificService/anomarchology_researcher.ymlResources/Prototypes/_Scp/Roles/Jobs/ScientificService/junior_researcher.ymlResources/Prototypes/_Scp/Roles/Jobs/ScientificService/lead_researcher.ymlResources/Prototypes/_Scp/Roles/Jobs/ScientificService/production_specialist.ymlResources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_commandant.ymlResources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/heavy_containment_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/junior_heavy_containment_zone_officer.ymlResources/Prototypes/_Scp/Roles/Jobs/SpecialPurposeSquad/senior_heavy_containment_zone_officer.yml
💤 Files with no reviewable changes (3)
- Content.IntegrationTests/_Sunrise/DummyMidiManager.cs
- Content.Server/_Sunrise/GameTicking/Events/RoundLobbyReadyEvent.cs
- Content.Server/_Sunrise/GameTicking/Events/RoundEndMusicSelectionEvent.cs
Comment on lines
+436
to
+441
| // Fire added start - predict SCP knowledge examine verbs locally | ||
| predictedVerbs = new List<Verb>(); | ||
| _scpKnowledge.AddPredictedExamineVerbs(playerEnt.Value, entity, predictedVerbs); | ||
| if (predictedVerbs.Count == 0) | ||
| predictedVerbs = null; | ||
| // Fire added end |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Избыточная аллокация списка для предсказанных глаголов.
Список создаётся всегда, даже если AddPredictedExamineVerbs не добавит ни одного элемента. Можно избежать аллокации, используя out параметр или проверяя наличие глаголов до создания списка.
♻️ Предлагаемое исправление
- // Fire added start - predict SCP knowledge examine verbs locally
- predictedVerbs = new List<Verb>();
- _scpKnowledge.AddPredictedExamineVerbs(playerEnt.Value, entity, predictedVerbs);
- if (predictedVerbs.Count == 0)
- predictedVerbs = null;
- // Fire added end
+ // Fire added start - predict SCP knowledge examine verbs locally
+ if (_scpKnowledge.TryGetPredictedExamineVerbs(playerEnt.Value, entity, out predictedVerbs) && predictedVerbs.Count == 0)
+ predictedVerbs = null;
+ // Fire added end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Client/Examine/ExamineSystem.cs` around lines 436 - 441, Avoid always
allocating predictedVerbs by changing the call pattern with
_scpKnowledge.AddPredictedExamineVerbs: either modify AddPredictedExamineVerbs
to return a bool and an out List<Verb> (or return IEnumerable<Verb>/nullable
list) so the list is only created when there are verbs, or add a pre-check API
on _scpKnowledge (e.g. HasPredictedExamineVerbs) and only instantiate
predictedVerbs when that check returns true; update the call site in
ExamineSystem (predictedVerbs variable and the AddPredictedExamineVerbs
invocation) accordingly so you no longer allocate an empty List<Verb>
unconditionally.
Comment on lines
+4
to
+7
| public readonly record struct ScpKnowledgeSpeechHeardEvent( | ||
| EntityUid Source, | ||
| ScpKnowledgeTextAnalysis Analysis, | ||
| EntityUid[] Listeners); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверка мест создания/обработки события и потенциальной мутации Listeners.
# 1) Где создаётся и где обрабатывается событие
rg -nP --type=cs -C3 '\bScpKnowledgeSpeechHeardEvent\b'
# 2) Где используется args.Listeners / Listeners в контексте обработчиков
rg -nP --type=cs -C3 '\b(args\.)?Listeners\b'
# 3) Поиск возможных мутаций массива (индексация присваиванием/Array.* вызовы рядом с Listeners)
rg -nP --type=cs -C3 'Listeners\s*\[\s*.*\]\s*=|Array\.(Clear|Resize|Sort)\s*\(.*Listeners'Repository: space-sunrise/project-fire
Length of output: 3985
🏁 Script executed:
# Поиск всех обработчиков события и анализ доступа к Listeners
rg -A10 'OnSpeechHeard|SubscribeLocalEvent.*ScpKnowledgeSpeechHeardEvent' --type=csRepository: space-sunrise/project-fire
Length of output: 2098
🏁 Script executed:
# Проверка, есть ли другие места, где событие может быть переиспользовано или обработано по-другому
rg -B5 -A5 'ScpKnowledgeSpeechHeardEvent' --type=cs | grep -E '(EntityUid\[\]|RaiseLocalEvent|RaiseSharedEvent)'Repository: space-sunrise/project-fire
Length of output: 249
Рассмотрите использование ImmutableArray<EntityUid> для Listeners.
Поле EntityUid[] Listeners в readonly record struct остаётся изменяемым, хотя фактически код использует его только для чтения через foreach. Для увеличения безопасности и явного выражения намерения можно заменить на ImmutableArray<EntityUid>.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/_Scp/Knowledge/ScpKnowledgeSpeechHeardEvent.cs` around lines 4
- 7, ScpKnowledgeSpeechHeardEvent declares a mutable EntityUid[] Listeners
inside a readonly record struct; change its type to ImmutableArray<EntityUid> to
make it immutable, add the System.Collections.Immutable using where needed, and
update all call sites/creators of ScpKnowledgeSpeechHeardEvent (places that
construct the record) to pass an ImmutableArray via ImmutableArray.Create(...)
or .ToImmutableArray() so existing foreach/read-only usage continues to work
without copying.
Comment on lines
+26
to
+32
| private readonly Dictionary<EntProtoId, List<ProtoId<ScpKnowledgePrototype>>> _knowledgeByEntityPrototype = []; | ||
| private readonly Dictionary<string, List<CachedRecognitionPattern>> _knowledgePatternsByFirstToken = []; | ||
| private readonly Dictionary<EntityUid, CachedPaperAnalysis> _paperAnalysisCache = []; | ||
| private readonly List<CachedKnowledgeMatch> _matchedKnowledgeBuffer = []; | ||
| private readonly HashSet<ProtoId<ScpKnowledgePrototype>> _matchedKnowledgeIdsBuffer = []; | ||
| private readonly List<ScpKnowledgeTextMatch> _recognitionMatchesBuffer = []; | ||
| private readonly ScpKnowledgePatternMatchContext _patternMatchContext = new(); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Буферы для повторного использования - потенциальная проблема при вложенных вызовах.
Поля _matchedKnowledgeBuffer, _matchedKnowledgeIdsBuffer, _recognitionMatchesBuffer очищаются в начале CollectKnowledgeMatches, но если метод вызывается рекурсивно или из обработчика события во время итерации, данные могут быть потеряны.
В текущей реализации это маловероятно, так как AnalyzeRecognitionText вызывается синхронно и не триггерит события. Однако для защиты от будущих изменений рекомендуется добавить комментарий или использовать локальные коллекции для вложенных вызовов.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/_Scp/Knowledge/ScpKnowledgeSystem.cs` around lines 26 - 32,
The reusable buffers (_matchedKnowledgeBuffer, _matchedKnowledgeIdsBuffer,
_recognitionMatchesBuffer) are fields and get cleared at the start of
CollectKnowledgeMatches, which loses data if CollectKnowledgeMatches is ever
re-entered (e.g., via nested calls or event handlers). Fix by making these
buffers local to CollectKnowledgeMatches (or use a push/pop stack-pool) instead
of shared fields so each invocation has its own collections; update
CollectKnowledgeMatches to declare and use local List<CachedKnowledgeMatch>,
HashSet<ProtoId<ScpKnowledgePrototype>>, and List<ScpKnowledgeTextMatch> and
ensure AnalyzeRecognitionText still accepts/uses them or is adapted to receive
them as parameters.
| [Dependency] private readonly UserInterfaceSystem _ui = default!; | ||
|
|
||
| private const string KnowledgeHighlightColor = "#8b0000"; | ||
| private readonly List<TextHighlightRange> _highlightRanges = []; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Повторное использование экземплярного списка _highlightRanges может привести к проблемам при вложенных вызовах.
Поле _highlightRanges очищается и переиспользуется в BuildHighlightedText. Если метод будет вызван рекурсивно или вложенно (например, через событие), данные будут повреждены.
♻️ Предлагаемое исправление
Использовать локальную переменную вместо поля класса:
- private readonly List<TextHighlightRange> _highlightRanges = [];И в BuildHighlightedText:
+ var highlightRanges = new List<TextHighlightRange>();
// ... use highlightRanges instead of _highlightRangesЛибо использовать ArrayPool<T> или stackalloc для небольших буферов.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/_Scp/Knowledge/ScpKnowledgeSystem.Presentation.cs` at line 22,
Поле `_highlightRanges` повторно используется и очищается в методе
`BuildHighlightedText`, что ломает данные при вложенных/рекурсивных вызовах;
замените его на локальную коллекцию внутри `BuildHighlightedText` (например,
создать `var highlightRanges = new List<TextHighlightRange>()` в начале метода)
и используйте эту локальную переменную во всех местах метода вместо
`_highlightRanges`, либо, если вы хотите оптимизировать аллокации, примените
`ArrayPool<TextHighlightRange>` или `stackalloc` для небольших буферов; удалите
или прекратите использование `_highlightRanges` поле в коде, чтобы предотвратить
будущие гонки/повреждение данных.
Comment on lines
+16
to
+18
| public ScpKnowledgeTokenizedText TokenizedText { get; } = tokenizedText; | ||
| public IReadOnlyList<ScpKnowledgeAnalyzedMatch> Matches { get; } = matches; | ||
| public IReadOnlyList<ProtoId<ScpKnowledgePrototype>> MatchedKnowledgeIds { get; } = matchedKnowledgeIds; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Сделайте снимок входных коллекций в конструкторе для реальной неизменяемости.
Сейчас объект хранит внешние ссылки на коллекции; их можно изменить после создания экземпляра.
♻️ Предлагаемое улучшение
- public IReadOnlyList<ScpKnowledgeAnalyzedMatch> Matches { get; } = matches;
- public IReadOnlyList<ProtoId<ScpKnowledgePrototype>> MatchedKnowledgeIds { get; } = matchedKnowledgeIds;
+ public IReadOnlyList<ScpKnowledgeAnalyzedMatch> Matches { get; } = new List<ScpKnowledgeAnalyzedMatch>(matches);
+ public IReadOnlyList<ProtoId<ScpKnowledgePrototype>> MatchedKnowledgeIds { get; } = new List<ProtoId<ScpKnowledgePrototype>>(matchedKnowledgeIds);📝 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.
Suggested change
| public ScpKnowledgeTokenizedText TokenizedText { get; } = tokenizedText; | |
| public IReadOnlyList<ScpKnowledgeAnalyzedMatch> Matches { get; } = matches; | |
| public IReadOnlyList<ProtoId<ScpKnowledgePrototype>> MatchedKnowledgeIds { get; } = matchedKnowledgeIds; | |
| public ScpKnowledgeTokenizedText TokenizedText { get; } = tokenizedText; | |
| public IReadOnlyList<ScpKnowledgeAnalyzedMatch> Matches { get; } = new List<ScpKnowledgeAnalyzedMatch>(matches); | |
| public IReadOnlyList<ProtoId<ScpKnowledgePrototype>> MatchedKnowledgeIds { get; } = new List<ProtoId<ScpKnowledgePrototype>>(matchedKnowledgeIds); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/_Scp/Knowledge/ScpKnowledgeTextAnalysis.cs` around lines 16 -
18, В конструкторе ScpKnowledgeTextAnalysis снимите защитный снимок входных
коллекций вместо хранения внешних ссылок: создайте новые коллекции из параметров
matches и matchedKnowledgeIds (например new
List<ScpKnowledgeAnalyzedMatch>(matches).AsReadOnly() или
ImmutableArray/ToArray()) и присвойте их свойствам Matches и
MatchedKnowledgeIds; если TokenizedText представляет изменяемый объект —
выполните защитное копирование/клонирование при присвоении TokenizedText, чтобы
экземпляр был действительно неизменяемым.
Comment on lines
+409
to
+427
| const long maxExactDiffArea = 4_000_000; | ||
| if ((long) oldMiddleLength * newMiddleLength > maxExactDiffArea) | ||
| return; | ||
|
|
||
| var lcs = new int[oldMiddleLength + 1, newMiddleLength + 1]; | ||
| for (var oldIndex = 1; oldIndex <= oldMiddleLength; oldIndex++) | ||
| { | ||
| var oldCharacter = oldContent[oldMiddleStart + oldIndex - 1]; | ||
| for (var newIndex = 1; newIndex <= newMiddleLength; newIndex++) | ||
| { | ||
| if (oldCharacter == newContent[newMiddleStart + newIndex - 1]) | ||
| { | ||
| lcs[oldIndex, newIndex] = lcs[oldIndex - 1, newIndex - 1] + 1; | ||
| continue; | ||
| } | ||
|
|
||
| lcs[oldIndex, newIndex] = Math.Max(lcs[oldIndex - 1, newIndex], lcs[oldIndex, newIndex - 1]); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Потенциально высокое потребление памяти при LCS-диффе.
Двумерный массив lcs[oldMiddleLength + 1, newMiddleLength + 1] может занимать до ~16 МБ при максимальном разрешённом размере (4M ячеек × 4 байта). Это происходит при каждом редактировании бумаги со средним измененённым участком.
♻️ Альтернативный подход
Для экономии памяти можно использовать алгоритм Хиршберга (линейная память O(n+m)) или Myers' diff algorithm. Однако текущий лимит maxExactDiffArea является разумной защитой для типичных случаев использования.
Если память станет проблемой, рассмотрите:
// Использовать только две строки вместо полной матрицы
var prev = new int[newMiddleLength + 1];
var curr = new int[newMiddleLength + 1];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Shared/Paper/PaperSystem.cs` around lines 409 - 427, The current LCS
diff allocates a full 2D array lcs[oldMiddleLength + 1, newMiddleLength + 1],
which can be memory-heavy; replace it with a linear-memory DP using two 1D
arrays (e.g., prev and curr of length newMiddleLength + 1) and update the nested
loops in the block that computes lcs to compute curr[newIndex] from
prev[newIndex-1], prev[newIndex], and curr[newIndex-1], then swap prev and curr
each oldIndex iteration; keep the existing maxExactDiffArea check and the
variables oldMiddleLength, newMiddleLength, oldMiddleStart, newMiddleStart and
oldContent/newContent unchanged so behavior stays the same but memory usage
drops to O(newMiddleLength).
Contributor
There was a problem hiding this comment.
@helper342, заяц оставил нитпик 🐰 — в методе CopyUnchangedKnowledgeAuthors используется полный двумерный массив lcs[oldMiddleLength + 1, newMiddleLength + 1] для алгоритма LCS (поиска наибольшей общей подпоследовательности). При максимальном допустимом размере (~4M ячеек) это может занимать до ~16 МБ памяти при каждом редактировании бумаги.
Заяц предложил оптимизацию — использовать две одномерных строки вместо полной матрицы, что сократит потребление памяти с O(n×m) до O(m). Но поскольку уже есть защитная проверка maxExactDiffArea = 4_000_000, это нитпик тривиального уровня, не блокирующий слияние. 🐇
🧠 Learnings used
Learnt from: ThereDrD0
Repo: space-sunrise/fire-station PR: 704
File: Content.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Face.cs:90-92
Timestamp: 2025-12-08T22:39:14.399Z
Learning: In code reviews, verify usages of IsClientSide(Entity): it should reflect the entity’s client-side status, not the current execution context. Server entities that are replicated to the client will have IsClientSide() == false. Ensure checks rely on the entity’s client-side property rather than where the code is running.
Learnt from: ThereDrD0
Repo: space-sunrise/fire-station PR: 738
File: Content.Shared/_Scp/Scp096/Main/Systems/SharedScp096System.Face.cs:198-214
Timestamp: 2026-01-05T03:55:27.757Z
Learning: В проектах на C# с ECS-архитектурой (как в space-station) следует использовать Comp<T>() вместо TryComp там, где компонент обязателен по дизайну и неизбежно должен существовать (например, в SpawnFace для Scp096FaceComponent). Это позволяет ловить ошибки конфигурации прототипов через исключения, а не добавлять защитные проверки. Применяйте к файлам, где контракт компонента требует его наличия; если компонент может отсутствовать по условию, используйте TryComp.
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
Closes #907
Медиа (Видео/Скриншоты) | Media (Video/Screenshots)
TODO
Changelog
🆑 ThereDrD
Summary by CodeRabbit
Примечания к выпуску