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 |
---|---|
|
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 aRigidbody
. - 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
CharacterController
s 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 inFixedUpdate
, meaning that quick keypresses can be missed if multipleUpdate
s occur between them (if a key is pressed on the firstUpdate
, then released on the second, the followingFixedUpdate
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 ownAwake
method so other scripts can access it through that property. But for some unknown reason, some scripts useObject.FindObjectOfType
to find it themselves and thePlayerController
usesGetComponent
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 AudioClip
s 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; }
.
- Alternatively, they could have been Auto-Properties which don't get serialized anyway:
- 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 thePlayerController.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.StateMachineBehaviour
s have a similarly pointlessSMB
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 theMoveInput
property. Shortening fromMovement
toMove
just seems lazy and theInput
suffix is unnecessary when the class is already calledPlayerInput
. MoveInput
,CameraInput
, andJumpInput
all have that suffix, but thenAttack
andPause
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 protected field
- The
MeleeWeapon.PARTICLE_COUNT
constant also deserves a mention because other constants use thek_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 anAnimationClip
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.