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 #986

Closed
wants to merge 7 commits into from
Closed

Ports #986

Show file tree
Hide file tree
Changes from 6 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
24 changes: 24 additions & 0 deletions Content.Client/TapeRecorder/TapeRecorderSystem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using Content.Shared.TapeRecorder;

namespace Content.Client.TapeRecorder;

/// <summary>
/// Required for client side prediction stuff
/// </summary>
public sealed class TapeRecorderSystem : SharedTapeRecorderSystem
{
private TimeSpan _lastTickTime = TimeSpan.Zero;

public override void Update(float frameTime)
{
if (!Timing.IsFirstTimePredicted)
return;

//We need to know the exact time period that has passed since the last update to ensure the tape position is sync'd with the server
//Since the client can skip frames when lagging, we cannot use frameTime
var realTime = (float) (Timing.CurTime - _lastTickTime).TotalSeconds;
_lastTickTime = Timing.CurTime;

base.Update(realTime);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using Content.Shared.TapeRecorder.Components;
using Content.Shared.TapeRecorder.Events;
using Robust.Shared.Prototypes;
using Robust.Shared.Timing;

namespace Content.Client.TapeRecorder.Ui;

public sealed class TapeRecorderBoundUserInterface(EntityUid owner, Enum uiKey) : BoundUserInterface(owner, uiKey)
{
[Dependency] private readonly IEntityManager _entMan = default!;

[ViewVariables]
private TapeRecorderWindow? _window;

[ViewVariables]
private TimeSpan _printCooldown;

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

_window = new(_entMan, Owner);
_window.OnClose += Close;
_window.OnModeChanged += ChangeMode;
_window.OnPrintTranscript += PrintTranscript;
_window.OpenCentered();
}

private void ChangeMode(TapeRecorderMode mode)
{
SendMessage(new ChangeModeTapeRecorderMessage(mode));
}

private void PrintTranscript()
{
SendMessage(new PrintTapeRecorderMessage());

_window?.UpdatePrint(true);

Timer.Spawn(_printCooldown, () =>
{
_window?.UpdatePrint(false);
});
}
Comment on lines +40 to +44
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

Потенциальная утечка памяти в Timer.Spawn

Использование Timer.Spawn может привести к утечке памяти, если окно будет закрыто до завершения таймера. Рекомендуется добавить проверку на disposed состояние.

-        Timer.Spawn(_printCooldown, () =>
-        {
-            _window?.UpdatePrint(false);
-        });
+        var handle = Timer.Spawn(_printCooldown, () =>
+        {
+            if (!Disposed)
+                _window?.UpdatePrint(false);
+        });
+        _window?.OnClose += () => Timer.Cancel(handle);
📝 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
Timer.Spawn(_printCooldown, () =>
{
_window?.UpdatePrint(false);
});
}
var handle = Timer.Spawn(_printCooldown, () =>
{
if (!Disposed)
_window?.UpdatePrint(false);
});
_window?.OnClose += () => Timer.Cancel(handle);
}

Comment on lines +34 to +44
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

Улучшить обработку ошибок в PrintTranscript

