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

[PORT] Book printer #651

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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
74 changes: 74 additions & 0 deletions Content.Client/_Cats/BookPrinter/BookPrinterBoundUserInterface.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Оригинал данного файла был сделан @temporaldarkness (discord). Код был взят с https://github.com/ss14-ganimed/ENT14-Master.

using Content.Shared._Cats.BookPrinter;
using Content.Shared.Containers.ItemSlots;
using JetBrains.Annotations;

namespace Content.Client._Cats.BookPrinter
{
[UsedImplicitly]
public sealed class BookPrinterBoundUserInterface : BoundUserInterface
{
[ViewVariables]
private BookPrinterWindow? _window;

[ViewVariables]
private BookPrinterBoundUserInterfaceState? _lastState;

public BookPrinterBoundUserInterface(EntityUid owner, Enum uiKey) : base(owner, uiKey)
{
}

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

_window = new()
{
Title = EntMan.GetComponent<MetaDataComponent>(Owner).EntityName,
};

_window.EjectButton.OnPressed += _ => SendMessage(new ItemSlotButtonPressedEvent("bookSlot"));
_window.ClearButton.OnPressed += _ => SendMessage(new BookPrinterClearContainerMessage());
_window.UploadButton.OnPressed += _ => SendMessage(new BookPrinterUploadMessage());
_window.CopyPasteButton.OnPressed += _ => SendMessage(new BookPrinterCopyPasteMessage());


_window.OpenCentered();
_window.OnClose += Close;

_window.OnPrintBookButtonPressed += (args, button) => SendMessage(new BookPrinterPrintBookMessage(button.BookEntry));
_window.OnPrintBookButtonMouseEntered += (args, button) =>
{
if (_lastState is not null)
_window.UpdateContainerInfo(_lastState);
};
_window.OnPrintBookButtonMouseExited += (args, button) =>
{
if (_lastState is not null)
_window.UpdateContainerInfo(_lastState);
};
}

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

var castState = (BookPrinterBoundUserInterfaceState)state;
_lastState = castState;

_window?.UpdateState(castState);
}
Comment on lines +57 to +61
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Проверьте возможность возникновения исключения при приведении типа

В методе UpdateState() вы приводите state к BookPrinterBoundUserInterfaceState без проверки. Если state не является этим типом, это приведет к исключению InvalidCastException.

Добавьте проверку типа перед приведением или используйте оператор as с последующей проверкой на null.

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

-   var castState = (BookPrinterBoundUserInterfaceState)state;
+   if (state is not BookPrinterBoundUserInterfaceState castState)
+       return;

    _lastState = castState;

    _window?.UpdateState(castState);
}
📝 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 castState = (BookPrinterBoundUserInterfaceState)state;
_lastState = castState;
_window?.UpdateState(castState);
}
protected override void UpdateState(BoundUserInterfaceState state)
{
base.UpdateState(state);
if (state is not BookPrinterBoundUserInterfaceState castState)
return;
_lastState = castState;
_window?.UpdateState(castState);
}


[Obsolete]
protected override void Dispose(bool disposing)
Comment on lines +63 to +64
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Неуместный атрибут [Obsolete] на методе Dispose

Метод Dispose переопределяет базовый метод и не должен помечаться атрибутом [Obsolete], если нет веской причины. Это может вызвать предупреждения или недоразумения у других разработчиков.

Предлагаемое изменение:

-    [Obsolete]
     protected override void Dispose(bool disposing)
     {
         base.Dispose(disposing);

         if (disposing)
         {
             _window?.Dispose();
         }
     }
📝 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
[Obsolete]
protected override void Dispose(bool disposing)
protected override void Dispose(bool disposing)

{
base.Dispose(disposing);

if (disposing)
{
_window?.Dispose();
}
}
Comment on lines +63 to +72
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Устаревший метод Dispose(bool disposing)

Использование атрибута [Obsolete] на методе Dispose может привести к предупреждениям при компиляции и не рекомендуется для переопределений методов базового класса.

Удалите атрибут [Obsolete] или обдумайте необходимость переопределения метода Dispose.

-[Obsolete]
protected override void Dispose(bool disposing)
{
    base.Dispose(disposing);

    if (disposing)
    {
        _window?.Dispose();
    }
}
📝 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
[Obsolete]
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
if (disposing)
{
_window?.Dispose();
}
}
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
if (disposing)
{
_window?.Dispose();
}
}

}
}
47 changes: 47 additions & 0 deletions Content.Client/_Cats/BookPrinter/BookPrinterWindow.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<!-- Оригинал данного файла был сделан @temporaldarkness (discord). Код был взят с https://github.com/ss14-ganimed/ENT14-Master. -->

