Всем привет. Меня зовут Гриша Дядиченко, и я технический продюсер. Почему так сложно писать про хороший код? Меня периодически спрашивают, почему я так мало пишу про архитектуру. В то же время я даже среди заказчиков встречаю мнение что “в Unity пишется только плохой код”. Чтож, давайте один раз попробуем, а точнее я попробую показать, почему это очень сложно. Разработаем вместе такую “простую вещь” как инвентарь.
Итак, но начнём с неких общих слов. Есть такой замечательный эффект, как эффект Даннинга — Крюгера. Его суть заключается в том, что люди с низкой квалификацией всегда уверены, что они правы. А люди с высокой квалификацией наоборот же считают, что они в чём-то не до конца разбираются, могут придумать миллиард нюансов и где то, что они говорят не совсем так. И в этом плане архитектурные вопросы – это бездна. Так что давайте попробуем разработать наш идеальный инвентарь.
Что такое инвентарь?
Давайте рассуждать пока абстрактно, но помня примеры инвентарей. По сути это хранилище предметов. Предметы бывают разные, так что давайте сразу заведём абстрактный класс BaseItem, который будет выглядеть как-то так.
using System;
[Serializable]
public class BaseItem
{
public long ID;
public string VisualName;
public string IconUrl;
}Максимально абстрактный предмет в инвентаре по сути обладает идентификатором (для оптимизации возьмём не string, а long), путём до его иконки в инвентаре и именем. Да?
Конечно же нет. Переписываем класс, так как имени и иконки тут не должно быть. Почему? Потому что имя как сущность в игре вообще не существует, а существует у нас Alias локализации, да и иконка не относится к предмету, если мы хотим уметь играть в игру в консоли. Поэтому оставляем максимально абстрактный BaseItem.
using System;
[Serializable]
public class BaseItem
{
public long ID;
}Хорошо. Предмет у нас есть. Теперь давайте создадим инвентарь. Представим его как просто набором предметов.
public class Inventory
{
private BaseItem[] _items;
public Inventory(int size)
{
_items = new BaseItem[size];
}
}И вот мы создали инвентарь, теперь надо бы научиться добавлять в него предметы и вытаскивать их из него. Так как это у нас поведение, то давайте сразу без лишних прелюдий заведём интерфейс IInventoryActions.
using System.Collections.Generic;
public interface IInventoryActions
{
bool Add(int cellID, BaseItem item);
bool Remove(int cellID);
bool IsCellHasItem(int cellID);
BaseItem GetItem(int cellID);
IEnumerable<BaseItem> GetItems();
}Для получения всех предметов используем IEnumerable так как мало ли мы захотим в инвентаре наш массив заменить на словарь или на что-то ещё. Наша реализация теперь.
using System.Collections.Generic;
public class Inventory : IInventoryActions
{
private BaseItem[] _items;
public bool Add(int cellID, BaseItem item)
{
_items[cellID] = item;
return true;
}
public bool Remove(int cellID)
{
_items[cellID] = null;
return true;
}
public BaseItem GetItem(int cellID)
{
return _items[cellID];
}
public IEnumerable<BaseItem> GetItems()
{
return _items;
}
public int GetSize()
{
return _items.Length;
}
public bool IsCellHasItem(int cellID)
{
return _items[cellID] != null;
}
public Inventory(int size)
{
_items = new BaseItem[size];
}
}Add и Remove сразу выдают bool для методов валидации "а добавили ли мы предмет я в ячейку". Поверьте, это пригодится потом. И это не проверяется через критерий занятости ячейки. Совсем параноики конечно могут определить условие IsCellHasItem внутри этих методов. Но эти булы будут говорить не об этом. Итак, у нас есть что-то похожее на инвентарь. Наверное, чтобы удобнее его тестировать нам нужен к нему какой-то графический интерфейс. Пока забудем о том, что вы написали свой "самый удобный фреймворк для GUI внутри Unity" и напишем всё довольно просто и конкретно.
Визуал инвентаря
using System.Collections.Generic;
using UnityEngine;
public class UIInventoryWindow : MonoBehaviour
{
[SerializeField] private UIInventoryCell _cellTemplate;
[SerializeField] private RectTransform _cellsRoot;
private List<UIInventoryCell> _cells;
private IInventoryActions _inventory = new Inventory(20);
private void Awake()
{
_cells = new List<UIInventoryCell>();
for (int i = 0; i < _inventory.GetSize(); i++)
{
var cell = Instantiate(_cellTemplate, _cellsRoot);
_cells.Add(cell);
}
}
private void OnEnable()
{
int i = 0;
foreach (var baseItem in _inventory.GetItems())
{
_cells[i].Init(baseItem);
i++;
}
}
}
И ячейка выглядит как:
using System;
using UnityEngine;
using UnityEngine.EventSystems;
public class UIInventoryCell : MonoBehaviour, IPointerClickHandler
{
private BaseItem _item;
public event Action<BaseItem> OnClickCell;
public void Init(BaseItem item)
{
_item = item;
}
public void OnPointerClick(PointerEventData eventData)
{
OnClickCell?.Invoke(_item);
}
}
Так сказать первые монобехи. До этого играть можно было в консоли. Для теста мы создали инвентарь прям в окне, но вообще для него нужно какое-то хранилище. Например пользователь. Так что давайте заведём нашего пользователя. Он нам ещё пригодится.
public class User
{
private static User _Current;
public static User Current
{
get
{
if (_Current == null)
{
_Current = new User();
}
return _Current;
}
}
public Inventory Inventory;
private User()
{
Inventory = new Inventory(20);
}
}В рпг и сингл плеерных играх пользователя удобно делать синглтоном. Потому что все остальные юзеры, кроме текущего, на мой взгляд это Actors или вроде того. А пользователь у нас всегда один. Если у нас игра без hotseat и т.п. Но я последнее время предпочитаю не писать часть с приватным конструктором, а просто иметь статик доступ через Current, чтобы в случае необходимости прокидывать мок юзера. Вообще "как написать грамотно юзера" — этого на ещё одну статью хватит. Ну и окно инвентаря теперь выглядит вот так.
using System.Collections.Generic;
using UnityEngine;
public class UIInventoryWindow : MonoBehaviour
{
[SerializeField] private UIInventoryCell _cellTemplate;
[SerializeField] private RectTransform _cellsRoot;
private List<UIInventoryCell> _cells;
private IInventoryActions _inventory;
private void Awake()
{
_inventory = User.Current.Inventory;
_cells = new List<UIInventoryCell>();
for (int i = 0; i < _inventory.GetSize(); i++)
{
var cell = Instantiate(_cellTemplate, _cellsRoot);
_cells.Add(cell);
}
}
private void OnEnable()
{
int i = 0;
foreach (var baseItem in _inventory.GetItems())
{
_cells[i].Init(baseItem);
i++;
}
}
}
Хранилище
Едем дальше. По хорошему предметам нужны иконки, которые мы будем отображать в ячейках. Заведём данные нескольких игровых предметов и сторажд с ними.
Начнём с поведения:
public interface IStorageActions<T>
{
T GetData(long id);
}По сути мы только получаем данные из стораджа, при том отдельными объектами. Дальше напишем данные предмета.
using System;
using UnityEngine;
[Serializable]
public class ItemVisualData
{
[SerializeField] private long _id;
[SerializeField] private Sprite _icon;
[SerializeField] private string _visualName;
public long ID => _id;
public Sprite Icon => _icon;
public string VisualName => _visualName;
}Сразу оговорюсь, что пусть и хранилище мы ща пишем ридонли по всем канонам, я считаю паранойей делать такую защиту от дурака. Но тем не менее это по сути статическое хранилище данных, которые в рантайме мы менять не должны, так что пусть будет иммутабельно в пределах базовых типов.
А теперь в само хранилище:
using System.Collections.Generic;
using UnityEngine;
public class ItemsVisualDataStorage : IStorageActions<ItemVisualData>
{
private const string DataPath = "ItemsVisualDataCollection";
private Dictionary<long, ItemVisualData> _data;
public void Load()
{
var data = Resources.Load<ItemVisualDataSOCollection>(DataPath).data;
_data = new Dictionary<long, ItemVisualData>();
foreach (var itemVisualData in data)
{
if (!_data.ContainsKey(itemVisualData.ID))
{
_data.Add(itemVisualData.ID, itemVisualData);
}
}
}
public ItemVisualData GetData(long id)
{
return _data[id];
}
public ItemsVisualDataStorage()
{
Load();
}
}
Его уже не хочется делать статик контейнером в самом себе, поэтому сделаем так:
public class Storages
{
public static IStorageActions<ItemVisualData> ItemsVisualDataStorege = new ItemsVisualDataStorage();
}Ну и для загрузки данных для удобства пока используем SO без плясок с кастомной отрисовкой словарей в редакторе и их сериализацией, то есть без защиты от дурака. Хотя там тоже есть что написать.
using System.Collections.Generic;
using UnityEngine;
[CreateAssetMenu]
public class ItemVisualDataSOCollection : ScriptableObject
{
[SerializeField] private List<ItemVisualData> _data;
public List<ItemVisualData> data => _data;
}Теперь наш SO с предметами выглядит как-то так:
Добавив пока для теста в конструктор пользователя несколько предметов, мы увидим, что всё работает:
public User()
{
Inventory = new Inventory(20);
Inventory.Add(0, new BaseItem()
{
ID = 0
});
Inventory.Add(1, new BaseItem()
{
ID = 1
});
Inventory.Add(2, new BaseItem()
{
ID = 2
});
}
Небольшая ремарка. Почему инвентарь != storage. Ведь по сути инвентарь можно было бы воспринимать, как некую форму Storage. Я не очень люблю совсем общие обобщения всего подряд. Так как Storage в моём понимании рид-онли в рантайме. И так в разы проще отслеживать баги, да и править, когда сущности разделены.
Действия с инвентарём
Теперь предметы хотелось бы допустим перекладывать и удалять. Для иллюстрации концепта, удалять мы будем через клик, а перемещать через драг. И оставим пока наши тест предметы.
Для начала сделаем удаление. Чуть изменим наши классы графического интерфейса.
using System.Collections.Generic;
using UnityEngine;
public class UIInventoryWindow : MonoBehaviour
{
[SerializeField] private UIInventoryCell _cellTemplate;
[SerializeField] private RectTransform _cellsRoot;
private List<UIInventoryCell> _cells;
private IInventoryActions _inventory;
private void Awake()
{
_inventory = User.Current.Inventory;
_cells = new List<UIInventoryCell>();
for (int i = 0; i < _inventory.GetSize(); i++)
{
var cell = Instantiate(_cellTemplate, _cellsRoot);
cell.Init(i);
_cells.Add(cell);
}
}
private void OnEnable()
{
int i = 0;
foreach (var baseItem in _inventory.GetItems())
{
_cells[i].SetItem(baseItem);
i++;
}
}
}using System;
using UnityEngine;
using UnityEngine.EventSystems;
using UnityEngine.UI;
public class UIInventoryCell : MonoBehaviour, IPointerClickHandler
{
[SerializeField] private Image _icon;
private BaseItem _item;
private int _cellID;
public void Init(int cellID)
{
_cellID = cellID;
SetItem(null);
}
public void SetItem(BaseItem item)
{
_item = item;
if (item != null)
{
_icon.enabled = true;
_icon.sprite = Storages.ItemsVisualDataStorege.GetData(_item.ID).Icon;
}
else
{
_icon.enabled = false;
}
}
public void OnPointerClick(PointerEventData eventData)
{
User.Current.Inventory.Remove(_cellID);
SetItem(null);
}
}И удаление работает. С драгом же чуть посложнее. Давайте заведём DragContext.
using System;
public class DragContext<T>
{
private T _currentDraggable;
private IDroppable<T> _container;
public event Action<T> OnStartDrag;
public event Action<T> OnDrag;
public event Action<T> OnEndDrag;
public void StartDrag(T draggable)
{
_currentDraggable = draggable;
OnStartDrag?.Invoke(_currentDraggable);
}
public void EndDrag()
{
OnEndDrag?.Invoke(_currentDraggable);
if (_container != null)
{
_container.OnDrop(_currentDraggable);
}
_currentDraggable = default(T);
}
public void ProcessDrag()
{
OnDrag?.Invoke(_currentDraggable);
}
public void EnterContainer(IDroppable<T> container)
{
_container = container;
}
public void ExitContainer(IDroppable<T> container)
{
if (_container == container)
{
_container = null;
}
}
}
public interface IDroppable<T>
{
void OnDrop(T item);
}Зачем нужен контекст? В идеале с Drag&Drop удобно когда есть некий контекст драг энд дропа, чтобы нельзя было, да и не надо было обрабатывать, что если у нас есть две панели с драг энд дропом и ошибки, так как мы кидаем из одной в другую. Потому что они не взаимодействуют между друг другом благодаря разным контекстам.
Теперь же напишем контейнер для нашего контекста:
using System;
using UnityEngine;
public class UIInventoryDragContainer : MonoBehaviour
{
private static DragContext<Tuple<UIInventoryCell, BaseItem>> _context;
public static DragContext<Tuple<UIInventoryCell, BaseItem>> Context => _context;
[SerializeField] private RectTransform _dragContainer;
private GameObject _visualDraggableObject;
private Camera _camera;
private void Awake()
{
_camera = Camera.main;
_context = new DragContext<Tuple<UIInventoryCell, BaseItem>>();
_context.OnStartDrag += ContextStartDrag;
_context.OnEndDrag += ContextOnEndDrag;
_context.OnDrag += ContextOnDrag;
}
private void ContextOnDrag(Tuple<UIInventoryCell, BaseItem> data)
{
if (_visualDraggableObject != null)
{
_visualDraggableObject.transform.position = Input.mousePosition;
}
}
private void ContextStartDrag(Tuple<UIInventoryCell, BaseItem> data)
{
_visualDraggableObject = Instantiate(data.Item1.gameObject, _dragContainer);
}
private void ContextOnEndDrag(Tuple<UIInventoryCell, BaseItem> data)
{
Destroy(_visualDraggableObject);
}
}Создавать кнопку по внешнем контейнере с огромным Z-index пока мне показалось из всех способов реализации драга удобнее всего. Тут же мы заводим нужный нам контекст, который будет реализовывать наш переброс предметов внутри инвентаря. И осталось сделать реализацию в самой ячейке инвентаря.
using System;
using UnityEngine;
using UnityEngine.EventSystems;
using UnityEngine.UI;
public class UIInventoryCell : MonoBehaviour, IPointerClickHandler, IBeginDragHandler, IEndDragHandler, IDragHandler,
IPointerEnterHandler, IPointerExitHandler, IDroppable<Tuple<UIInventoryCell, BaseItem>>
{
[SerializeField] private Image _icon;
private BaseItem _item;
private int _cellID;
public void Init(int cellID)
{
_cellID = cellID;
SetItem(null);
}
public void SetItem(BaseItem item)
{
_item = item;
if (item != null)
{
_icon.enabled = true;
_icon.sprite = Storages.ItemsVisualDataStorage.GetData(_item.ID).Icon;
}
else
{
_icon.enabled = false;
}
}
public void OnPointerClick(PointerEventData eventData)
{
User.Current.Inventory.Remove(_cellID);
SetItem(null);
}
public void OnDrop(Tuple<UIInventoryCell, BaseItem> data)
{
if (!User.Current.Inventory.IsCellHasItem(_cellID))
{
if (this != data.Item1)
{
User.Current.Inventory.Remove(data.Item1._cellID);
data.Item1.SetItem(null);
}
User.Current.Inventory.Add(_cellID, data.Item2);
SetItem(data.Item2);
}
}
public void OnBeginDrag(PointerEventData eventData)
{
if(_item == null) return;
UIInventoryDragContainer.Context.StartDrag(new Tuple<UIInventoryCell, BaseItem>(this, _item));
}
public void OnDrag(PointerEventData eventData)
{
UIInventoryDragContainer.Context.ProcessDrag();
}
public void OnEndDrag(PointerEventData eventData)
{
UIInventoryDragContainer.Context.EndDrag();
}
public void OnPointerEnter(PointerEventData eventData)
{
UIInventoryDragContainer.Context.EnterContainer(this);
}
public void OnPointerExit(PointerEventData eventData)
{
UIInventoryDragContainer.Context.ExitContainer(this);
}
}
И вот спустя некоторое время Drag & Drop работает в нашем инвентаре.
Сохранение состояния
Чтож инвентарь есть, мы можем двигать в нём предметы. Теперь сохраним состояние инвентаря между запусками, как финальный штрих.
Для этого создадим динамический Storage и начнём как всегда с интерфейса:
public interface IDynamicStorageActions<T>
{
void Save(T data);
T Load();
}Теперь реализуем наш конкретный Storage:
using Newtonsoft.Json;
using UnityEngine;
public class UserDataJsonStorage : IDynamicStorageActions<User>
{
private const string PrefsUserKey = "CURRENT_USER";
public void Save(User user)
{
PlayerPrefs.SetString(PrefsUserKey, JsonConvert.SerializeObject(user));
PlayerPrefs.Save();
}
public User Load()
{
if (PlayerPrefs.HasKey(PrefsUserKey))
{
return JsonConvert.DeserializeObject<User>(PlayerPrefs.GetString(PrefsUserKey));
}
else
{
return null;
}
}
}И прокинем его в пользователя. Плюс создадим событие на изменение инвентаря, чтобы сохранятся скажем при каждом изменении:
using System;
using Newtonsoft.Json;
[Serializable]
public class User
{
private static User _Current;
public static User Current
{
get
{
if (_Current == null)
{
if (!Load())
{
CreateUser();
}
}
return _Current;
}
}
[JsonProperty] private Inventory _inventory;
public Inventory Inventory => _inventory;
private static bool Load()
{
var data = Storages.Dynamic.UserStorage.Load();
if (data == null)
{
return false;
}
else
{
_Current = data;
_Current._inventory.OnChange += Save;
return true;
}
}
public static void Save()
{
Storages.Dynamic.UserStorage.Save(_Current);
}
public static void CreateUser()
{
_Current = new User();
_Current._inventory = new Inventory(20);
_Current._inventory.Add(0, new BaseItem()
{
ID = 0
});
_Current._inventory.Add(1, new BaseItem()
{
ID = 1
});
_Current._inventory.Add(2, new BaseItem()
{
ID = 2
});
Save();
_Current._inventory.OnChange += Save;
}
}public class Storages
{
public static IStorageActions<ItemVisualData> ItemsVisualDataStorage = new ItemsVisualDataStorage();
public class Dynamic
{
public static IDynamicStorageActions<User> UserStorage = new UserDataJsonStorage();
}
}
using System;
using System.Collections.Generic;
using Newtonsoft.Json;
[Serializable]
public class Inventory : IInventoryActions
{
[JsonProperty] private BaseItem[] _items;
public event Action OnChange;
public bool Add(int cellID, BaseItem item)
{
_items[cellID] = item;
OnChange?.Invoke();
return true;
}
public bool Remove(int cellID)
{
_items[cellID] = null;
OnChange?.Invoke();
return true;
}
public BaseItem GetItem(int cellID)
{
return _items[cellID];
}
public IEnumerable<BaseItem> GetItems()
{
return _items;
}
public int GetSize()
{
return _items.Length;
}
public bool IsCellHasItem(int cellID)
{
return _items[cellID] != null;
}
public Inventory(int size)
{
_items = new BaseItem[size];
}
}Теперь наш инвентарь умеет сохранять состояние предметов пользователя, запоминая что и куда мы положили. Мок инициализацию я пока оставил чисто для демонстрации, чтобы не делать полноценный мок. Ну вроде базовый инвентарь готов, его можно расширять и делать что-то. А теперь наконец-то поговорим про код. Весь репозиторий можно посмотреть тут.
Хороший ли это код?
Ну я считаю довольно неплохой, но тут есть множество вопросов. Всё классно, а что если у нас асинхронный сторадж (скажем сохранения по сети?) и почему не написать всё через Task сразу? А что есть у нас WebGL и Task там адекватно не работает и нужно юзать Uni Task? А что если у нас инвентарь имеет форму, а предметы размер как в некоторых RPG. Тогда для правильной синхронизации графический интерфейс должен отражать эту форму, а модель данных должна валидировать это всё (и придётся очень многое переписывать).
Так же тут заложен потенциальный риск бага, который возможно не все сразу заметили, что при подобном подходе к отрисовке и изменению состояния ячеек, есть риск того, что состояние на экране не будет соответствовать состоянию модели данных. Так как интерфейс перерисовывается по действиям, а не реактивно, как я больше люблю. То есть когда мы изменили "стейт" — перерисуй всё.
Сторадж для визуальных данных предмета. Зачем его делать отдельно? Ведь можно все данные о предмете собрать в один SO и там удобно будет отредактировать. А можно сделать один SO и распихивать данные по куче стораджей из него, при этом гуй не поменяется, но зато провайдеры данных будут разделены. И скажем иконки могут весить много и уехать на CDN, а может нам нужна кор механика, а поменять только названия и иконки.
И это только я сам к себе так могу придраться. Поэтому очень тяжело писать про хороший код, так как у каждого своё понятие хорошего кода, исходя из опыта. Есть как бы норм код, и не норм. А остальное вкусы и вопрос контекста задачи. Я считаю, что не бывает универсальных решений, если это не супер мелкая задача.
Спасибо за внимание, надеюсь эта статья была вам полезна и вы узнали из неё что-то новое. Полный проект с инвентарём можно найти тут.
Комментарии (21)
ralyeh
18.11.2022 21:45+11У меня статья вызывает больше недопонимания. Оговорюсь сразу, я очень далек от Unity.
Не существует никакого "идеального" инвентаря. Любая разработка начинается с определения набора требований. Как я должен понять требования к инвентарю по описанию эффекта Даннинга-Крюгера и некой непонятой картинке инвентаря? Что означает каждая секция инвентаря? Почему у ячейки инвентаря нет количества предметов?
Почему абстрактный класс BaseItem без ключевого слово abstract? Зачем вообще строить какую-то невнятную иерархию наследования без обозначения, кто его наследники? Какая вообще проблема решается через наследование? Почему просто не создать структуру только для чтения под названием InventoryCell (BaseItem - слишком абстрактное название)?
Почему ID у BaseItem задан как long? Я вижу сходу две явные реализации ID: как Enum (когда заранее известны все возможные предметы) и как еще одна структура только для чтения (когда все виды предметов загружаются динамически из ресурсов, когда надо валидировать ID и чтобы не дать возможность ID присвоить, скажем, CellID).
Почему количество ячеек в инвентаря определяется динамически, если на картинке нет полос прокруток?
Почему у Inventory ровно один интерфейс с очень странным названием IInventoryActions? Почему не просто IInventory? Зачем этот интерфейс вообще нужен, если в классе User он не используется? Почему IInventory не наследуется хотя бы от IEnumerable? Почему не используется индексатор для доступа к ячейке инвентаря? Зачем Add и Remove возвращают bool? Я должен поверить на слово? Это прямое нарушение YAGNI и принципа единственной ответственности из SOLID.
UIInventoryWindow. С какой целью абсолютно все поля во всех наследниках MonoBehaviour помечены атрибутом SerializeField, чтобы было, вдруг пригодится? Почему вообще вызывается Instantiate для ячейки, неужели нельзя обойтись без этого? Почему инвентарь не попадает в UIInventoryWindow через инверсию зависимости?
Почему UIInventoryCell содержит OnClickCell? Где вообще было сказано, что по ячейкам кликают мышкой? Почему OnClickCell не часть BaseItem? Почему у события неправильное название? OnCellClicked - это название для обработчика события, а не для события. Зачем вообще нужно это событие, какая проблема решается? У такой реализации событий очень много проблем, что проще забыть вообще про ключевое слово event. Одна из проблем прямо наглядно видна в методе OnPointerClick с ненужной проверкой на null.
Почему у класса User нет интерфейса? Почему приватное поле названо с большой буквы? Почему User знает как создавать инвентарь и самого себя? Почему не используется современный C# (или вовсе избавиться от приватного поля):
public static User Current { get => _current ?? new User(); }И что с того, что в играх на одного человека всего один игрок? Как этот код поддерживать? Как написать тесты на этот код? Про существование шаблона одиночка лучше вообще забыть.
ID чего принимает GetData в интерфейсе IStorageActions? Зачем здесь вообще обобщение используется?
В классе ItemVisualData (плохое название, класс описывает информацию о предмете) можно повесить атрибут SerializeField на свойство. Зачем вообще нужен этот атрибут? Для сериализации он не нужен. Почему VisualName - это строка, а не ID ресурса локализации? Статическое и только для чтения - это разные слова с разным смыслом, т.е. "статическое хранилище" - вообще не означает "только для чтения".
Зачем в классе ItemsVisualDataStorage метод Load? Не проще ли перенести содержимое метода в конструктор? И не надо в словаре вызывать метод ContainsKey, достаточно использовать индексатор.
ScriptableObject - это очень ужасное дизайнерское решение разработчиков Unity, что-то сопоставимое с шаблоном одиночка. Не надо в коде использовать сокращения, такие как SO. Почему эти данные не берутся из ресурсов?
Для тестов надо писать тесты, а не портить класс User.
Storage и Inventory. Раз приходится пояснять, почему это не одно и тоже, значит в коде что-то не так. У меня прослеживается логика, что Inventory хранит ячейки инвентаря, а Storage хранит описания предметов. Т.е. Storage - изначально неправильное название сущности + класс занимается загрузкой данных, чем нарушает принцип единственной ответственности из SOLID и отсутствие инверсии зависимостей очень сильно усложняет тестирование такого кода. Почему это не репозиторий, который возвращает обычный массив?
Что такое null в классе UIInventoryCell? Есть же хотя бы шаблон Null Object, который упростит код в методе SetItem. То, как достается иконка в методе, тихий ужас.
DragContext. Как быть, если инвентарь пользователя взаимодействует с инвентарем ящика?
Не надо вызывать Instantiate в классе UIInventoryDragContainer. Зачем использовать Tuple, когда в Unity поддерживается C# с нормальными именованными кортежами, а не вот это вот Item1.
Зачем вводить IDynamicStorageActions, когда уже был обобщенный класс для хранения?
Не надо было в класс User добавлять в свойство вызов метода. Следовало в таком случае отказаться от свойства вообще. И не надо смешивать доменную модель с способом хранения: атрибут JsonProperty не к месту и нарушает принцип единственной ответственности в SOLID. Не надо делать такую жесть как сохранения пользователя на изменение инвентаря. Или возвращение false, когда ничего не удалось загрузить. Никто не вызовет метод Load, если загружать нечего.
Хороший ли это код? Ну не знаю, на мой взгляд код ужасен. Как-то ожидаешь лучшего от Master of Unity3D, а не статью на непонятном языке: "монобехи", "SO", ридонли, иммутабельно, стейт и так далее. И не решена изначальная проблема на самой первой картинке с особым размещением ячеек по нескольким сеткам.
DyadichenkoGA Автор
18.11.2022 21:48-3Эта проблема и не решалась. Хотя в рамках текущей архитектуры решается базово имплементацией второго инвентаря. Но вот про это в статье я и говорю :)
Писать про архитектуру дело неблагодарное. Слово abstact допустим. А что это меняет с точки зрения архитектуры?) Синтаксическое ограничение того, что класс абстрактный в контексте примера?)Половина списка - это вкусовщина, которая никак не влияет не на расширение, ни на эксплуатацию реального решения. Про SRP вообще в целом не правда, так как атрибут не добавляет дополнительной ответственности классу. Особенно такой, которая создавала бы проблемы. Вот поэтому я и не пишу про хороший код, так как это просто неблагодарный труд. Так как придут люди без продуктового обоснования почему, а просто со вкусовыми предпочтениями :)
DyadichenkoGA Автор
18.11.2022 22:24Но давайте я по отвечаю
Не существует никакого "идеального" инвентаря. Любая разработка начинается с определения набора требований. Как я должен понять требования к инвентарю по описанию эффекта Даннинга-Крюгера и некой непонятой картинке инвентаря? Что означает каждая секция инвентаря? Почему у ячейки инвентаря нет количества предметов?
Потому что я не решаю задачу с картинки и не заявлял этого темой статьи
Почему абстрактный класс BaseItem без ключевого слово abstract? Зачем вообще строить какую-то невнятную иерархию наследования без обозначения, кто его наследники? Какая вообще проблема решается через наследование? Почему просто не создать структуру только для чтения под названием InventoryCell (BaseItem - слишком абстрактное название)?
Стандартный подход в геймдеве. Да это может быть класс контейнер в дальнейшем компонентном подходе. Это классика, это при разработке 5-6 взрослых игр знать надо
Почему количество ячеек в инвентаря определяется динамически, если на картинке нет полос прокруток?
Потому что это абстрактный инвентарь. Когда мы пишем по MVVM в целом странно такие вещи определять на каком-то уровне кроме View. Зачем ограничивать размер инвентаря в изначальной архитектуре системы? Для какой цели?
Почему у Inventory ровно один интерфейс с очень странным названием IInventoryActions? Почему не просто IInventory? Зачем этот интерфейс вообще нужен, если в классе User он не используется? Почему IInventory не наследуется хотя бы от IEnumerable? Почему не используется индексатор для доступа к ячейке инвентаря? Зачем Add и Remove возвращают bool? Я должен поверить на слово? Это прямое нарушение YAGNI и принципа единственной ответственности из SOLID.
Название - мой кодстайл. Это не нарушает единственность ответственности. Это нарушает что каждая фигня должна быть реализована, с чем я изначально в солиде не совсем согласен. А изначально это заложено для асинхронных операций, чтобы при доступе на сервер или куда-то ещё при ошибке авторизации или сетевых ошибках знать, что что-то пошло не так. Но я бы написал TryAdd, а не просто Add, это было бы вернее
Почему UIInventoryCell содержит OnClickCell? Где вообще было сказано, что по ячейкам кликают мышкой? Почему OnClickCell не часть BaseItem? Почему у события неправильное название? OnCellClicked - это название для обработчика события, а не для события. Зачем вообще нужно это событие, какая проблема решается? У такой реализации событий очень много проблем, что проще забыть вообще про ключевое слово event. Одна из проблем прямо наглядно видна в методе OnPointerClick с ненужной проверкой на null.
Это потом выпиливается. Но если брать до того, чтобы ячейка не знала про окно, но окно рулило кросс взаймодействием как контекст. Потом я решил контекст завести отдельно
Почему у класса User нет интерфейса? Почему приватное поле названо с большой буквы? Почему User знает как создавать инвентарь и самого себя? Почему не используется современный C# (или вовсе избавиться от приватного поля):
Опечатка, не относится к архитектуре. Сахар, тоже не относится к архитектуре и мелочь. Рихтер в целом считает сахар злом. Вкусовщина
И что с того, что в играх на одного человека всего один игрок? Как этот код поддерживать? Как написать тесты на этот код? Про существование шаблона одиночка лучше вообще забыть.
Прикольное заблуждение. Проекты зарабатывающие миллионы долларов, которые не тонут в техдолге при наличии одиночки как-то существуют. Я в целом не согласен с критикой. Но тут ошибка концептуальная, так как у меня не одиночка и оно спокойно мокается. Там просто статик контейнер
ID чего принимает GetData в интерфейсе IStorageActions? Зачем здесь вообще обобщение используется?
Доступ к статическим ресурсам, чтобы получить единицу даты по неопределённому алгоритму. Сторажд может быть SO, на диске, сетевой реализацией и много чем ещё
В классе ItemVisualData (плохое название, класс описывает информацию о предмете) можно повесить атрибут SerializeField на свойство. Зачем вообще нужен этот атрибут? Для сериализации он не нужен. Почему VisualName - это строка, а не ID ресурса локализации? Статическое и только для чтения - это разные слова с разным смыслом, т.е. "статическое хранилище" - вообще не означает "только для чтения".
Коммент не знания контекста юнити. Он нужен для Unity сериализации. Статическое хранилище в моём контексте ридонли ресурсы, не изменяемое в рантайме
ScriptableObject - это очень ужасное дизайнерское решение разработчиков Unity, что-то сопоставимое с шаблоном одиночка. Не надо в коде использовать сокращения, такие как SO. Почему эти данные не берутся из ресурсов?
Это оболочка над текстовым файлом с готовым интерфейсом. Обычный статический ресурс и в коде он берётся из ресурсов, а не синглтоном. Просто не умение пользоваться Unity. Это как ругаться на ньютонсофт json для хранения статических конфигов.
Для тестов надо писать тесты, а не портить класс User.
Не понимаю вообще к чему и почему
Storage и Inventory. Раз приходится пояснять, почему это не одно и тоже, значит в коде что-то не так. У меня прослеживается логика, что Inventory хранит ячейки инвентаря, а Storage хранит описания предметов. Т.е. Storage - изначально неправильное название сущности + класс занимается загрузкой данных, чем нарушает принцип единственной ответственности из SOLID и отсутствие инверсии зависимостей очень сильно усложняет тестирование такого кода. Почему это не репозиторий, который возвращает обычный массив?
Не нарушает солид, банальное разделение сущностей по доменам если уж мы в термины пойдём подобные. Это два несвязанных домена, поэтому не стоит их обобщать
Что такое null в классе UIInventoryCell? Есть же хотя бы шаблон Null Object, который упростит код в методе SetItem. То, как достается иконка в методе, тихий ужас.
Хотелось бы пояснение в чём. Null - отсутствие предмета. Ничё особеннного
DragContext. Как быть, если инвентарь пользователя взаимодействует с инвентарем ящика?
Использовать тот же контекст
Не надо вызывать Instantiate в классе UIInventoryDragContainer. Зачем использовать Tuple, когда в Unity поддерживается C# с нормальными именованными кортежами, а не вот это вот Item1.
Почему не надо вызывать? Тоже вкусовщина не относящаяся к архитектуре.
Зачем вводить IDynamicStorageActions, когда уже был обобщенный класс для хранения?
Мутабельные и иммутабельные данные. Просто так удобнее разделить
Не надо было в класс User добавлять в свойство вызов метода. Следовало в таком случае отказаться от свойства вообще. И не надо смешивать доменную модель с способом хранения: атрибут JsonProperty не к месту и нарушает принцип единственной ответственности в SOLID. Не надо делать такую жесть как сохранения пользователя на изменение инвентаря. Или возвращение false, когда ничего не удалось загрузить. Никто не вызовет метод Load, если загружать нечего.
На это я ответил выше. Оно ничего не нарушает (хотя это дискуссионный вопрос), просто удобство сериализации данных юзера, чтобы не выносить их в отдельные классы с данными. Это уже спорный момент, но не ведущий ни к каким проблемам с точки зрения проектировки
ralyeh
18.11.2022 23:17+4Проблема в том, что нигде в статье не были описаны требования к инвентарю, кроме картинки, чтобы говорить о чем-то конкретном. В статье есть очень удачное предложение, которое характеризует большинство моих пунктов, что до ввода MonoBehaviour можно было играть в консоли. Они описывают тесную связь логики и представления с примесью статических классов. Никакие интерфейсы, абстракции и события в отделении логики от представления не помогут в этом случае, если их использовать таким образом. Код же необходимо писать таким образом, чтобы он был понятен другому разработчику, а не потому что и так все работает. Большую часть времени код читается, а не пишется.
В User нарушен SRP, потому что он управляет жизненным циклом инвентаря, управляет сохранением и загрузкой состояния, прибит гвоздями к конкретной реализации сериализации. Атрибут Serializable не требует наличие атрибута JsonProperty для сериализации классом JsonUtility, а сериализация классом JsonConvert не требует атрибут JsonProperty для сериализации всех свойств. Т.е. добавляется еще головная боль с тем, а всё ли можно засериализовать, и с тем, что нужно сериализовать, а что нет.
Касательно Null Object, речь про то, что у объекта два состояния: объект есть и объекта нет. Поведение с иконкой вылезло за границы объекта. Я знаю, что в Unity такое обычно решается через шаблон состояние.
Instantiate и зарабатывание миллионов долларов - связанные вещи. Мне так надоело, когда в том же Steam, появляется очередная игра усеянная багами, что иногда даже запустить ее невозможно. Конкретно здесь речь, когда условный тетрис (очень простая игра), отжирает огромное количество ресурсов или падает при странных условиях. Некоторые разработчики даже не удосуживаются банально проверять локаль системы, из-за чего падает DateTime, зато продают игру за 30 долларов. ААА игры тоже страдают от слабого уровня программистов, например Wasteland 3 долгое время грузил уровни по несколько минут, сбрасывал прогресс при загрузке сохранения. Главное верить, что озлобленные потребители не вернут деньги и не перестанут покупать будущие игры.
ScriptableObject обычно используется именно так, как я описал. Бывает даже в него домешивают поведение, никто же не запрещает. Был даже один довольно популярный доклад, который предлагал все на них делать. Такая веселая сущность, что даже сбрасывает все свои свойства при небольшом рефакторинге.
DyadichenkoGA Автор
18.11.2022 23:29-2Я не обозначал целью статьи конкретную реализацию инвентаря. Все перекладывания и т.п. в текущей реализации вообще делаются на уровне префабов при желании. Часть с мелкими доработки. Изначальный тезис статьи, не бывает идеального кода в абстрактном случае. А в конкретном случае писать нет смысла, так как у каждого такой конкретный случай свой. И исходя из своего опыта к любому коду можно придраться, даже если он целиком решает задачу.
Да не нарушен, он управляет жизненным циклом юзера. А интерфейс это часть данных юзера. Это вполне нормальный контейнер обобщение.
Да не нужен тут шаблон состояние, это пере усложнение бизнес логики ни за чем.
Про Instantiate. Ну не зная как работает в Unity GC и в целом сборка мусора, а так же Destroy, и что он в этом случае будет делать, то конечно можно представить невообразимые ужасы. В в данном контексте использования объект будет небольшим и удалится всего лишь ямль конфиг, который соберётся и при стратегии работы с большой кучей не вызовет никогда проблемы. Можно оптимизировать, когда понятна конкретика и контекст.
Про SO, ну кем-то используется, ток я его кажется использую чисто как статический ресурс. В шарпе тоже много вещей, которые нужно когда юзаешь понимать, как их стоит юзать)
yatanai
19.11.2022 02:09+1В моём понимании, реализацию чего-то идеального возможна, но не всегда оправдана по деньгам. Даже в системе с инвентарем, мы можем написать два класса для разных жанров (онлайн), напихать дженериков и написать некий абстрактный интерфейс для доступа к каким-то полям и жить себе спокойно. Просто это займёт до 4 раз больше времени на проектирование, что немного тупо и в целом ничего не поменяет.
lymes
19.11.2022 09:41+3Вам сделали шикарный код ревью, причем бесплатно. Вы бы хоть спасибо сказали.
snobit
19.11.2022 09:24Эта проблема и не решалась
Теперь понятно, откуда у нас столько тормозящего софта. Вместо того, чтобы решить конкретную проблему, будет делать абстракции над абстракциями.
https://www.joelonsoftware.com/2001/04/21/dont-let-architecture-astronauts-scare-you/
Mingun
19.11.2022 10:33Проблемы на самом деле начинаются уже с фразы
Конечно же нет. Переписываем класс, так как имени и иконки тут не должно быть.
Это класс данных. Имя (на самом деле это конечно ID ресурса локализации, он это уже низкоуровневые подробности, которые могут и должны быть сокрыты в типе поля Name) и иконка как раз тут должны быть. А вот рисовать ли их в каждом конкретном случае и как это делать уже решает слой визуализации.
kelegorm
19.11.2022 01:52+1Почему абстрактный класс BaseItem без ключевого слово abstract? Зачем вообще строить какую-то невнятную иерархию наследования без обозначения, кто его наследники? Какая вообще проблема решается через наследование? Почему просто не создать структуру только для чтения под названием InventoryCell (BaseItem - слишком абстрактное название)?
Стандартный подход в геймдеве. Да это может быть класс контейнер в дальнейшем компонентном подходе. Это классика, это при разработке 5-6 взрослых игр знать надо
Я поддержу второго комментатора. Чтобы применять наследование, нужна весомая причина. Данные в инвентаре — это модели, у них я бы не стал делать разное поведение, или какое-то поведение в принципе.
Но ваша аргументация в духе "все так делают, это знать надо" оставляет меня желать лучшего.
Но я радуюсь, когда в моделях предлагают не хранить графику и локализацию. Разделение — это хорошо.kelegorm
19.11.2022 01:57Я щас подумал, что в старых играх я видел мешочки: такой элемент в инвентаре, куда предметы можно засовывать. То есть это предмет с предметами.
В таком случае можно использовать наследование для разделения простой ячейки с предметом и ячейки-контейнера с кучкой предметов. Но можно и не через наследование это сделать.
И в таком случае этот класс IItemCell будет внутренней реализацией внутри структуры инвентаря. А класс, непосредственно описывающий предмет инвентаря, наследовать какой-либо абстрактный класс не должен.
DyadichenkoGA Автор
19.11.2022 02:09-1Дак дело не в аргументации. Дело в том, что я говорю по компонентный подход. У меня наследование даже не используется. По сути про наследование только говорится :)
Аргументация может быть эмоциональной. А теперь давайте пойдём в ООП. Предмет это абстрактный класс или же интерфейс? Так как это объект с данными, а не объект поведения - это абстрактный класс. А вот стоит ли навешивать функциональность компонентами или же делать это через наследование - вопрос к системе. Разное поведение, Ну предположим у нас в инвентаре есть шмот перса, а есть расходники. Это объект одного класса? Если да, то почему и зачем? На клике что происходит, кто определяет и как. расходник не является предметом или шмотка не является предметом.
Просто вот реальный косяк тут тоже есть, но его будто бы никто не замечает, что меня огорчает. И это IEnumerable в интерфейсе инвентаря, но при этом я завязываюсь на логику индексов в идентификаторах ячеек. То есть по сути определяю выше поведение списка, и заменив реализацию на уровне инвентаря - всё сломается и будет баг. И это ошибка проектировки. Причём довольно стандартная)
А многие делают поведения компонентами. Вопрос не в том, что все знают. В этом я выразился некорректно, согласен. Вопрос в том, что обсуждаемое, мантры из SOLID, и т.п. не говорят об одной важной вещи. В чём конкретно проблема этого подхода? Почему стоит сделать иначе? Какой выигрыш это даст?
Суть не в паттернах, солиде, MVC, MVVM и т.п. А в том "для чего". Пример ужасного кода я так же могу показать, но суть в том, что код это далеко не он.
kelegorm
19.11.2022 12:02+1Предмет это абстрактный класс или же интерфейс? Так как это объект с данными, а не объект поведения - это абстрактный класс.
Не могу согласиться. Предмет в инвентаре — это модель, ни больше, ни меньше. Совсем не обязательно использовать абстрактные классы и наследование для разделения объектов по общим значениям в модели. Я, например, в моделях никогда вообще не использую наследование, методы, абстракции.
Вот ниже пример, как предметы героя описываются простым классом. Теги — лишь как один из способов хранить дополнительные особенности каждого предмета.
itemTypeполе описывает основную разницу между предметами. По типу легко определить, что с этим предметом можно делать. Но можно вообще на одних только тегах все описать.Set— это лист, массив, в котором не могут повторяться одинаковые значения, а порядок не важен.@immutable class ItemModel { final String id; final ItemType itemType; final Set<Tag> tags; }Ну предположим у нас в инвентаре есть шмот перса, а есть расходники. Это объект одного класса? Если да, то почему и зачем?
Да, у них одинаковое поведение. Они просто лежат в кармане героя, пока их не достанут. У них разный тип, но тип совсем не обязательно указывать через класс. Можно взять enum.
На клике что происходит, кто определяет и как. расходник не является предметом или шмотка не является предметом.
"Пользователей" предмета полно. Это может быть окно крафта, окно продажи, окно экипировки героя. И везде будут разные функции на "клик" или "drag-n-drop". Вот тот, кто вызывает из памяти список предметов, тот и говорит, что он хочет делать с предметами.
Класс модели совсем не должен содержать поведения, только описание предмета. Поведение делает бизнес-логика.
Скорее всего это просто функция, которая по описанию предмета и определяет, как с ним сделать что-то: какие возможные действия предложить, какую цену на продажу выставить, на какую часть тела это можно одеть. На каждую задачу своя функция, своя логика.
Например, вот способ подсчета стоимости предмета:
double getItemPrice(ItemType type, Set tags) { late double price; switch (type) { case ItemType.sword01: price = 12.0; } if (tags.contans(Tag.broken)) price = price * 0.1; if (tags.contains(Tag.rare)) price = price * 10; }Вопрос в том, что обсуждаемое, мантры из SOLID, и т.п. не говорят об одной важной вещи. В чём конкретно проблема этого подхода? Почему стоит сделать иначе? Какой выигрыш это даст?
Я понимаю, что можно и через ООП и наследование все описать, или еще какими-нибудь другими способами, двумерными массивами, например, вообще без каких-либо классов. Тут нет явно правильного и неправильного варианта. Просто у каждого подхода будут свои особенности.
На своем опыте я пришел к тому, что наследование приносит много проблем, делает код слишком жестким к изменениям, его сложно поддерживать, приходится порою придумывать костыли для обхода механизмов наследования.
А что наследование дает взамен здесь? Я не знаю. Но чем меньше модель знает о вариантах поведения, тем лучше. Чем меньше связанности в коде — тем лучше. Чем гибче код — тем проще его изменять, это хорошо.
DyadichenkoGA Автор
19.11.2022 12:18+1Да согласен. Тут нет вопросов, всё понятно и аргументировано. Спасибо. Я немного обсудил в своём канале в комментах к этому посту. Мой подход немного в другом, скопирую сюда часть, но суть та же. Enum это тоже не очень хорошо.
Вопрос:
Допустим потребуется у каждого предмета в инвентаре указывать его количество. Для реализации этого нужно будет добавить int Count в BaseItem? Или нужно будет отнаследоваться? Мне, честно, не очевидно, какой именно из вариантов подразумевается.
Или если посложнее - нужно будет добавить новую фичу - айтем-рюкзак. Как это реализовывать нужно будет? Наследоваться от BaseItem? Вводить какой-то enum в BaseItem, указывающий на тип предмета? Добавлять список компонентов и в этот список добавлять объект, отнаследованный от IInventoryActions? По-хорошему - над этим должен думать не исполнитель, которому поручена эта задача, а архитектор, изначально написавший этот код.
Т. е. моя претензия в том, что поручить кому-то независимо расширять этот код невозможно - исполнителю по-любому придётся связываться с архитектором, и в этом его косяк.
Ответ:
Да нет, Count в BaseItem добавлять нельзя. Это приведёт к тому, что придётся делать его Nullable, а обработка на null (или -1) из-за того, что предмет не имеет понятия Count везде — это плохо. Нужно отнаследоваться из этих двух выборов. А вообще нужно по хорошему завести свойство, которое запрашивается по айди из стораджа. И от него уже скачет реализация.
Миллион свойств, миллион обработок в ячейке на отображение и на клик?) Да нет, так как там будут только свойства обрабатываться относящиеся к инвентарю — это зависит уже от сложности реализации инвентаря. Чистое наследование — это скорее плохо, так как прийдётся либо тайпкастить, либо разделять ячейки на типы по типу предмета для отрисовки предмета. Логичнее, как и с иконкой идти по чекналл и отрисовке свойств в самой ячейке, вряд ли их там будет миллиард :)
И любой айтем можно таким образом добавить, так как вопрос лишь в том, как получить к нему данные, а для этого нам нужен только айди. А инвентарь занимается отрисовкой свойств относящихся к нему. И ему плевать что сумка-айтем умеет что-то ещё. Так что BaseItem надо было запечатать. А само по себе это для читаемости.
Это даже не класс модели. По сути этот класс можно было запечатать и свести к идентификатору. Называется он так для читаемости. Так как это по сути скелет, на который набрасываются свойства. Он даже не знает о существовании этих свойств, они набрасываются в зависимости от контекста использования. Как иконка. И дальше идёт базовый механизм расширения функциональности любого вью, без кучи "наследников вью, тайпкастов" и прочего мракобесия. Проверка обладает ли этот ID этим свойством, и если да то его отрисовкой в соответствующей панели. Так можно расширять до бесконечности от конкретики интерфейса. Просто есть стораджи разных свойств, эти свойства приписаны к идентификатором предмета. Вью или контроллеры запрашивают из стораждей (по сути моделей). "А у этого айди есть это свойство?" Если ответ да, то подключают нужную функциональность.
Просто это не делает код ужасным. Спорным в контексте конкретных применений - да. И как раз про это статья. Просто если скажем предметов у нас два типа или у них нет множества свойств. То это вообще не имеет значения. И можно наследоваться. А подход описанный мной сейчас - это оверинжиниринг.
kelegorm
19.11.2022 14:07Да, у предметов может быть много самых разных свойств, которые может быть трудно уложить в одну древовидную структуру.
Например, количество предмета, патронов. Можно описать поле countable, которое является union типом. Может выглядеть так:
class ItemModel { final Countable countable; final ItemType itemType; ItemModel({this.type, this.countable}); //... } final ammo50 = new ItemModel( type: ItemType.ammo50, countable: Countable.many(137), ); final int count = ammo50.countable.when( single: () => return 1, many: (count) => return count );Этот union type штука удобная, из функционального программирования пришла. Отличная вещь!
Таким образом предметам можно описать много разных видов свойств, которые по характеристикам отличаются друг от друга. Можно разные варианты множественности вещей описать.
Например, патроны не просто в количестве 137 штук, а они в инвентаре должны отображаться пачками по 50. Тогда
Countableможет предусматривать третий вариантstackedс дополнительным полемstackMax = 50.Удобно, потому что свойством "количество" могут обладать предметы из разных категорий. Не нужно будет делать сложные наследования, миксины. А самое главное, не нужно будет делать проверки на тип предмета.
Еще с наследованием проблема в том, что когда мы делаем проверки на тип, мы должны знать, какой список классов и интерфейсов проверяем. А если его надо обновить из-за того, что мы изменили структуру дерева наследования? А если таких мест в коде много, и их еще поискать надо все, чтобы не забыть?
RomeoGolf
19.11.2022 08:52+2TL; DR:
Поэтому очень тяжело писать про хороший код, так как у каждого своё понятие хорошего кода, исходя из опыта.
Спасибо, Кэп!
И это в предпоследнем абзаце. Хотел посмотреть авторский ответ на вопрос из заголовка. Нашел авторское определение эффекта Даннинга-Крюгера, авторские размышления о какой-то игрушке с инвентарем, авторскую простыню кода. И немножко рассуждений, хорош ли конкретно код из простыни. Ну, спасибо и на том.
klimkinMD
19.11.2022 09:38+2Меня периодически спрашивают, почему я так мало пишу про архитектуру
Навеяло
"Ко мне обратилась группа товарищей из молодежи с предложением высказать свое мнение в печати по вопросам языкознания..."
Suvitruf
19.11.2022 18:03А потом у нас появятся предметы, которые занимают более одной ячейки...
DyadichenkoGA Автор
19.11.2022 18:07Ага. Но это не меняет логику «ячейка-предмет». Просто появляется свойство предмета «форма в инвентаре», как отдельная информация по ID, и валидация возможности положить предмет в инвентарь в зависимости от форм, которые там уже находятся.
Sektor2350
19.11.2022 23:16+2"Почему тяжело писать про хороший код?"
- Ревью кода в комментариях показывает почему.
У меня самого давно сложилось мнение и ощущение, того, что в большинстве своём в гейм-деве происходит тихий ужас под капотом. Реализации намертво прибиты к чему-то, очень плохая выстроенная система, SOLID, паттерны и т.д. и т.п. редко применимы. Подавляющая часть видео, уроков, статей, смотришь код и страшно. Начиная с банального именования. Такое ощущение что гейм-дев постоянно отстаёт от современной разработки.
Имхо писать надо стараться нормальный читаемый код, хорошо сразу не получится, нет пределов совершенству. Доводить его до хорошего да стоит, иначе разгребать тех долг через какое-то время станет больно
SadOcean
Хорошая статья
Возможно стоит подробнее обсуждать минусы решений в некоторых сносках, например вот это решение вызовет проблемы при синхронизации по сети, а синглтоны конфликтуют с тестами и могут вызвать проблемы с пользователем как с сущностью (для сейв лоада)
Но вообще вопросы хорошие - в сети описано много по хорошему коду, но мало по хорошей архитектуре, хорошим модулям и компонентам.
И тут раскрывается главное - не бывает хорошего в вакууме, есть много приемов, которые хороши для тех или иных требований и имеют свою цену.
Практичный подход помогает.