Правильный способ получить правильную реализованную перегрузку в дочернем классе

Question

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

В Unity я хочу, чтобы некоторые GameObject s (например, игрок) могли подбирать предметы (другие GameObject s).

Для этого я разработал следующий базовый код:

Компонент, который тянет и поднимает предметы:

public class PickupMagnet : MonoBehaviour
{   
    // [...]
    private void Update()
    {
        Transform item = FindClosestItemInRange(); // Well, this line doesn't exist, it's just a simplification.
            if (ítem != null)
             Pickup(item);
    }

    private void Pickup(Transform item)
    {
        IPickup pickup = item.GetComponent<IPickup>();
        if (pickup != null)
        {
            pickup.Pickup();
            Destroy(item);
        }
    }   
}

Интерфейс тех (, что на данный момент) предметов:

public interface IPickup
{
    void Pickup();
    // [...]
}

И единственный предмет, который я сделал в тот момент:

public class Coin : MonoBehaviour, IPickup
{
    private int price;
    // [...]

    void IPickup.Pickup()
    {
        Global.money += price; // Increase player money
    }   
    // [...]
}

Все работало отлично, пока я не хотел добавить новый предмет: аптечку. Этот предмет увеличивает здоровье существа, которое его подбирает. Но для этого мне нужен экземпляр скрипта существа: LivingObject.

public class HealthPack: MonoBehaviour, IPickup
{
    private int healthRestored;
    // [...]

    void IPickup.Pickup(LivingObject livingObject)
    {
        livingObject.TakeHealing(healthRestored);
    }   
    // [...]
}

Проблема в том, что IPickup.Pickup() не имеет никаких параметров. Очевидно, я мог бы просто изменить его на IPickup.Pickup(LivingObject livingObject) и проигнорировать параметр на Coin.Pickup, но что если в будущем я захочу добавить больше видов элементов, которые требуют других аргументов? Другим вариантом будет добавление нового метода в интерфейс, но это заставит меня реализовать Coin.Pickup(LivingObject livingObject) и реализовать его.

Подумав об этом, я удалил IPickup и заменил его на:

public abstract class Pickupable : MonoBehaviour
{
    // [...]
    public abstract bool ShouldBeDestroyedOnPickup { get; }
    public virtual void Pickup() => throw new NotImplementedException();
    public virtual void Pickup(LivingObject livingObject) => throw new NotImplementedException();
}

А затем переопределите необходимый метод в Coin и в HealthPack. Также я изменил PickupMagnet.Pickup(Transform item) на:

public class PickupMagnet : MonoBehaviour
{
    // [...]
    private LivingObject livingObject;

    private void Start()
    {
        livingObject = gameObject.GetComponent<LivingObject>();
    }
    // [...]
    private void Pickup(Transform item)
    {
        Pickupable pickup = item.GetComponent<Pickupable>();
        if (pickup != null)
        {
            Action[] actions = new Action[] { pickup.Pickup, () => pickup.Pickup(livingObject) };
            bool hasFoundImplementedMethod = false;
            foreach (Action action in actions)
            {
                try
                {
                    action();
                    hasFoundImplementedMethod = true;
                    break;
                }
                catch (NotImplementedException) { }
            }

            if (!hasFoundImplementedMethod)
                throw new NotImplementedException($"The {item.gameObject}'s {nameof(Pickup)} class lack of any Pickup method implementation.");
            else if (pickup.ShouldBeDestroyedOnPickup)
                Destroy(item.gameObject);
        }
    }
}

По сути, это перебирает все методы, определенные в actions, и выполняет их. Если они поднимают NotImplementedException, он продолжает пытаться использовать другие методы из массива.

Этот код работает нормально, но лично мне не понравилась идея определения этого массива с каждой перегрузкой Pickable.Pickup.

Итак, я начал проводить некоторые исследования и обнаружил нечто, называемое «отражение». Я до сих пор не знаю, как это работает, но мне удалось создать этот рабочий код.