<DefaultWindow
xmlns="https://spacestation14.io"
xmlns:gfx="clr-namespace:Robust.Client.Graphics;assembly=Robust.Client"
Title="{Loc 'book-printer-title'}"
SetSize="525 475"
MinSize="525 475">
<BoxContainer Orientation="Vertical">
<ScrollContainer HScrollEnabled="False" HorizontalExpand="True" VerticalExpand="True" MinSize="0 250">
<BoxContainer Orientation="Vertical" Name="BooksList" Access="Public">
</BoxContainer>
</ScrollContainer>
Comment on lines +10 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Необходимо добавить индикацию состояния списка

Для улучшения пользовательского опыта рекомендуется:

  • Добавить индикатор загрузки
  • Добавить текст для пустого состояния списка
  • Добавить визуальную индикацию возможности прокрутки
 <ScrollContainer HScrollEnabled="False" HorizontalExpand="True" VerticalExpand="True" MinSize="0 250">
-    <BoxContainer Orientation="Vertical" Name="BooksList" Access="Public">
+    <BoxContainer Orientation="Vertical" Name="BooksList" Access="Public" Margin="5">
+        <Label Name="EmptyStateLabel" 
+               Text="{Loc 'book-printer-no-books'}"
+               HorizontalAlignment="Center"
+               VerticalAlignment="Center"/>
     </BoxContainer>
 </ScrollContainer>

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

<Control MinSize="0 10"/>
<BoxContainer Orientation="Horizontal">
<Label Text="{Loc 'book-printer-window-book-label'}"/>
<Control MinSize="10 0"/>
<Button Name="UploadButton"
Access="Public"
Text="{Loc 'book-printer-window-upload-button'}"
StyleClasses="OpenRight"/>
<Button Name="ClearButton"
Access="Public"
Text="{Loc 'book-printer-window-clear-button'}"
StyleClasses="OpenBoth"/>
<Button Name="CopyPasteButton"
Access="Public"
Text="{Loc 'book-printer-window-copy-button'}"
StyleClasses="OpenBoth"/>
<Button Name="EjectButton"
Access="Public"
Text="{Loc 'book-printer-window-eject-button'}"
StyleClasses="OpenLeft"/>
</BoxContainer>
Comment on lines +15 to +34
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Требуются улучшения доступности элементов управления

Рекомендуемые улучшения:

  • Добавить всплывающие подсказки (tooltips) для кнопок
  • Добавить горячие клавиши
  • Сгруппировать связанные действия визуально
 <BoxContainer Orientation="Horizontal">
     <Label Text="{Loc 'book-printer-window-book-label'}"/>
     <Control MinSize="10 0"/>
     <Button Name="UploadButton"
             Access="Public"
             Text="{Loc 'book-printer-window-upload-button'}"
+            ToolTip="{Loc 'book-printer-window-upload-tooltip'}"
+            HotKeyText="Ctrl+U"
             StyleClasses="OpenRight"/>
     <Button Name="ClearButton"
             Access="Public"
             Text="{Loc 'book-printer-window-clear-button'}"
+            ToolTip="{Loc 'book-printer-window-clear-tooltip'}"
+            HotKeyText="Delete"
             StyleClasses="OpenBoth"/>
     <Button Name="CopyPasteButton"
             Access="Public"
             Text="{Loc 'book-printer-window-copy-button'}"
+            ToolTip="{Loc 'book-printer-window-copy-tooltip'}"
+            HotKeyText="Ctrl+C"
             StyleClasses="OpenBoth"/>
     <Button Name="EjectButton"
             Access="Public"
             Text="{Loc 'book-printer-window-eject-button'}"
+            ToolTip="{Loc 'book-printer-window-eject-tooltip'}"
+            HotKeyText="Ctrl+E"
             StyleClasses="OpenLeft"/>
 </BoxContainer>

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

<Control MinSize="0 10"/>
<PanelContainer SizeFlagsStretchRatio="6" MinSize="0 100">
<PanelContainer.PanelOverride>
<gfx:StyleBoxFlat BackgroundColor="#1b1b1e" />
</PanelContainer.PanelOverride>
<BoxContainer Name="ContainerInfo"
Orientation="Vertical"
HorizontalExpand="True">
<Label Text="{Loc 'book-printer-window-no-book-loaded-text'}"/>
</BoxContainer>
</PanelContainer>
Comment on lines +36 to +45
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Необходимо улучшить стилизацию информационной панели