Метод PrintTranscript не проверяет успешность отправки сообщения и не обрабатывает возможные исключения.

     private void PrintTranscript()
     {
-        SendMessage(new PrintTapeRecorderMessage());
+        try 
+        {
+            if (_window == null)
+                return;
+                
+            SendMessage(new PrintTapeRecorderMessage());
+            _window.UpdatePrint(true);
 
-        _window?.UpdatePrint(true);
-
-        Timer.Spawn(_printCooldown, () =>
-        {
-            _window?.UpdatePrint(false);
-        });
+            var handle = Timer.Spawn(_printCooldown, () =>
+            {
+                if (!Disposed && _window != null)
+                    _window.UpdatePrint(false);
+            });
+            _window.OnClose += () => Timer.Cancel(handle);
+        }
+        catch (Exception e)
+        {
+            Logger.ErrorS("tape-recorder", e, "Ошибка при печати транскрипции");
+        }
📝 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
private void PrintTranscript()
{
SendMessage(new PrintTapeRecorderMessage());
_window?.UpdatePrint(true);
Timer.Spawn(_printCooldown, () =>
{
_window?.UpdatePrint(false);
});
}
private void PrintTranscript()
{
try
{
if (_window == null)
return;
SendMessage(new PrintTapeRecorderMessage());
_window.UpdatePrint(true);
var handle = Timer.Spawn(_printCooldown, () =>
{
if (!Disposed && _window != null)
_window.UpdatePrint(false);
});
_window.OnClose += () => Timer.Cancel(handle);
}
catch (Exception e)
{
Logger.ErrorS("tape-recorder", e, "Ошибка при печати транскрипции");
}
}


protected override void UpdateState(BoundUserInterfaceState state)
{
base.UpdateState(state);

if (state is not TapeRecorderState cast)
return;

_printCooldown = cast.PrintCooldown;

_window?.UpdateState(cast);
}

protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
if (disposing)
_window?.Dispose();
}
}
23 changes: 23 additions & 0 deletions Content.Client/TapeRecorder/Ui/TapeRecorderWindow.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<controls:FancyWindow
xmlns="https://spacestation14.io"
xmlns:controls="clr-namespace:Content.Client.UserInterface.Controls"
MinSize="440 220"
SetSize="440 220"
Title="{Loc 'tape-recorder-menu-title'}"
Resizable="False">
<BoxContainer Margin = "10 5" Orientation="Vertical" SeparationOverride="5">
<BoxContainer Orientation="Vertical">
<Label Margin = "5 0" Name="CassetteLabel" Text="{Loc 'tape-recorder-menu-no-cassette-label'}" Align="Left" StyleClasses="StatusFieldTitle" />
<Slider Name="PlaybackSlider" HorizontalExpand="True" />
</BoxContainer>
<BoxContainer Name ="Test" Margin = "0 5 0 0" Orientation="Horizontal" VerticalExpand = "True">
<BoxContainer Orientation="Vertical" HorizontalExpand = "True">
<Label Text="{Loc 'tape-recorder-menu-controls-label'}" Align="Center" />
<BoxContainer Name="Buttons" Orientation="Horizontal" VerticalExpand="True" Align="Center"/> <!-- Populated in constructor -->
</BoxContainer>
</BoxContainer>
<BoxContainer Margin = "0 2 0 0" Orientation="Horizontal">
<Button Name="PrintButton" Text="{Loc 'tape-recorder-menu-print-button'}" TextAlign="Center" HorizontalExpand ="True"/>
</BoxContainer>
</BoxContainer>
</controls:FancyWindow>
133 changes: 133 additions & 0 deletions Content.Client/TapeRecorder/Ui/TapeRecorderWindow.xaml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
using Content.Client.UserInterface.Controls;
using Content.Shared.Shuttles.Components;
using Content.Shared.TapeRecorder.Components;
using Content.Shared.TapeRecorder.Events;
using Robust.Client.AutoGenerated;
using Robust.Client.UserInterface.Controls;
using Robust.Client.UserInterface.XAML;
using Robust.Shared.Timing;

namespace Content.Client.TapeRecorder.Ui;

[GenerateTypedNameReferences]
public sealed partial class TapeRecorderWindow : FancyWindow
{
private IEntityManager _entMan;

private EntityUid _owner;
private bool _onCooldown;
private bool _hasCasette;
private TapeRecorderMode _mode = TapeRecorderMode.Stopped;

private RadioOptions<TapeRecorderMode> _options = default!;
private bool _updating;

public Action<TapeRecorderMode>? OnModeChanged;
public Action? OnPrintTranscript;

public TapeRecorderWindow(IEntityManager entMan, EntityUid owner)
{
RobustXamlLoader.Load(this);
IoCManager.InjectDependencies(this);

_entMan = entMan;

_owner = owner;

_options = new RadioOptions<TapeRecorderMode>(RadioOptionsLayout.Horizontal);
Buttons.AddChild(_options);
_options.FirstButtonStyle = "OpenRight";
_options.LastButtonStyle = "OpenLeft";
_options.ButtonStyle = "OpenBoth";
foreach (var mode in Enum.GetValues<TapeRecorderMode>())
{
var name = mode.ToString().ToLower();
_options.AddItem(Loc.GetString($"tape-recorder-menu-{name}-button"), mode);
}

_options.OnItemSelected += args =>
{
if (_updating) // don't tell server to change mode to the mode it told us
return;
args.Button.Select(args.Id);
var mode = args.Button.SelectedValue;
OnModeChanged?.Invoke(mode);
};

PrintButton.OnPressed += _ => OnPrintTranscript?.Invoke();

SetEnabled(TapeRecorderMode.Recording, false);
SetEnabled(TapeRecorderMode.Playing, false);
SetEnabled(TapeRecorderMode.Rewinding, false);
}

private void SetSlider(float maxTime, float currentTime)
{
PlaybackSlider.Disabled = true;
PlaybackSlider.MaxValue = maxTime;
PlaybackSlider.Value = currentTime;
}

public void UpdatePrint(bool disabled)
{
PrintButton.Disabled = disabled;
_onCooldown = disabled;
}

public void UpdateState(TapeRecorderState state)
{
if (!_entMan.TryGetComponent<TapeRecorderComponent>(_owner, out var comp))
return;

_mode = comp.Mode; // TODO: update UI on handling state instead of adding UpdateUI to everything
_hasCasette = state.HasCasette;

_updating = true;

CassetteLabel.Text = _hasCasette
? Loc.GetString("tape-recorder-menu-cassette-label", ("cassetteName", state.CassetteName))
: Loc.GetString("tape-recorder-menu-no-cassette-label");

// Select the currently used mode
_options.SelectByValue(_mode);

// When tape is ejected or a button can't be used, disable it
// Server will change to paused once a tape is inactive
var tapeLeft = state.CurrentTime < state.MaxTime;
SetEnabled(TapeRecorderMode.Recording, tapeLeft);
SetEnabled(TapeRecorderMode.Playing, tapeLeft);
SetEnabled(TapeRecorderMode.Rewinding, state.CurrentTime > float.Epsilon);

if (state.HasCasette)
SetSlider(state.MaxTime, state.CurrentTime);

_updating = false;
}

private void SetEnabled(TapeRecorderMode mode, bool condition)
{
_options.SetItemDisabled((int) mode, !(_hasCasette && condition));
}

protected override void FrameUpdate(FrameEventArgs args)
{
base.FrameUpdate(args);

if (!_entMan.HasComponent<ActiveTapeRecorderComponent>(_owner))
return;

if (!_entMan.TryGetComponent<TapeRecorderComponent>(_owner, out var comp))
return;

if (_mode != comp.Mode)
{
_mode = comp.Mode;
_options.SelectByValue(_mode);
}

var speed = _mode == TapeRecorderMode.Rewinding
? -comp.RewindSpeed
: 1f;
PlaybackSlider.Value += args.DeltaSeconds * speed;
}
Comment on lines +107 to +132
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

Моделирование проигрывания в режиме Rewinding
При перемотке установка «PlaybackSlider.Value += args.DeltaSeconds * speed» со знаком «-» корректна. Убедитесь, что граничные условия (Value не уходит в отрицательные значения) тоже учтены при длительных перемотках.

-PlaybackSlider.Value += args.DeltaSeconds * speed;
+var newValue = PlaybackSlider.Value + args.DeltaSeconds * speed;
+PlaybackSlider.Value = MathF.Max(0f, newValue);
📝 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
private void SetEnabled(TapeRecorderMode mode, bool condition)
{
_options.SetItemDisabled((int) mode, !(_hasCasette && condition));
}
protected override void FrameUpdate(FrameEventArgs args)
{
base.FrameUpdate(args);
if (!_entMan.HasComponent<ActiveTapeRecorderComponent>(_owner))
return;
if (!_entMan.TryGetComponent<TapeRecorderComponent>(_owner, out var comp))
return;
if (_mode != comp.Mode)
{
_mode = comp.Mode;
_options.SelectByValue(_mode);
}
var speed = _mode == TapeRecorderMode.Rewinding
? -comp.RewindSpeed
: 1f;
PlaybackSlider.Value += args.DeltaSeconds * speed;
}
private void SetEnabled(TapeRecorderMode mode, bool condition)
{
_options.SetItemDisabled((int) mode, !(_hasCasette && condition));
}
protected override void FrameUpdate(FrameEventArgs args)
{
base.FrameUpdate(args);
if (!_entMan.HasComponent<ActiveTapeRecorderComponent>(_owner))
return;
if (!_entMan.TryGetComponent<TapeRecorderComponent>(_owner, out var comp))
return;
if (_mode != comp.Mode)
{
_mode = comp.Mode;
_options.SelectByValue(_mode);
}
var speed = _mode == TapeRecorderMode.Rewinding
? -comp.RewindSpeed
: 1f;
var newValue = PlaybackSlider.Value + args.DeltaSeconds * speed;
PlaybackSlider.Value = MathF.Max(0f, newValue);
}

}
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 = [];

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

