Problems with the Mecanim Character

This page is part of the 3D Game Kit sample.

The character implemented in this sample is directly based on the player character from the 3D Game Kit Lite, but it's not an exact copy because the Mecanim character had several problems which we want to avoid.

Script Referencing

In order to avoid causing errors when the 3D Game Kit Lite is not in your project, the scripts in this sample can't directly reference any of the scripts from the 3D Game Kit Lite. This isn't a problem with the Game Kit itself, just a general logistical issue.

So to avoid needing identical copies of all those scripts, this sample uses UnityEvents where interaction between systems is required even though you would probably not want to set it up like that in a real project.

This mostly applies to scripts that want to play sounds using the RandomAudioPlayer script, so instead of:

[SerializeField]
private RandomAudioPlayer _SomeSound;

_SomeSound.PlayRandomClip();

These scripts instead use a UnityEvent which can be configured to call the correct method in the Inspector without the script actually referencing RandomAudioPlayer:

Code Inspector
[SerializeField]
private UnityEvent _SomeSound;

_SomeSound.Invoke();

Locomotion

The character's Locomotion has several issues that make it feel unresponsive:

  • Lack of Full Movement Control means the character will move in a direction the player doesn't want while turning towards the desired direction.
  • The left and right quick turn animations have different import settings (one loops and the other does not, among other things) and different transition settings. It is unknown whether there is actually a good reason for this or if it was just an oversight.
  • You have limited Acceleration. This isn't necessarily a "problem" as such, but the implementation is not physically consistent and it's unusual for a character using Unity's CharacterController component instead of a Rigidbody.
  • Sometimes when you jump, the character stays in the Idle animation instead of entering the Airborne Blend Tree properly so it looks like she's just floating upwards for no reason:

Character Controller

Unity's CharacterController component is an easy to use alternative to Rigidbody physics for characters, but it has several notable drawbacks which must be considered when deciding how to implement characters in your own game. Unfortunately, actually fixing these issues is beyond the scope of this sample since they're purely physics problems that have nothing to do with animations or character state logic.

Ground Checks

A CharacterController is a capsule shape and it's possible to get the edge of the bottom curve of your capsule on top of a corner, causing it to react as if you were properly on the ground even though the center of the character is actually standing in mid air. If you get close enough to the edge, it can even be far enough down the curve that you can't even move onto the platform because it's too steep, meaning it's treating that edge as both the ground and a wall at the same time.

This problem also occurs on steep slopes:

Rigidbody Interactions

CharacterControllers also have inconsistent interactions with Rigidbody objects. The destructible boxes in the 3D Game Kit generally act as solid walls when the player runs into them, except that sometimes for no apparent reason they stop doing that and the player is able to push them around without any resistance as long as the box keeps moving:

Input

There are several issues with the way player input is handled:

  • The PlayerController only reacts to jump input in FixedUpdate, meaning that quick keypresses can be missed if multiple Updates occur between them (if a key is pressed on the first Update, then released on the second, the following FixedUpdate will not detect that keypress). This might seem obvious, but randomly ignoring legitimate player input is not good.
  • You can't Attack during the Quick Turn animations. This is another arbitrary lack of control. You can almost always attack when on the ground, except when you happen to turn sharp enough while moving quickly enough to trigger one of the quick turn animations, in which case you suddenly can't attack for a short time.
  • You also can't Attack during the exit transition of a previous attack:
    • You can't start a new attack during most of the previous attack.
    • Then there's a very small window of time where you can execute the next attack in the combo.
    • Then the attack ends and any attack inputs are ignored until you completely finish returning to Idle.
    • Then once Idle you can attack again.
  • The logical flow from pressing a button to executing the desired action (such as Jumping or Attacking) is extremely disorganised and hard to follow.

