Как можно улучшить код этот прыжка?

Question

[Fjfgs]

Я прошу вас посмотреть этот код реализации прыжка для телефонов(пока что я не имею доступа к ПК). Можно ли улучшить код и если да, то как? Я тут вижу 1 проблему. Transform.position телепортирует объект, а этот подход в корне неправильный, ведь персонаж будет входить в колайдер другого объекта. Или я не прав?

using UnityEngine;

public class PhysicsJump : MonoBehaviour
{
    [SerializeField] private AnimationCurve YAnimation;
    
    [SerializeField] private PhysicsMovement Movement;
    
    private float ExpiredTime;
    private float Duration = 1;
    
    private float Height = 5;
    
    private bool IsJumping;
    
    private bool IsOnGrounded;
    
    private void Update()
    {
        if (IsJumping)
        {
            Movement.Speed = 40;
            
            ExpiredTime += Time.deltaTime;
            
            if (ExpiredTime > Duration)
            {
                ExpiredTime = 0;
            }
            
            float progress = ExpiredTime / Duration;
            
            transform.position = new Vector3(0, YAnimation.Evaluate(progress) * Height, 0);
        }
    }
    
    public void Jump()
    {
        if (IsOnGrounded)
        {
            IsJumping = true;
        }
    }
    
    private void OnCollisionEnter(Collision collision)
    {
        if (collision.gameObject.tag == ("Ground"))
        {
            IsOnGrounded = true;
            
            IsJumping = false;
        }
    }
        
    private void OnCollisionExit()
    {
        if (collision.gameObject.tag == ("Ground"))
        {
            IsOnGrounded = false;
        }
    }
}

Answer 1

[K0TlK]

Убери проверку по тэгу.
transform.position замени на Rigidbody.MovePosition.
Таймер из апдейта в корутину перемести и в Jump ее запускай. Чтобы остановить прыжок, закэшируй корутину и останавливай ее, когда нужно.
40 вынеси в отдельное поле.
Это у тебя не PhysicsJump, физики здесь никакой нет, это анимированный прыжок, переименуй. IsOnGrounded - "является на приземленный". grounded достаточно. У тебя IsOnGrounded всегда противоположно IsJumping. Зачем здесь два флага?
Это лишний монобех, перемести код отсюда в Movement свой. Если движение у тебя вдоль поверхности, то нормаль, которую ты получаешь при движении, можно использовать в качестве замены флагов grounded и jumping (если нормаль не нулевая, то можно прыгать, если нулевая, то прыжок происходит). Соответственно, теперь тебе не нужны тэги, т.к. землю ты проверяешь через нормаль, флаги, по той же причине, не нужен апдейт, т.к. анимация происходит в корутине, не нужен лишний монобех, чтобы не делать из объекта кучу монобехов. Остается только паблик метод Jump и корутина.

Comments

[Fjfgs]

Типо такого? (Только я не перенес код прыжка в код передвижения)

using UnityEngine;

[RequireComponent(typeof(Rigidbody))]
public class AnimationJump : MonoBehaviour
{
    [SerializeField] private RigidBody Rigidbody;
    
    [SerializeField] private AnimationCurve YAnimation;
    
    [SerializeField] private PhysicsMovement Movement;
    
    private float ExpiredTime;
    private float Duration = 1;
    
    private float Height = 5;
    
    private float SpeedWhileJumping = 40;
    
    private bool IsJumping;
    
    private Coroutine JumpCoroutine;
    
    private void Start()
    {
        Rigidbody = GetComponent<RigidBody>();
    }
    
    private void Update()
    {
        IsJumping = Movement.Normal is null ? true : false;
    }
    
    public void Jump()
    {
        if (IsJumping)
        {
            JumpCoroutine = StartCoroutine(JumpCoroutine());
        }
    }
    
    private IEnumerator JumpCoroutine()
    {
        Movement.Speed = SpeedWhileJumping;
        
        while (progress < 1)
        {
            ExpiredTime += Time.deltaTime;
            
            float progress = ExpiredTime / Duration;
            
            Rigidbody.MovePosition(0, YAnimation.Evaluate(progress) * Height, 0);
            
            yield return null;
        }
        
        ExpiredTime = 0;
        
        StopCoroutine(JumpCoroutine());
    }
}

[Fjfgs]

Fjfgs, как я понимаю здесь можно даже не кэшировать коротину и не использовать StopCoroutine()

[K0TlK]

Fjfgs, Проверка на null не очень хорошая идея. Нормаль представлена в виде вектора, у вектора есть Vector3.zero. Когда нет соприкосновения с землей, нормаль = Vector3.zero.
StopCoroutine в конце корутины лишнее, она сама завершится.
ExpiredTime перемести из скоупа класса в скоуп корутины и тебе не нужно будет обнулять его:

private IEnumerator Jumping()
{
    Movement.Speed = SpeedWhileJumping;
    var expiredTime = 0f;
    ...
}

Апдейт здесь не нужен. Ты вызываешь метод Jump и там уже выполняется проверка: если нормаль != Vector3.zero, то прыгать, соответственно, IsJumping тоже не нужен.

Кэшировать корутину надо, если тебе нужно прервать ее выполнение до момента, когда она сама завершится.

[Fjfgs]

K0TlK, большое спасибо за своеобразное ревью. А апдейт все же я оставлю. Я специально его оставил, чтобы сделать скрит по типу состояний игрока. В методе jump() состояние будет обновляться частично, что есть плохо

[K0TlK]

Fjfgs, Ужасное решение. IsJumping делает только то, что проверяет нормаль на нул, делает оно это каждый кадр, что неразумно с точки зрения логики. Если код будешь на гитхабе у себя оставлять как пример проекта или отправлять на ревью, то ревьюер сразу поймет, что что-то здесь не так.

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

Не понял что ты хотел сказать. Состояние игрока должно быть инкапсулировано в объекте игрока и не должно быть доступно извне.

В методе jump() состояние будет обновляться частично, что есть плохо

А оно и не должно обновляться. У тебя нигде в коде не используется IsJumping, кроме как метода Jump, метод Jump вызывается единоразово, соответственно, ему единожды нужно проверить нормаль в момент, когда метод вызывается. Его не нужно каждый кадр обновлять.
Я для этого и писал, что нужно перенести всё это в компонент движения, чтобы не дергать какой-то лишний компонент.

Answer 2

[Некит Медведев]

разрабы юнити предлагают такой вариант:
https://docs.unity3d.com/ru/530/ScriptReference/Ri...
правда нужно добавить проверку на двойной прыжок и возможно прибавлять вектор, а не приравнивать