Текущая реализация имеет следующие проблемы:

  • Жестко закодированный цвет фона
  • Отсутствует визуальная иерархия
  • Нет обработки динамического контента
 <PanelContainer SizeFlagsStretchRatio="6" MinSize="0 100">
     <PanelContainer.PanelOverride>
-        <gfx:StyleBoxFlat BackgroundColor="#1b1b1e" />
+        <gfx:StyleBoxFlat BackgroundColor="{ThemeResource BackgroundDark}" />
     </PanelContainer.PanelOverride>
     <BoxContainer Name="ContainerInfo"
                   Orientation="Vertical"
-                  HorizontalExpand="True">
+                  HorizontalExpand="True"
+                  Margin="10">
-        <Label Text="{Loc 'book-printer-window-no-book-loaded-text'}"/>
+        <Label Name="InfoTitle"
+               StyleClasses="LabelHeading"
+               Text="{Loc 'book-printer-window-info-title'}"/>
+        <Control MinSize="0 5"/>
+        <Label Name="InfoContent"
+               Text="{Loc 'book-printer-window-no-book-loaded-text'}"
+               StyleClasses="LabelSubText"/>
     </BoxContainer>
 </PanelContainer>
📝 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
<PanelContainer SizeFlagsStretchRatio="6" MinSize="0 100">
<PanelContainer.PanelOverride>
<gfx:StyleBoxFlat BackgroundColor="#1b1b1e" />
</PanelContainer.PanelOverride>
<BoxContainer Name="ContainerInfo"
Orientation="Vertical"
HorizontalExpand="True">
<Label Text="{Loc 'book-printer-window-no-book-loaded-text'}"/>
</BoxContainer>
</PanelContainer>
<PanelContainer SizeFlagsStretchRatio="6" MinSize="0 100">
<PanelContainer.PanelOverride>
<gfx:StyleBoxFlat BackgroundColor="{ThemeResource BackgroundDark}" />
</PanelContainer.PanelOverride>
<BoxContainer Name="ContainerInfo"
Orientation="Vertical"
HorizontalExpand="True"
Margin="10">
<Label Name="InfoTitle"
StyleClasses="LabelHeading"
Text="{Loc 'book-printer-window-info-title'}"/>
<Control MinSize="0 5"/>
<Label Name="InfoContent"
Text="{Loc 'book-printer-window-no-book-loaded-text'}"
StyleClasses="LabelSubText"/>
</BoxContainer>
</PanelContainer>

</BoxContainer>
</DefaultWindow>
163 changes: 163 additions & 0 deletions Content.Client/_Cats/BookPrinter/BookTerminalWindow.xaml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
// Оригинал данного файла был сделан @temporaldarkness (discord). Код был взят с https://github.com/ss14-ganimed/ENT14-Master.

using System.Linq;
using System.Numerics;
using Content.Client.Stylesheets;
using Content.Shared._Cats.BookPrinter;
using Robust.Client.AutoGenerated;
using Robust.Client.UserInterface;
using Robust.Client.UserInterface.Controls;
using Robust.Client.UserInterface.CustomControls;
using Robust.Client.UserInterface.XAML;
using Robust.Shared.Prototypes;
using static Robust.Client.UserInterface.Controls.BoxContainer;

