-
Notifications
You must be signed in to change notification settings - Fork 154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Port] Surgery Fixes #965
[Port] Surgery Fixes #965
Changes from 6 commits
f6604c1
11d8926
300e58f
64130bd
0d32b54
b706412
3a547c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -133,7 +133,10 @@ public void Execute(IConsoleShell shell, string argStr, string[] args) | |||||||||||||||||||||||||||||||||||
if (attachAt == default) | ||||||||||||||||||||||||||||||||||||
attachAt = bodySystem.GetBodyChildren(entity, body).First(); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
var slotId = part.GetHashCode().ToString(); | ||||||||||||||||||||||||||||||||||||
// Shitmed Change Start | ||||||||||||||||||||||||||||||||||||
var slotId = $"{part.Symmetry.ToString().ToLower()} {part.GetHashCode().ToString()}"; | ||||||||||||||||||||||||||||||||||||
part.SlotId = part.GetHashCode().ToString(); | ||||||||||||||||||||||||||||||||||||
// Shitmed Change End | ||||||||||||||||||||||||||||||||||||
Comment on lines
+136
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Критическая проблема: Несоответствие идентификаторов слота и части тела Обнаружено несоответствие между генерируемыми идентификаторами:
Это несоответствие может привести к ошибкам при попытке прикрепления части тела. Предлагаемое исправление: - var slotId = $"{part.Symmetry.ToString().ToLower()} {part.GetHashCode().ToString()}";
- part.SlotId = part.GetHashCode().ToString();
+ var hashCode = part.GetHashCode().ToString();
+ var slotId = $"{part.Symmetry.ToString().ToLower()} {hashCode}";
+ part.SlotId = slotId; 📝 Committable suggestion
Suggested change
Comment on lines
+136
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Критическая проблема: Небезопасное использование GetHashCode() Использование GetHashCode() для генерации идентификаторов слотов может привести к проблемам, так как:
Предлагаемое решение: - var slotId = $"{part.Symmetry.ToString().ToLower()} {part.GetHashCode().ToString()}";
- part.SlotId = part.GetHashCode().ToString();
+ var slotId = $"{part.Symmetry.ToString().ToLower()}_{Guid.NewGuid():N}";
+ part.SlotId = slotId; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (!bodySystem.TryCreatePartSlotAndAttach(attachAt.Id, slotId, hand, BodyPartType.Hand, attachAt.Component, part)) | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -98,8 +98,15 @@ public void Execute(IConsoleShell shell, string argStr, string[] args) | |||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
var slotId = $"AttachBodyPartVerb-{partUid}"; | ||||||||||||||||||||||||||||||
// Shitmed Change Start | ||||||||||||||||||||||||||||||
var slotId = ""; | ||||||||||||||||||||||||||||||
if (part.Symmetry != BodyPartSymmetry.None) | ||||||||||||||||||||||||||||||
slotId = $"{part.Symmetry.ToString().ToLower()} {part.GetHashCode().ToString()}"; | ||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||
slotId = $"{part.GetHashCode().ToString()}"; | ||||||||||||||||||||||||||||||
Comment on lines
+102
to
+106
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Несогласованность в генерации идентификаторов В коде присутствует несогласованность между генерацией
Это может привести к проблемам при сопоставлении слотов и частей тела. Предлагаемое исправление: var slotId = "";
if (part.Symmetry != BodyPartSymmetry.None)
slotId = $"{part.Symmetry.ToString().ToLower()} {part.GetHashCode().ToString()}";
else
slotId = $"{part.GetHashCode().ToString()}";
-part.SlotId = part.GetHashCode().ToString();
+part.SlotId = slotId; Also applies to: 108-108
Comment on lines
+102
to
+106
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Улучшите генерацию идентификаторов слотов Текущая реализация генерации
Предлагаю следующие изменения: - var slotId = "";
- if (part.Symmetry != BodyPartSymmetry.None)
- slotId = $"{part.Symmetry.ToString().ToLower()} {part.GetHashCode().ToString()}";
- else
- slotId = $"{part.GetHashCode().ToString()}";
+ var slotId = part.Symmetry != BodyPartSymmetry.None
+ ? $"{part.Symmetry.ToString().ToLower()}_{part.GetType().Name}_{Guid.NewGuid()}"
+ : $"{part.GetType().Name}_{Guid.NewGuid()}"; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
part.SlotId = part.GetHashCode().ToString(); | ||||||||||||||||||||||||||||||
// Shitmed Change End | ||||||||||||||||||||||||||||||
Comment on lines
+101
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Необходимо улучшить генерацию идентификаторов слотов Текущая реализация имеет несколько проблем:
Предлагаю следующие изменения: - // Shitmed Change Start
- var slotId = "";
- if (part.Symmetry != BodyPartSymmetry.None)
- slotId = $"{part.Symmetry.ToString().ToLower()} {part.GetHashCode().ToString()}";
- else
- slotId = $"{part.GetHashCode().ToString()}";
-
- part.SlotId = part.GetHashCode().ToString();
- // Shitmed Change End
+ var slotId = part.Symmetry != BodyPartSymmetry.None
+ ? $"{part.Symmetry.ToString().ToLower()}_{Guid.NewGuid():N}"
+ : Guid.NewGuid().ToString("N");
+
+ part.SlotId = slotId; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
// ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract | ||||||||||||||||||||||||||||||
if (body.RootContainer.ContainedEntity != null) | ||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
using Content.Shared.Backmen.Surgery.Steps.Parts; | ||
using Content.Shared.Damage.Prototypes; | ||
using Content.Shared.FixedPoint; | ||
using Content.Shared.Inventory; | ||
|
||
// ReSharper disable once CheckNamespace | ||
namespace Content.Shared.Body.Systems; | ||
|
@@ -181,23 +182,20 @@ private void OnBodyDamageModify(Entity<BodyComponent> bodyEnt, ref DamageModifyE | |
if (args.TargetPart != null) | ||
{ | ||
var (targetType, _) = ConvertTargetBodyPart(args.TargetPart.Value); | ||
args.Damage = args.Damage * GetPartDamageModifier(targetType); | ||
args.Damage *= GetPartDamageModifier(targetType); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Изменена логика модификации урона Модификация урона теперь учитывает инвентарь, что может привести к неожиданному поведению при отсутствии компонента инвентаря. Рекомендуется:
Предлагаемые изменения: - if (partEnt.Comp.Body != null
- && TryComp(partEnt.Comp.Body.Value, out InventoryComponent? inventory))
- _inventory.RelayEvent((partEnt.Comp.Body.Value, inventory), ref args);
+ if (partEnt.Comp.Body != null)
+ {
+ if (TryComp(partEnt.Comp.Body.Value, out InventoryComponent? inventory))
+ {
+ _inventory.RelayEvent((partEnt.Comp.Body.Value, inventory), ref args);
+ }
+ else
+ {
+ Log.Debug($"No inventory component found for body {partEnt.Comp.Body.Value}");
+ }
+ } Also applies to: 192-193, 198-198 |
||
} | ||
} | ||
|
||
private void OnPartDamageModify(Entity<BodyPartComponent> partEnt, ref DamageModifyEvent args) | ||
{ | ||
if (partEnt.Comp.Body != null | ||
&& TryComp(partEnt.Comp.Body.Value, out DamageableComponent? damageable) | ||
&& damageable.DamageModifierSetId != null | ||
&& _prototypeManager.TryIndex<DamageModifierSetPrototype>(damageable.DamageModifierSetId, out var modifierSet)) | ||
// TODO: We need to add a check to see if the given armor covers this part to cancel or not. | ||
args.Damage = DamageSpecifier.ApplyModifierSet(args.Damage, modifierSet); | ||
&& TryComp(partEnt.Comp.Body.Value, out InventoryComponent? inventory)) | ||
_inventory.RelayEvent((partEnt.Comp.Body.Value, inventory), ref args); | ||
|
||
if (_prototypeManager.TryIndex<DamageModifierSetPrototype>("PartDamage", out var partModifierSet)) | ||
args.Damage = DamageSpecifier.ApplyModifierSet(args.Damage, partModifierSet); | ||
|
||
args.Damage = args.Damage * GetPartDamageModifier(partEnt.Comp.PartType); | ||
args.Damage *= GetPartDamageModifier(partEnt.Comp.PartType); | ||
} | ||
|
||
private bool TryChangePartDamage(EntityUid entity, | ||
|
@@ -300,7 +298,7 @@ private static TargetBodyPart GetRandomPartSpread(IRobustRandom random, ushort t | |
|
||
/// This should be called after body part damage was changed. | ||
/// </summary> | ||
protected void CheckBodyPart( | ||
public void CheckBodyPart( | ||
Entity<BodyPartComponent> partEnt, | ||
TargetBodyPart? targetPart, | ||
bool severed, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
using Robust.Shared.GameStates; | ||
using Robust.Shared.Prototypes; | ||
|
||
namespace Content.Shared.Backmen.Surgery; | ||
|
||
/// <summary> | ||
/// Prevents the entity from causing toxin damage to entities it does surgery on. | ||
/// </summary> | ||
[RegisterComponent, NetworkedComponent] | ||
public sealed partial class SanitizedComponent : Component { } |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,6 +15,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Popups; | ||||||||||||||||||||||||||||||||||||||||||||||||
using Robust.Shared.Prototypes; | ||||||||||||||||||||||||||||||||||||||||||||||||
using System.Linq; | ||||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Backmen.Surgery; | ||||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Backmen.Mood; | ||||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Backmen.Surgery.Body.Events; | ||||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Backmen.Surgery.Body.Organs; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -23,7 +24,6 @@ | |||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Backmen.Surgery.Steps.Parts; | ||||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Backmen.Surgery.Tools; | ||||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Containers.ItemSlots; | ||||||||||||||||||||||||||||||||||||||||||||||||
using Content.Shared.Medical.Surgery; | ||||||||||||||||||||||||||||||||||||||||||||||||
using AmputateAttemptEvent = Content.Shared.Body.Events.AmputateAttemptEvent; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
namespace Content.Shared.Backmen.Surgery; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -127,11 +127,14 @@ private void OnToolStep(Entity<SurgeryStepComponent> ent, ref SurgeryStepEvent a | |||||||||||||||||||||||||||||||||||||||||||||||
RaiseLocalEvent(args.Body, new MoodEffectEvent("SurgeryPain")); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if (!_inventory.TryGetSlotEntity(args.User, "gloves", out var _) | ||||||||||||||||||||||||||||||||||||||||||||||||
|| !_inventory.TryGetSlotEntity(args.User, "mask", out var _)) | ||||||||||||||||||||||||||||||||||||||||||||||||
|| !_inventory.TryGetSlotEntity(args.User, "mask", out var _)) | ||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||
var sepsis = new DamageSpecifier(_prototypes.Index<DamageTypePrototype>("Poison"), 5); | ||||||||||||||||||||||||||||||||||||||||||||||||
var ev = new SurgeryStepDamageEvent(args.User, args.Body, args.Part, args.Surgery, sepsis, 0.5f); | ||||||||||||||||||||||||||||||||||||||||||||||||
RaiseLocalEvent(args.Body, ref ev); | ||||||||||||||||||||||||||||||||||||||||||||||||
if (!HasComp<SanitizedComponent>(args.User)) | ||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||
var sepsis = new DamageSpecifier(_prototypes.Index<DamageTypePrototype>("Poison"), 5); | ||||||||||||||||||||||||||||||||||||||||||||||||
var ev = new SurgeryStepDamageEvent(args.User, args.Body, args.Part, args.Surgery, sepsis, 0.5f); | ||||||||||||||||||||||||||||||||||||||||||||||||
RaiseLocalEvent(args.Body, ref ev); | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+130
to
+137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Проверьте логику применения урона от сепсиса Текущая реализация имеет несколько проблем:
Предлагаемые изменения: -if (!_inventory.TryGetSlotEntity(args.User, "gloves", out var _)
- || !_inventory.TryGetSlotEntity(args.User, "mask", out var _))
+private const float SEPSIS_DAMAGE = 5f;
+
+if (!_inventory.TryGetSlotEntity(args.User, "gloves", out var _) &&
+ !_inventory.TryGetSlotEntity(args.User, "mask", out var _))
{
if (!HasComp<SanitizedComponent>(args.User))
{
- var sepsis = new DamageSpecifier(_prototypes.Index<DamageTypePrototype>("Poison"), 5);
+ var sepsis = new DamageSpecifier(_prototypes.Index<DamageTypePrototype>("Poison"), SEPSIS_DAMAGE);
var ev = new SurgeryStepDamageEvent(args.User, args.Body, args.Part, args.Surgery, sepsis, 0.5f);
RaiseLocalEvent(args.Body, ref ev);
}
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,7 +12,7 @@ | |||||||||||||||||||
namespace Content.Shared.Body.Part; | ||||||||||||||||||||
|
||||||||||||||||||||
[RegisterComponent, NetworkedComponent, AutoGenerateComponentState] | ||||||||||||||||||||
[Access(typeof(SharedBodySystem))] | ||||||||||||||||||||
//[Access(typeof(SharedBodySystem))] | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Удаление атрибута [Access] может привести к проблемам безопасности На основе анализа кодовой базы:
Рекомендуется:
🔗 Analysis chainВнимание: Проверьте необходимость отключения ограничения доступа! Комментирование атрибута [Access] может привести к нежелательному доступу к компоненту из других систем. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Поиск использования BodyPartComponent в других системах
ast-grep --pattern 'class $_ {
$$$
private $_($_ : BodyPartComponent)
$$$
}'
Length of output: 84 Script: #!/bin/bash
# Поиск использования BodyPartComponent в системах
rg -l "BodyPartComponent" --type cs
# Поиск прямых обращений к BodyPartComponent
rg "BodyPartComponent\." --type cs -A 2
# Поиск систем, которые используют BodyPartComponent
ast-grep --pattern 'class $_System {
$$$
BodyPartComponent
$$$
}'
Length of output: 1896 Script: #!/bin/bash
# Проверяем, используется ли SharedBodySystem как основная система для работы с BodyPartComponent
rg "class SharedBodySystem" --type cs -A 5
# Проверяем, есть ли другие системы с прямым доступом к BodyPartComponent
rg "GetComponent<BodyPartComponent>" --type cs
# Проверяем атрибуты доступа у других компонентов в той же области
rg "\[Access\]" --type cs --glob "Content.Shared/Body/**"
Length of output: 3780 |
||||||||||||||||||||
public sealed partial class BodyPartComponent : Component, ISurgeryToolComponent | ||||||||||||||||||||
{ | ||||||||||||||||||||
// Need to set this on container changes as it may be several transform parents up the hierarchy. | ||||||||||||||||||||
|
@@ -22,9 +22,6 @@ public sealed partial class BodyPartComponent : Component, ISurgeryToolComponent | |||||||||||||||||||
[DataField, AutoNetworkedField] | ||||||||||||||||||||
public EntityUid? Body; | ||||||||||||||||||||
|
||||||||||||||||||||
[DataField, AutoNetworkedField] | ||||||||||||||||||||
public EntityUid? OriginalBody; | ||||||||||||||||||||
|
||||||||||||||||||||
[DataField, AutoNetworkedField] | ||||||||||||||||||||
public BodyPartSlot? ParentSlot; | ||||||||||||||||||||
|
||||||||||||||||||||
|
@@ -38,6 +35,9 @@ public sealed partial class BodyPartComponent : Component, ISurgeryToolComponent | |||||||||||||||||||
[DataField, AlwaysPushInheritance] | ||||||||||||||||||||
public string ToolName { get; set; } = "A body part"; | ||||||||||||||||||||
|
||||||||||||||||||||
[DataField, AlwaysPushInheritance] | ||||||||||||||||||||
public string SlotId { get; set; } = ""; | ||||||||||||||||||||
|
||||||||||||||||||||
Comment on lines
+38
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Необходимо документировать назначение SlotId Добавлено новое свойство Предлагаю добавить документацию: + /// <summary>
+ /// Уникальный идентификатор слота для данной части тела.
+ /// Используется для определения позиции части тела в родительском объекте.
+ /// </summary>
[DataField, AlwaysPushInheritance]
public string SlotId { get; set; } = ""; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||
[DataField, AutoNetworkedField] | ||||||||||||||||||||
public bool? Used { get; set; } = null; | ||||||||||||||||||||
|
||||||||||||||||||||
|
@@ -153,7 +153,7 @@ public sealed partial class BodyPartComponent : Component, ISurgeryToolComponent | |||||||||||||||||||
public bool IsVital; | ||||||||||||||||||||
|
||||||||||||||||||||
[DataField, AutoNetworkedField] | ||||||||||||||||||||
public BodyPartSymmetry Symmetry = BodyPartSymmetry.None; | ||||||||||||||||||||
public BodyPartSymmetry Symmetry { get; set; } = BodyPartSymmetry.None; | ||||||||||||||||||||
|
||||||||||||||||||||
/// <summary> | ||||||||||||||||||||
/// When attached, the part will ensure these components on the entity, and delete them on removal. | ||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Проверьте надежность генерации идентификаторов слотов
Текущая реализация генерации slotId и SlotId использует GetHashCode(), что может привести к коллизиям. GetHashCode() не гарантирует уникальность между запусками приложения.
Предлагаю следующие изменения:
📝 Committable suggestion