Code review request: architectural solution for a very simple directional sprite system

Unity Engine
,
1

So. I wrote scripts that process the sprite of an object based on the angle at which the camera is looking at it (doom-style directional sprites). I’d like you to review my code, point out bad decisions, and generally tell me what you think about it.

The abstract class SpriteRotation contains the logic of assigning a sprite to a SpriteRenderer of an object:

[RequireComponent(typeof(SpriteRenderer))]
public abstract class SpriteRotation : MonoBehaviour
{
        [SerializeField] protected Transform character;

        protected SpriteRenderer spriteRenderer;
        protected Transform cameraTransform;
   
        private void Start()
        {
            spriteRenderer = GetComponent<SpriteRenderer>();
            cameraTransform = Camera.main!.transform;
        }

        private void Update()
        {
            spriteRenderer.sprite = GetSectorSprite();
        }
   
        protected float GetCameraToCharacterAngle() =>
Vector2.SignedAngle(character.forward.XZ(), cameraTransform.forward.XZ());

        protected abstract Sprite GetSectorSprite();
}

The art style of my project requires sprite replacement in both 8 and 16 directions. Therefore, I have child classes from the abstract class that override the method containing the logic of determining the appropriate sprite:

public class EightDirectional : SpriteRotation
{
        [SerializeField] private EightDirectionalSpritesContainer spritesContainer;
   
        protected override Sprite GetSectorSprite()
        {
            var angle = GetCameraToCharacterAngle();

            return Mathf.Abs(angle) switch
            {
                < 22.5f => spritesContainer.up,
                < 67.5f => angle < 0 ? spritesContainer.upLeft : spritesContainer.upRight,
                < 112.5f => angle < 0 ? spritesContainer.left : spritesContainer.right,
                < 157.5f => angle < 0 ? spritesContainer.downLeft : spritesContainer.downRight,
                _ => spritesContainer.down
            };
        }
}

public class SixteenDirectional : SpriteRotation
{
        [SerializeField] private SixteenDirectionalSpritesContainer spritesContainer;
   
        protected override Sprite GetSectorSprite()
        {
            var angle = GetCameraToCharacterAngle();

            return Mathf.Abs(angle) switch
            {
                < 11.25f => spritesContainer.up,
                < 33.75f => angle < 0 ? spritesContainer.upLeft0 : spritesContainer.upRight0,
                < 56.25f => angle < 0 ? spritesContainer.upLeft : spritesContainer.upRight,
                < 78.75f => angle < 0 ? spritesContainer.upLeft1 : spritesContainer.upRight1,
                < 101.25f => angle < 0 ? spritesContainer.left : spritesContainer.right,
                < 123.75f => angle < 0 ? spritesContainer.downLeft0 : spritesContainer.downRight0,
                < 146.25f => angle < 0 ? spritesContainer.downLeft : spritesContainer.downRight,
                < 168.75f => angle < 0 ? spritesContainer.downLeft1 : spritesContainer.downRight1,
                _ => spritesContainer.down
            };
        }
}

Sprites are stored in Serializable structures respectively:

[System.Serializable]
public struct EightDirectionalSpritesContainer
{
        [FormerlySerializedAs("Up")] public Sprite up;
        [FormerlySerializedAs("UpRight")] public Sprite upRight;
        [FormerlySerializedAs("Right")] public Sprite right;
        [FormerlySerializedAs("DownRight")] public Sprite downRight;
   
        [FormerlySerializedAs("Down")] public Sprite down;
        [FormerlySerializedAs("DownLeft")] public Sprite downLeft;
        [FormerlySerializedAs("Left")] public Sprite left;
        [FormerlySerializedAs("UpLeft")] public Sprite upLeft;
}

[System.Serializable]
public struct SixteenDirectionalSpritesContainer
{
        [FormerlySerializedAs("Up")] public Sprite up;
        [FormerlySerializedAs("UpRight0")] public Sprite upRight0;
        [FormerlySerializedAs("UpRight")] public Sprite upRight;
        [FormerlySerializedAs("UpRight1")] public Sprite upRight1;
        [FormerlySerializedAs("Right")] public Sprite right;
        [FormerlySerializedAs("DownRight0")] public Sprite downRight0;
        [FormerlySerializedAs("DownRight")] public Sprite downRight;
        [FormerlySerializedAs("DownRight1")] public Sprite downRight1;
   
        [FormerlySerializedAs("Down")] public Sprite down;
        [FormerlySerializedAs("DownLeft0")] public Sprite downLeft0;
        [FormerlySerializedAs("DownLeft")] public Sprite downLeft;
        [FormerlySerializedAs("DownLeft1")] public Sprite downLeft1;
        [FormerlySerializedAs("Left")] public Sprite left;
        [FormerlySerializedAs("UpLeft0")] public Sprite upLeft0;
        [FormerlySerializedAs("UpLeft")] public Sprite upLeft;
        [FormerlySerializedAs("UpLeft1")] public Sprite upLeft1;
}

I have a feeling that child classes are superfluous and inappropriate, but at the same time I don’t know how I could change this.

Thank you very much in advance for your reply

2

Something has gone horribly wrong with your cut/paste above. Perhaps try paste-match-style instead?

If you post a code snippet, ALWAYS USE CODE TAGS:

How to use code tags: Using code tags properly

3

If I understood you correctly, this is exactly what I am doing: I am getting the signed angle between the directions of camera and object and the based on that angle determine in which direction the object is in relation to the player: UpLeft, UpRight, Down, Up, etc. And I also have a script for billboarding that I didn’t include here because it has nothing to do with replacing sprites.

4

For the math I’d have used, well, math: sNum=(signedAngle+180-12.25f)/22.5f should give you up, up-right, right … for 8 of them, and similar for 16. Maybe some trial and error. Then the struct holding them could be an array, allowing you to find the sprite with mySprites[sNum];. But I can see the small advantage of having the sprites in named variables, even if it does require a pile of IF’s.

Using child classes seems fine – not better, but not really worse (the most common way subclasses are worse is that people don’t understand them). I’m assuming each “thing” is locked to an 8 or 16 sprite count (ogres always have 8, dogs always have 16). If not, changing the subclass is a pain. So you could do it old school – no subclasses, instead add a spriteCount variable, set to 8 or 16, and useif(spriteCount==8).

1 Like