namespace Content.Client._Cats.BookPrinter
{
[GenerateTypedNameReferences]
public sealed partial class BookPrinterWindow : DefaultWindow
{
[Dependency] private readonly IPrototypeManager _prototypeManager = default!;
[Dependency] private readonly IEntityManager _entMan = default!;
public event Action<BaseButton.ButtonEventArgs, PrintBookButton>? OnPrintBookButtonPressed;
public event Action<GUIMouseHoverEventArgs, PrintBookButton>? OnPrintBookButtonMouseEntered;
public event Action<GUIMouseHoverEventArgs, PrintBookButton>? OnPrintBookButtonMouseExited;


public BookPrinterWindow()
{
RobustXamlLoader.Load(this);
IoCManager.InjectDependencies(this);
}

public void UpdateState(BoundUserInterfaceState state)
{
var castState = (BookPrinterBoundUserInterfaceState)state;
UpdateContainerInfo(castState);
UpdateBooksList(castState);

CopyPasteButton.Text = Loc.GetString(castState.CopyPaste
? "book-printer-window-paste-button"
: "book-printer-window-copy-button");

UploadButton.Disabled = !castState.RoutineAllowed || castState.WorkProgress is not null;
ClearButton.Disabled = !castState.RoutineAllowed || castState.WorkProgress is not null;
EjectButton.Disabled = castState.BookName is null || castState.WorkProgress is not null;
CopyPasteButton.Disabled = !castState.RoutineAllowed || castState.WorkProgress is not null;
}

public void UpdateBooksList(BoundUserInterfaceState state)
{
var castState = (BookPrinterBoundUserInterfaceState)state;

if (BooksList == null)
return;

BooksList.Children.Clear();

if (castState.BookEntries is null)
return;


foreach (var entry in castState.BookEntries.OrderBy(r => r.Name))
{
var button = new PrintBookButton(entry, CutDescription(entry.Name ?? ""));
button.OnPressed += args => OnPrintBookButtonPressed?.Invoke(args, button);
button.OnMouseEntered += args => OnPrintBookButtonMouseEntered?.Invoke(args, button);
button.OnMouseExited += args => OnPrintBookButtonMouseExited?.Invoke(args, button);
button.Disabled = !castState.RoutineAllowed || castState.WorkProgress is not null;
BooksList.AddChild(button);
}
}

public string CutDescription(string? text)
{
if (text is null)
return "";

if (text.Length <= 47)
return text;

return text.Substring(0, 47) + "...";
}

Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Формула расчёта прогресса некорректна

Текущая формула (int)(100.0f * (1 - state.WorkProgress)) инвертирует значение прогресса. Если WorkProgress представляет собой долю выполненной работы от 0 до 1, то формула показывает оставшийся процент работы вместо выполненного. Это может вводить пользователей в заблуждение, так как обычно прогресс-бары показывают процент выполнения, а не оставшееся время.

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