private void Pickup(Transform item)
{
    Pickupable pickup = item.GetComponent<Pickupable>();
    if (pickup != null)
    {
        bool hasFoundImplementedMethod = false;
        foreach (MethodInfo method in typeof(Pickupable).GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly))
        {
            if (method.Name == "Pickup")
            {
                ParameterInfo[] parametersGetted = method.GetParameters();
                int parametersAmount = parametersGetted.Length;                    
                object[] parametersObjects = new object[parametersAmount ];
                for (int i = 0; i < parametersAmount; i++)
                {
                    Type parameterType = parametersGetted[i].ParameterType;
                    if (parameters.TryGetValue(parameterType, out object parameterObject))
                        parametersObjects[i] = parameterObject;
                    else
                        throw new KeyNotFoundException($"The key Type {parameterType} was not found in the {nameof(parameters)} dictionary.");
                }
                bool succed = TryCatchInvoke(pickup, method, parametersObjects);
                if (succed) hasFoundImplementedMethod = true;                    
            }
        }

        if (!hasFoundImplementedMethod)
            throw new NotImplementedException($"The {item.gameObject}'s {nameof(Pickup)} class lack of any Pickup method implementation.");
        else if (pickup.ShouldBeDestroyedOnPickup)
            Destroy(item.gameObject);
    }
}

private bool TryCatchInvoke(Pickupable instance, MethodInfo method, object[] args)
{
    try
    {
        method.Invoke(instance, args);
        return true;
    }
    catch (Exception) // NotImplementedException doesn't work...
    {
        return false;
    }
}

И добавлено к MagnetPickup:

private LivingObject livingObject;
private Dictionary<Type, object> parameters;

private void Start()
{
    livingObject = gameObject.GetComponent<LivingObject>();
    parameters = new Dictionary<Type, object> { { typeof(LivingObject), livingObject } };
}

... и работает.

Я не очень знаком с профилировщиком Unity, но я думаю, что последний код работал немного (менее 1%) быстрее.

Проблема в том, что я не уверен, что этот код принесет мне проблемы в будущем, так что это мой вопрос: Является ли отражение правильным способом решения этой проблемы, или мне следует использовать попытку try / catch или, может быть, другой код?

Только на 1% я не уверен, стоит ли мне рисковать его использованием. Я не ищу лучшее исполнение, просто правильный способ решить эту проблему.

Answer 1

Я думаю, что лучший способ сделать это - отправить ссылку на ваш объект Player в Pickup, а затем выполнить пользовательскую логику для каждого типа объекта Pickup.Возможно, вы могли бы пропустить интерфейс и просто иметь базовый объект с именем «PickupObject» или что-то еще, а затем позволить FindClosestItemInRange вернуть их.

И, наконец, вы должны уничтожить GameObject вместо того, что вы когда-либо передавали в Pickup.функция.Возможно, вы можете уничтожить Transform и получить тот же результат (я не пробовал этого), но это просто хорошая практика - уничтожить GameObject, а не какой-либо компонент GameObject

public class PickupObject : MonoBehaviour
{
    virtual void Pickup(Player playerObject) { }
}

public class Coin : PickupObject 
{
    public int price;
    override void Pickup(Player playerObject)
    {
        playerObject.money += price; // Move money over to the player as it probably makes more sense
    }
}
public class HealthPack : PickupObject 
{
    public int healthRestored;
    override void Pickup(Player playerObject)
    {
        playerObject.health += healthRestored;
    }
}

public class PickupMagnet : MonoBehaviour
{   
    public Player PlayerObject;
    private void Update()
    {
        PickupObject item = FindClosestItemInRange();
        Pickup(item);
    }

    private void Pickup(PickupObject pickup)
    {
            pickup.Pickup(PlayerObject);
            Destroy(pickup.gameObject);
    }   
}

Edit, некоторые общие мыслив вашем коде:

Если у вас есть общий "LivingObject", который может забирать как здоровье, так и монеты, как подсказывает ваш код, вы, вероятно, обобщаете слишком много.Похоже, у вас просто есть игрок, которому нужно что-то подбирать.Позволить «чему-либо» быть в состоянии уловить «что-нибудь» - в моем опыте слишком много обобщений.Не пытайтесь решить каждую будущую проблему в первых строках кода.Если вы не уверены, куда направляетесь, или структура сложна на этом раннем этапе.Напишите код, который делает то, что вам нужно, и реорганизуйте его при появлении шаблонов и повторений.