Skip to content
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

Ports #987

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Ports #987

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
using Content.Client.UserInterface.Controls;
using Robust.Client.UserInterface;
using Robust.Shared.Input;
using System.Linq;
using Content.Shared._CorvaxNext.Medical.SmartFridge;
using SmartFridgeMenu = Content.Client._CorvaxNext.Medical.SmartFridge.UI.SmartFridgeMenu;

namespace Content.Client._CorvaxNext.Medical.SmartFridge;

public sealed class SmartFridgeBoundUserInterface(EntityUid owner, Enum uiKey) : BoundUserInterface(owner, uiKey)
{
[ViewVariables]
private SmartFridgeMenu? _menu;

[ViewVariables]
private List<SmartFridgeInventoryItem> _cachedInventory = [];

Comment on lines +10 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Отсутствует освобождение ресурсов

Класс управляет UI-ресурсами, но не реализует IDisposable для их корректного освобождения.

Рекомендуется добавить реализацию IDisposable:

-public sealed class SmartFridgeBoundUserInterface(EntityUid owner, Enum uiKey) : BoundUserInterface(owner, uiKey)
+public sealed class SmartFridgeBoundUserInterface(EntityUid owner, Enum uiKey) : BoundUserInterface(owner, uiKey), IDisposable
 {
     [ViewVariables]
     private SmartFridgeMenu? _menu;
 
     [ViewVariables]
     private List<SmartFridgeInventoryItem> _cachedInventory = [];
+    
+    public void Dispose()
+    {
+        _menu?.Dispose();
+        _menu = null;
+    }
 }
📝 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 sealed class SmartFridgeBoundUserInterface(EntityUid owner, Enum uiKey) : BoundUserInterface(owner, uiKey)
{
[ViewVariables]
private SmartFridgeMenu? _menu;
[ViewVariables]
private List<SmartFridgeInventoryItem> _cachedInventory = [];
public sealed class SmartFridgeBoundUserInterface(EntityUid owner, Enum uiKey) : BoundUserInterface(owner, uiKey), IDisposable
{
[ViewVariables]
private SmartFridgeMenu? _menu;
[ViewVariables]
private List<SmartFridgeInventoryItem> _cachedInventory = [];
public void Dispose()
{
_menu?.Dispose();
_menu = null;
}

protected override void Open()
{
base.Open();

_menu = this.CreateWindow<SmartFridgeMenu>();
_menu.OpenCenteredLeft();
_menu.Title = EntMan.GetComponent<MetaDataComponent>(Owner).EntityName;
_menu.OnItemSelected += OnItemSelected;
Refresh();
}

public void Refresh()
{
var system = EntMan.System<SmartFridgeSystem>();
_cachedInventory = system.GetInventoryClient(Owner);

_menu?.Populate(_cachedInventory);
}
Comment on lines +29 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Добавьте проверку на null для system

Необходимо добавить проверку на null при получении системы через EntMan.System().

Рекомендуемые изменения:

  public void Refresh()
  {
      var system = EntMan.System<SmartFridgeSystem>();
+     if (system == null)
+         return;
      _cachedInventory = system.GetInventoryClient(Owner);

      _menu?.Populate(_cachedInventory);
  }
📝 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 void Refresh()
{
var system = EntMan.System<SmartFridgeSystem>();
_cachedInventory = system.GetInventoryClient(Owner);
_menu?.Populate(_cachedInventory);
}
public void Refresh()
{
var system = EntMan.System<SmartFridgeSystem>();
if (system == null)
return;
_cachedInventory = system.GetInventoryClient(Owner);
_menu?.Populate(_cachedInventory);
}

Comment on lines +31 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Необходима дополнительная проверка на null

Метод Refresh() может вызвать исключение, если EntMan не инициализирован. Также отсутствует проверка на null для system.

Рекомендуемые изменения:

 public void Refresh()
 {
+    if (EntMan == null)
+        return;
+        
     var system = EntMan.System<SmartFridgeSystem>();
+    if (system == null)
+        return;
+        
     _cachedInventory = system.GetInventoryClient(Owner);
     _menu?.Populate(_cachedInventory);
 }
📝 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
var system = EntMan.System<SmartFridgeSystem>();
_cachedInventory = system.GetInventoryClient(Owner);
_menu?.Populate(_cachedInventory);
}
if (EntMan == null)
return;
var system = EntMan.System<SmartFridgeSystem>();
if (system == null)
return;
_cachedInventory = system.GetInventoryClient(Owner);
_menu?.Populate(_cachedInventory);
}


private void OnItemSelected(GUIBoundKeyEventArgs args, ListData data)
{
if (args.Function != EngineKeyFunctions.UIClick)
return;

if (data is not VendorItemsListData { ItemIndex: var itemIndex })
return;

if (_cachedInventory.Count == 0)
return;

var selectedItem = _cachedInventory.ElementAtOrDefault(itemIndex);

if (selectedItem == null)
return;

SendMessage(new SmartFridgeEjectMessage(selectedItem.StorageSlotId));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using Content.Shared._CorvaxNext.Medical.SmartFridge;
using SmartFridgeComponent = Content.Shared._CorvaxNext.Medical.SmartFridge.SmartFridgeComponent;

namespace Content.Client._CorvaxNext.Medical.SmartFridge;

public sealed class SmartFridgeSystem : SharedSmartFridgeSystem
{
[Dependency] private readonly SharedUserInterfaceSystem _uiSystem = default!;

public override void Initialize()
{
base.Initialize();

SubscribeLocalEvent<SmartFridgeComponent, AfterAutoHandleStateEvent>(OnVendingAfterState);
}

private void OnVendingAfterState(EntityUid uid, SmartFridgeComponent component, ref AfterAutoHandleStateEvent args)
{
if (_uiSystem.TryGetOpenUi<SmartFridgeBoundUserInterface>(uid, SmartFridgeUiKey.Key, out var bui))
{
bui.Refresh();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<controls:FancyWindow
xmlns="https://spacestation14.io"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:controls="clr-namespace:Content.Client.UserInterface.Controls"
xmlns:co="clr-namespace:Content.Client.UserInterface.Controls"
MinSize="350,300"
SetSize="400,400"
Title="{Loc 'smart-fridge-title'}">

<PanelContainer StyleClasses="WindowPanel">
<BoxContainer Name="MainContainer" Orientation="Vertical" Margin="10">
<BoxContainer Orientation="Vertical" Margin="0 0 0 8">
<Label Text="{Loc 'smart-fridge-search'}:" Margin="0 0 0 5" HorizontalAlignment="Center"/>
<LineEdit Name="SearchBar" PlaceHolder="{Loc 'vending-machine-component-search-filter'}" HorizontalExpand="True"/>
</BoxContainer>
Comment on lines +13 to +15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Повторное использование локализационных ключей

В интерфейсе используются ключи локализации от торгового автомата (vending-machine-*). Это может привести к проблемам при обновлении текстов.

Необходимо создать специфичные ключи локализации для холодильника:

-    <LineEdit Name="SearchBar" PlaceHolder="{Loc 'vending-machine-component-search-filter'}" HorizontalExpand="True"/>
+    <LineEdit Name="SearchBar" PlaceHolder="{Loc 'smart-fridge-search-filter'}" HorizontalExpand="True"/>

     <!-- ... -->
     
-    <Label Name="OutOfStockLabel" Text="{Loc 'vending-machine-component-try-eject-out-of-stock'}"
+    <Label Name="OutOfStockLabel" Text="{Loc 'smart-fridge-out-of-stock'}"

Also applies to: 20-26


<PanelContainer StyleClasses="WindowPanelInner" Margin="0 0 0 8" VerticalExpand="True">
<BoxContainer Orientation="Vertical" Name="ContentPanel" Margin="10" VerticalExpand="True">
<co:SearchListContainer Name="VendingContents" VerticalExpand="True"/>
<Label Name="OutOfStockLabel"
Text="{Loc 'vending-machine-component-try-eject-out-of-stock'}"
Margin="4"
HorizontalExpand="True"
HorizontalAlignment="Center"
Visible="False"/>
</BoxContainer>
</PanelContainer>

<PanelContainer StyleClasses="LowDivider" Margin="0 8 0 0"/>
<BoxContainer Orientation="Horizontal" Margin="0 4 0 0" VerticalAlignment="Bottom">
<Label Text="{Loc 'vending-machine-flavor-left'}" StyleClasses="WindowFooterText" Margin="0 0 10 0"/>
<Label Text="{Loc 'vending-machine-flavor-right'}" StyleClasses="WindowFooterText"
HorizontalAlignment="Right" HorizontalExpand="True" Margin="0 0 5 0"/>
<TextureRect StyleClasses="NTLogoDark" Stretch="KeepAspectCentered"
VerticalAlignment="Center" HorizontalAlignment="Right" SetSize="24 24"/>
</BoxContainer>
</BoxContainer>
</PanelContainer>
</controls:FancyWindow>
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
using System.Numerics;
using Content.Client.UserInterface.Controls;
using Content.Client.VendingMachines.UI;
using Content.Shared._CorvaxNext.Medical.SmartFridge;
using Robust.Client.AutoGenerated;
using Robust.Client.UserInterface.Controls;
using Robust.Client.UserInterface.XAML;
using Robust.Client.UserInterface;
using Robust.Shared.Prototypes;

namespace Content.Client._CorvaxNext.Medical.SmartFridge.UI;

[GenerateTypedNameReferences]
public sealed partial class SmartFridgeMenu : FancyWindow
{
[Dependency] private readonly IPrototypeManager _prototypeManager = default!;
[Dependency] private readonly IEntityManager _entityManager = default!;

private readonly Dictionary<EntProtoId, EntityUid> _dummies = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Необходимо освобождение ресурсов для _dummies

Словарь _dummies содержит сущности, но отсутствует их очистка. Это может привести к утечке памяти.

Рекомендуется добавить метод Dispose():

+ public override void Dispose()
+ {
+     base.Dispose();
+     foreach (var dummy in _dummies.Values)
+     {
+         _entityManager.DeleteEntity(dummy);
+     }
+     _dummies.Clear();
+ }

Committable suggestion skipped: line range outside the PR's diff.


public event Action<GUIBoundKeyEventArgs, ListData>? OnItemSelected;

public SmartFridgeMenu()
{
RobustXamlLoader.Load(this);
IoCManager.InjectDependencies(this);

VendingContents.SearchBar = SearchBar;
VendingContents.DataFilterCondition += DataFilterCondition;
VendingContents.GenerateItem += GenerateButton;
VendingContents.ItemKeyBindDown += (args, data) => OnItemSelected?.Invoke(args, data);
}

private static bool DataFilterCondition(string filter, ListData data)
{
if (data is not VendorItemsListData { ItemText: var text })
return false;

return string.IsNullOrEmpty(filter) || text.Contains(filter, StringComparison.CurrentCultureIgnoreCase);
}

private void GenerateButton(ListData data, ListContainerButton button)
{
if (data is not VendorItemsListData { ItemProtoID: var protoId, ItemText: var text })
return;

button.AddChild(new VendingMachineItem(protoId, text));
button.ToolTip = text;
}

public void Populate(List<SmartFridgeInventoryItem> inventory)
{
if (inventory.Count == 0)
{
SearchBar.Visible = false;
VendingContents.Visible = false;
OutOfStockLabel.Visible = true;
return;
}

SearchBar.Visible = true;
VendingContents.Visible = true;
OutOfStockLabel.Visible = false;

var listData = new List<VendorItemsListData>();

for (var i = 0; i < inventory.Count; i++)
{
var entry = inventory[i];

if (!_prototypeManager.TryIndex(entry.Id, out var prototype))
continue;

if (!_dummies.TryGetValue(entry.Id, out var dummy))
{
dummy = _entityManager.Spawn(entry.Id);
_dummies.Add(entry.Id, dummy);
}

var itemText = $"{entry.ItemName} [{entry.Quantity}]";
listData.Add(new VendorItemsListData(prototype.ID, itemText, i));
}
Comment on lines +67 to +82
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Оптимизировать создание списка элементов

Текущая реализация создает новую сущность для каждого прототипа. Это может быть неэффективно при большом количестве элементов.

Предлагается кэшировать результаты:

- if (!_dummies.TryGetValue(entry.Id, out var dummy))
- {
-     dummy = _entityManager.Spawn(entry.Id);
-     _dummies.Add(entry.Id, dummy);
- }
+ var dummy = _dummies.GetOrAdd(entry.Id, id => _entityManager.Spawn(id));
📝 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
for (var i = 0; i < inventory.Count; i++)
{
var entry = inventory[i];
if (!_prototypeManager.TryIndex(entry.Id, out var prototype))
continue;
if (!_dummies.TryGetValue(entry.Id, out var dummy))
{
dummy = _entityManager.Spawn(entry.Id);
_dummies.Add(entry.Id, dummy);
}
var itemText = $"{entry.ItemName} [{entry.Quantity}]";
listData.Add(new VendorItemsListData(prototype.ID, itemText, i));
}
for (var i = 0; i < inventory.Count; i++)
{
var entry = inventory[i];
if (!_prototypeManager.TryIndex(entry.Id, out var prototype))
continue;
var dummy = _dummies.GetOrAdd(entry.Id, id => _entityManager.Spawn(id));
var itemText = $"{entry.ItemName} [{entry.Quantity}]";
listData.Add(new VendorItemsListData(prototype.ID, itemText, i));
}


VendingContents.PopulateList(listData);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
using Content.Shared.DeviceLinking;
using Robust.Shared.Audio;
using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototype;
using Content.Server._CorvaxNext.ExecutionChair;

namespace Content.Server._CorvaxNext.ExecutionChair;

/// <summary>
/// This component represents the state and configuration of an Execution Chair entity.
/// It holds data fields that determine how the chair behaves when it delivers electric shocks
/// to entities buckled into it. It also provides fields for connecting to and receiving signals
/// from the device linking system.
/// </summary>
[RegisterComponent, Access(typeof(ExecutionChairSystem))]
public sealed partial class ExecutionChairComponent : Component
{
/// <summary>
/// The next scheduled time at which this chair can deliver damage to strapped entities.
/// This is used to control the rate of repeated electrocution ticks.
/// </summary>
[ViewVariables]
public TimeSpan NextDamageTick = TimeSpan.Zero;

/// <summary>
/// Indicates whether the chair is currently enabled. If true, and all conditions (powered, anchored, etc.)
/// are met, the chair will deliver electrical damage to any buckled entities at regular intervals.
/// </summary>
[DataField, AutoNetworkedField]
public bool Enabled = false;
Comment on lines +28 to +29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Добавить событие изменения состояния

При изменении состояния Enabled не генерируется событие, что может привести к рассинхронизации состояния.

Рекомендуется добавить событие:

+ public event Action<bool>? OnEnabledChanged;
- public bool Enabled = false;
+ private bool _enabled = false;
+ public bool Enabled
+ {
+     get => _enabled;
+     set
+     {
+         if (_enabled == value) return;
+         _enabled = value;
+         OnEnabledChanged?.Invoke(_enabled);
+     }
+ }
📝 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
[DataField, AutoNetworkedField]
public bool Enabled = false;
[DataField, AutoNetworkedField]
public event Action<bool>? OnEnabledChanged;
private bool _enabled = false;
public bool Enabled
{
get => _enabled;
set
{
if (_enabled == value) return;
_enabled = value;
OnEnabledChanged?.Invoke(_enabled);
}
}


/// <summary>
/// Determines whether the chair should play a sound when entities are shocked. If set to true,
/// a sound from <see cref="ShockNoises"/> will be played each time damage is dealt.
/// </summary>
[DataField]
public bool PlaySoundOnShock = true;

/// <summary>
/// Specifies which sound collection is played when entities are shocked. By default, uses a collection of
/// "sparks" sounds. This allows multiple random sparks audio clips to be played.
/// </summary>
[DataField]
public SoundSpecifier ShockNoises = new SoundCollectionSpecifier("sparks");

/// <summary>
/// Controls how loud the shock sound is. This value is applied to the base volume of the chosen sound
/// when played.
/// </summary>
[DataField]
public float ShockVolume = 20;

/// <summary>
/// The amount of damage delivered to a buckled entity each damage tick while the chair is active.
/// </summary>
[DataField]
public int DamagePerTick = 25;
Comment on lines +55 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Добавить проверку урона

Фиксированное значение урона может быть опасным. Необходимо добавить проверки и возможность настройки через конфигурацию.

Предлагается изменить реализацию:

- public int DamagePerTick = 25;
+ [DataField("damagePerTick", required: true)]
+ public int DamagePerTick { get; set; }
+ 
+ [DataField("maxDamagePerTick")]
+ public int MaxDamagePerTick = 50;
📝 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
[DataField]
public int DamagePerTick = 25;
[DataField("damagePerTick", required: true)]
public int DamagePerTick { get; set; }
[DataField("maxDamagePerTick")]
public int MaxDamagePerTick = 50;


/// <summary>
/// The duration in seconds for which the electrocution effect is applied each time damage is dealt.
/// For example, if set to 4, it electrocutes an entity for 4 seconds.
/// </summary>
[DataField]
public int DamageTime = 4;

/// <summary>
/// The name of the device link port used to toggle the chair's state. Receiving a signal on this port
/// switches the enabled state from on to off or from off to on.
/// </summary>
[DataField]
public string TogglePort = "Toggle";

/// <summary>
/// The name of the device link port used to force the chair's state to enabled (on).
/// Receiving a signal here ensures the chair is active.
/// </summary>
[DataField]
public string OnPort = "On";

/// <summary>
/// The name of the device link port used to force the chair's state to disabled (off).
/// Receiving a signal here ensures the chair is inactive.
/// </summary>
[DataField]
public string OffPort = "Off";
}
Loading
Loading