  • Заменить формулу на (int)(100.0f * state.WorkProgress) для отображения процента выполненной работы
  • Или добавить поясняющий комментарий, если текущее поведение намеренное
🔗 Analysis chain

Убедитесь в правильности вычисления прогресса работы

При расчёте прогресса используется выражение (int)(100.0f * (1 - state.WorkProgress)). Проверьте, соответствует ли это ожидаемому поведению: отображению оставшегося или выполненного процента работы.

Запустите следующий скрипт для проверки значений state.WorkProgress в кодовой базе:


Let me gather more context about the progress calculation and its usage.


Let me check the state handling in the terminal window implementation.


Let me check the state definition to understand how WorkProgress is defined.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Описание: Поиск использования state.WorkProgress

# Ожидается: Найти все упоминания state.WorkProgress для анализа контекста
rg 'state\.WorkProgress'

Length of output: 361


Script:

#!/bin/bash
# Search for the WorkProgress property definition and related code
ast-grep --pattern 'WorkProgress'

# Search for localization string definition
rg "book-printer-window-working"

Length of output: 394


Script:

#!/bin/bash
# Get the full content of the terminal window implementation
cat "Content.Client/_Cats/BookPrinter/BookTerminalWindow.xaml.cs"

# Search for any state-related classes or interfaces
rg -g "*.cs" "interface.*State" -A 5

Length of output: 8796


Script:

#!/bin/bash
# Search for BookPrinterBoundUserInterfaceState definition
rg -g "*.cs" "class.*BookPrinterBoundUserInterfaceState" -A 10

Length of output: 1091

public void UpdateContainerInfo(BookPrinterBoundUserInterfaceState state)
{
ContainerInfo.Children.Clear();

if (state.WorkProgress is not null)
{
ContainerInfo.Children.Add(new Label { Text = Loc.GetString("book-printer-window-working", ("progress", (int)(100.0f * (1 - state.WorkProgress)))) });
return;
}

if (state.BookName is null)
{
ContainerInfo.Children.Add(new Label { Text = Loc.GetString("book-printer-window-no-book-loaded-text") });
}
else
{

Comment on lines +99 to +100
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Добавьте обработку случая, когда книга не найдена

Если _entMan.TryGetEntity(state.BookEntity, out var bookEntity) возвращает false, то bookEntity будет неопределён, и вызов bookPreview.SetEntity(bookEntity) может привести к ошибке. Рекомендуется добавить обработку этого случая.

Примените следующий дифф:

 if (_entMan.TryGetEntity(state.BookEntity, out var bookEntity))
     bookPreview.SetEntity(bookEntity);
+else
+    // Добавьте обработку ситуации, когда сущность книги не найдена

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

var bookPreview = new SpriteView
{
Scale = new Vector2(2, 2),
OverrideDirection = Direction.South,
VerticalAlignment = VAlignment.Center,
SizeFlagsStretchRatio = 1
};
if (_entMan.TryGetEntity(state.BookEntity, out var bookEntity))
bookPreview.SetEntity(bookEntity);

var bookLabel = new Label
{
Text = $"{CutDescription(state.BookName)}"
};

var bookSublabel = new Label
{
Text = $"{CutDescription(state.BookDescription)}",
StyleClasses = { StyleNano.StyleClassLabelSecondaryColor }
};

var boxInfo = new BoxContainer
{
Orientation = LayoutOrientation.Vertical,
Children = {
new Control { MinSize = new Vector2(0, 10) },
bookLabel,
bookSublabel }
};

ContainerInfo.Children.Add(new BoxContainer
{
Orientation = LayoutOrientation.Horizontal,
Children = { bookPreview, boxInfo }
});

}

if (state.CartridgeCharge is null)
{
ContainerInfo.Children.Add(new Label { Text = Loc.GetString("book-printer-window-no-cartridge-loaded-text"), FontColorOverride = Color.DarkRed });
return;
}
else if (state.CartridgeCharge <= -10.0f)
{
ContainerInfo.Children.Add(new Label { Text = Loc.GetString("book-printer-window-cartridge-empty"), FontColorOverride = Color.DarkRed });
return;
}
ContainerInfo.Children.Add(new Label { Text = Loc.GetString("book-printer-window-cartridge-charge", ("charge", (int)(100 * state.CartridgeCharge))) });
}
}

public sealed class PrintBookButton : Button
{
public SharedBookPrinterEntry BookEntry { get; }

public PrintBookButton(SharedBookPrinterEntry bookEntry, string text)
{
BookEntry = bookEntry;
Text = text;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Оригинал данного файла был сделан @temporaldarkness (discord). Код был взят с https://github.com/ss14-ganimed/ENT14-Master.

using Content.Shared._Cats.BookPrinter.Components;
using Content.Shared.Containers.ItemSlots;
using Robust.Client.GameObjects;

namespace Content.Client._Cats.BookPrinter.Visualizers;

public sealed partial class BookPrinterVisualizerSystem : VisualizerSystem<BookPrinterVisualsComponent>
{
[Dependency] private readonly ItemSlotsSystem _itemSlotsSystem = default!;

protected override void OnAppearanceChange(EntityUid uid, BookPrinterVisualsComponent component, ref AppearanceChangeEvent args)
{
if (args.Sprite == null || !EntityManager.TryGetComponent<ItemSlotsComponent>(uid, out var slotComp))
return;

if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.Working, out var workLayer))
{
args.Sprite.LayerSetVisible(workLayer, component.DoWorkAnimation);
}

if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.Slotted, out var slotLayer))
{
args.Sprite.LayerSetVisible(slotLayer, (_itemSlotsSystem.GetItemOrNull(uid, "cartridgeSlot") is not null));
}

var cartridge = _itemSlotsSystem.GetItemOrNull(uid, "cartridgeSlot");

if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.Full, out var fullLayer))
{
args.Sprite.LayerSetVisible(fullLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(fullLayer!, cartridgeComp.CurrentCharge == cartridgeComp.FullCharge);
}

if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.High, out var highLayer))
{
args.Sprite.LayerSetVisible(highLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(highLayer, cartridgeComp.CurrentCharge >= cartridgeComp.FullCharge / 1.43f && cartridgeComp.CurrentCharge < cartridgeComp.FullCharge);
}

if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.Medium, out var mediumLayer))
{
args.Sprite.LayerSetVisible(mediumLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(mediumLayer, cartridgeComp.CurrentCharge >= cartridgeComp.FullCharge / 2.5f && cartridgeComp.CurrentCharge < cartridgeComp.FullCharge / 1.43f);
}

if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.Low, out var lowLayer))
{
args.Sprite.LayerSetVisible(lowLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(lowLayer, cartridgeComp.CurrentCharge > 0 && cartridgeComp.CurrentCharge < cartridgeComp.FullCharge / 2.5f);
}

if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.None, out var noneLayer))
{
args.Sprite.LayerSetVisible(noneLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(noneLayer, cartridgeComp.CurrentCharge < 1);
}
Comment on lines +30 to +63
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Необходима оптимизация кода визуализации уровней заряда

Текущая реализация имеет несколько проблем:

  1. Дублирование кода при проверке компонентов и установке видимости слоёв
  2. Использование магических чисел для порогов заряда (1.43f, 2.5f)
  3. Повторное получение компонента картриджа

Предлагаю следующую оптимизацию:

+ private const float HighChargeThreshold = 0.7f;  // 1/1.43
+ private const float MediumChargeThreshold = 0.4f;  // 1/2.5

+ private void UpdateChargeLayer(SpriteComponent sprite, BookPrinterVisualLayers layer, 
+     EntityUid? cartridge, float minCharge, float maxCharge = 1.0f)
+ {
+     if (!sprite.LayerMapTryGet(layer, out var spriteLayer))
+         return;
+         
+     sprite.LayerSetVisible(spriteLayer, false);
+     if (cartridge is not null && 
+         EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out var comp))
+     {
+         var chargePercent = comp.CurrentCharge / comp.FullCharge;
+         sprite.LayerSetVisible(spriteLayer, 
+             chargePercent >= minCharge && chargePercent < maxCharge);
+     }
+ }