Splitting PlayerInput away from the rest of the PlayerController is good for Separation of Concerns, however some of those concerns ended up in the wrong place:

  • Converting the player's desired movement direction into world space should be done in PlayerInput. That way if a different input system is used - such as using AI to control the character or recording events and then replaying them later on - then that system gets to decide exactly how it wants to move the player instead of having the player interpret everything it says to be relative to the camera.
  • The LocomotionSM has an arbitrary input "dead zone" in the conditions of its transitions where any speed below 0.1 is treated as not moving. Again, this is something that should be handled as part of the input system so it can be easily customised, not hidden as a bunch of magic numbers in half a dozen different Animator Controller transitions.
  • It contains a public static PlayerInput Instance singleton property that it assigns in its own Awake method so other scripts can access it through that property. But for some unknown reason, some scripts use Object.FindObjectOfType to find it themselves and the PlayerController uses GetComponent to find it. It is possible that there's a genuine reason for these differences, but that seems unlikely and without comments there's no easy way to tell.

Audio

The RandomAudioPlayer script references multiple AudioClips so that it can play a random one each time and also provides a different selection depending on the Material of the object the character is standing on. In particular, this is used to have footstep sounds that eash sound slightly different and have a different set of sounds for different surfaces (stone, grass, etc.).

The concept is sound, but the implementation has one notable flaw: the playing and canPlay fields:

[HideInInspector]
public bool playing;
[HideInInspector]
public bool canPlay;
  • The [HideInInspector] attribute does what it says: it prevents the field from appearing in the Inspector. But it doesn't actually prevent the value from being saved as part of the scene. This means a script could set the value in the Unity Editor and you would never know why it was in the wrong state on startup (and it would waste performance loading that value). Instead, the [NonSerialized] attribute should have been used to prevent them from being saved at all.
    • Alternatively, they could have been Auto-Properties which don't get serialized anyway: public bool Playing { get; set; }.
  • But the real issue is that they shouldn't even be in this class in the first place. The RandomAudioPlayer class doesn't use them itself and of the several dozen places throughout the project that use it, the only place that actually uses these two fields is in the PlayerController.PlayAudio method for the footstep sounds so there's no reason for them not to be in that class instead.

Naming Conventions

There are also a number of issues with the quality of the code and project structure in the 3D Game Kit. These barely even qualify as minor problems, but they're worth noting in a product that was made by Unity Technologies and should thus be held to a high standard.

It doesn't matter so much whether you use m_Name for private fields or camelCase for properties or whatever specific naming convention you use. The most important things are to use a meaningful convention and to apply it consistently.

For a naming convention to be meaningful, it should provide or emphasize useful information that isn't already easily accessible:

  • In the player's Animator Controller, the main sub state machines have an SM suffix which differentiates them from other states (IdleSM, LocomotionSM, etc.). This is totally unnecessary since sub state machines are already clearly differentiated by having a hexagon shape instead of being rectangular.
    • StateMachineBehaviours have a similarly pointless SMB suffix.
  • Conversely, the Blend Trees don't have any particular suffix to differentiate them even though they look the same as regular states. This would've been a good situation for a naming rule to cover for a deficiency in the user interface.

Applying rules consistently is also important because variations imply a difference of intent:

  • The PlayerInput script in particular has several notable issues:
    • The protected field m_Movement is exposed publicly by the MoveInput property. Shortening from Movement to Move just seems lazy and the Input suffix is unnecessary when the class is already called PlayerInput.
    • MoveInput, CameraInput, and JumpInput all have that suffix, but then Attack and Pause don't. This implies something different about those groups of properties, but upon investigation that doesn't seem to be the case.
    • The CameraInput property is never used (camera rotation seems to be handled by Cinemachine). Unused code should generally be removed to avoid wasting people's time when they are trying to figure out how things work.
  • The MeleeWeapon.PARTICLE_COUNT constant also deserves a mention because other constants use the k_PascalCase convention.
  • There are plenty of places in the code and the Animator Controller that specifically mention the character's name (Ellen), which is silly because the name of the character has no bearing on their logic.
    • EllenRunForwardLandingFast is fine as the name of an AnimationClip because it's specific to that character model and giving her a name makes it easier to remember than something like "Character 3". But it shouldn't also be used for the name of the Animator Controller state that it's used in. That state should have a general name like "Hard Landing" to describe what it's actually used for more concisely and allow any character with a "Hard Landing" state to be controlled by the same script.