_menu = this.CreateWindow<SmartFridgeMenu>();
_menu.OpenCenteredLeft();
_menu.Title = EntMan.GetComponent<MetaDataComponent>(Owner).EntityName;
_menu.OnItemSelected += OnItemSelected;
Comment on lines +24 to +25
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

Отсутствует обработка исключений при получении MetaDataComponent

GetComponent может выбросить исключение, если компонент отсутствует.

-        _menu.Title = EntMan.GetComponent<MetaDataComponent>(Owner).EntityName;
+        if (EntMan.TryGetComponent<MetaDataComponent>(Owner, out var meta))
+            _menu.Title = meta.EntityName;
+        else
+            _menu.Title = "Smart Fridge";
📝 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
_menu.Title = EntMan.GetComponent<MetaDataComponent>(Owner).EntityName;
_menu.OnItemSelected += OnItemSelected;
if (EntMan.TryGetComponent<MetaDataComponent>(Owner, out var meta))
_menu.Title = meta.EntityName;
else
_menu.Title = "Smart Fridge";
_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

Потенциальная проблема синхронизации в методе Refresh

Метод Refresh не защищен от одновременного доступа к _cachedInventory.

+    private readonly object _inventoryLock = new();
+
     public void Refresh()
     {
         var system = EntMan.System<SmartFridgeSystem>();
-        _cachedInventory = system.GetInventoryClient(Owner);
-
-        _menu?.Populate(_cachedInventory);
+        List<SmartFridgeInventoryItem> inventory;
+        lock (_inventoryLock)
+        {
+            inventory = system.GetInventoryClient(Owner);
+            _cachedInventory = inventory;
+        }
+        _menu?.Populate(inventory);
     }
📝 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);
}
private readonly object _inventoryLock = new();
public void Refresh()
{
var system = EntMan.System<SmartFridgeSystem>();
List<SmartFridgeInventoryItem> inventory;
lock (_inventoryLock)
{
inventory = system.GetInventoryClient(Owner);
_cachedInventory = inventory;
}
_menu?.Populate(inventory);
}


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>

<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>
Loading
Loading