- // Существующий код с проверками слоёв
+ var cartridge = _itemSlotsSystem.GetItemOrNull(uid, CartridgeSlotId);
+ UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.Full, cartridge, 1.0f);
+ UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.High, cartridge, HighChargeThreshold, 1.0f);
+ UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.Medium, cartridge, MediumChargeThreshold, HighChargeThreshold);
+ UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.Low, cartridge, 0.01f, MediumChargeThreshold);
+ UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.None, cartridge, 0f, 0.01f);
📝 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
if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.Full, out var fullLayer))
{
args.Sprite.LayerSetVisible(fullLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(fullLayer!, cartridgeComp.CurrentCharge == cartridgeComp.FullCharge);
}
if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.High, out var highLayer))
{
args.Sprite.LayerSetVisible(highLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(highLayer, cartridgeComp.CurrentCharge >= cartridgeComp.FullCharge / 1.43f && cartridgeComp.CurrentCharge < cartridgeComp.FullCharge);
}
if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.Medium, out var mediumLayer))
{
args.Sprite.LayerSetVisible(mediumLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(mediumLayer, cartridgeComp.CurrentCharge >= cartridgeComp.FullCharge / 2.5f && cartridgeComp.CurrentCharge < cartridgeComp.FullCharge / 1.43f);
}
if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.Low, out var lowLayer))
{
args.Sprite.LayerSetVisible(lowLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(lowLayer, cartridgeComp.CurrentCharge > 0 && cartridgeComp.CurrentCharge < cartridgeComp.FullCharge / 2.5f);
}
if (args.Sprite.LayerMapTryGet(BookPrinterVisualLayers.None, out var noneLayer))
{
args.Sprite.LayerSetVisible(noneLayer, false);
if (cartridge is not null && EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out BookPrinterCartridgeComponent? cartridgeComp))
args.Sprite.LayerSetVisible(noneLayer, cartridgeComp.CurrentCharge < 1);
}
private const float HighChargeThreshold = 0.7f; // 1/1.43
private const float MediumChargeThreshold = 0.4f; // 1/2.5
private void UpdateChargeLayer(SpriteComponent sprite, BookPrinterVisualLayers layer,
EntityUid? cartridge, float minCharge, float maxCharge = 1.0f)
{
if (!sprite.LayerMapTryGet(layer, out var spriteLayer))
return;
sprite.LayerSetVisible(spriteLayer, false);
if (cartridge is not null &&
EntityManager.TryGetComponent<BookPrinterCartridgeComponent>(cartridge, out var comp))
{
var chargePercent = comp.CurrentCharge / comp.FullCharge;
sprite.LayerSetVisible(spriteLayer,
chargePercent >= minCharge && chargePercent < maxCharge);
}
}
var cartridge = _itemSlotsSystem.GetItemOrNull(uid, CartridgeSlotId);
UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.Full, cartridge, 1.0f);
UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.High, cartridge, HighChargeThreshold, 1.0f);
UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.Medium, cartridge, MediumChargeThreshold, HighChargeThreshold);
UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.Low, cartridge, 0.01f, MediumChargeThreshold);
UpdateChargeLayer(args.Sprite, BookPrinterVisualLayers.None, cartridge, 0f, 0.01f);

}
}
Loading
Loading