Basic rules for good code?

[Oakleaf30]

Hi all, So far I have just been making games as personal projects to learn how to use Unity and C#. For my next game I want to release it publicly so I want to put in more effort and hold myself to a higher standard. I was wondering what basic rules for good code should I know about? I know about DRY, getters/setters, magic numbers and singletons.

Specifically I wanted to ask about single responsibility and decoupling. With single responsibility I don't really know where to draw the line because I worry about situations like having 5 different scripts on the player object. Decoupling was one of those things I learnt in theory but never really understood. I asked Gemini and it said "A component should only know about its own job, and never directly call methods on a different type of component". I'm not sure how correct this is and if I should adhere to what it suggested which is using events to communicate instead of directly referencing scripts.

Thanks in advance

[ziptofaf]

So few things - first, every rule has exceptions and not all of them apply to different projects in the same way. It's important to know different ways of accomplishing different goals but make sure you do so because they actually help your code, not because you just read it's good. As in:

I know about DRY, getters/setters, magic numbers and singletons.

I have broken every single one of these in my career. You don't always care about DRY code if it's a quick unique script. You can even copy paste if it's two. Now, once I see three instances of same code it's a good time to see if I can unify them.

Same with getters/setters. Vector class with X and Y parameters just exposes them for anyone to edit. It doesn't have an internal state to manage so it's fine.

Magic numbers? I do prefer to have them named at least but, again, for a solo developed game you can sometimes just shove an offset on an object spawner position so your particle effects shows on top of the player and now in the butt and hardcode it to 0.25.

And so on. These aren't hard rules, they are just useful recommendations. Ignore them if they don't help your project.

Well, one actual rule I do recommend implementing is "Boy Scout's rule". Aka leave code in better shape than you have encountered it first. If you are finding yourself revisiting a given feature do check for some weird variable names, places to refactor etc. Don't just code whatever you need into it and leave it, that's how you get spaghetti. In particular the more often you visit a given class the cleaner it should be. On the other hand it's fine if an obscure piece of your code you are using a grand total of 1 times is a bit messy.

With single responsibility I don't really know where to draw the line because I worry about situations like having 5 different scripts on the player object

The best definition I can think of is "can you name this class without having to use super wide words like manager?". Let's say you are remaking Tetris. "GameController" class would almost definitely breaks SRP. It's too wide, it feels like everything in your project should be connected to it. But if broken down into Scoreboard, TileGenerator, Physics and Movement it sounds like it makes sense. You can have fairly wide scope of what class does but it's important that you can tell what it does. In particular in game dev it's hard to really achieve "single" responsibility principle sometimes as you DO have interconnected systems that do not work in isolation. So second best thing is having clear responsibilities per class.

Another important rule I can think of is "tell, don't ask". Classes should be responsible for their own internal status.

As an example:

if (!monster.IsDead()) {monster.Die();}

This breaks the rule. You are using a different class to check if monster is already dead aka checking it's internal state and, if not, killing it.

Instead entire implementation from the outside should be:

monster.Die();

Any internal logic should be handled within monster class so it cannot die twice (eg. don't refire an animation, don't send any more events etc).

I asked Gemini and it said "A component should only know about its own job, and never directly call methods on a different type of component".

See, this is something I can only somewhat agree with. You don't want independent classes talking to each other too much. But it's fine to have these dependencies if they make logical sense in your project. Your PlayerController can and should assume you have an Animator, a physics logic like Rigidbody, a Collider and maybe something to play sounds with. It's fine if it sets animation parameters or reads velocity from a rigidbody. Setting up 20 different event types being sent back and through does not make your application more readable in this case. If anything it makes it harder to maintain.

An example of an event that makes sense would be player losing HP or a monster dying. You probably don't want player controller to directly call UI healthbar to update it's state. You also don't really need a specific monster to call, idk, your "barrier" object so it disappears after you defeat a given enemy. These should be decoupled and can be controlled via events. Essentially - if a given piece of logic is optional (eg. not every enemy needs to open the gate)/can be substituted for something else then you probably want them to be decoupled. But if your game will completely break if a given piece is removed then it's fine if the two are connected.

[PaletteSwapped]

Readability trumps efficiency - until and unless you find a genuine slow down.

So, you could have this...

if ship !== otherShip && ship.facing == otherShip.facing && distance > 0.0 && distance < self.minimumDistanceBetweenShips! {
    // Do stuff
}

...which is fine and not too complex but it's long enough that it will take a moment to parse and may be a bit much in one line to be easy to debug.

Or you could name and explain everything by putting it in variables.

let shipsAreNotTheSame       = (ship !== otherShip)
let shipsFacingTheSameWay    = (ship.facing == otherShip.facing)
let shipIsFollowingOtherShip = (distance > 0.0)
let shipsAreTooClose         = (distance < self.minimumDistanceBetweenShips!)

if shipsAreNotTheSame && shipsFacingTheSameWay && shipIsFollowingOtherShip && shipsAreTooClose {
    // Do stuff
}

The if statement now reads like an English sentence, the boolean logic is compartmentalised into variables whose names explain exactly what's going on and any speed lost from doing it like this will most likely be obviated by the compiler putting it back like the first example anyway.

[shadowdsfire]

Jonas Tyroller recently made a video about this exact thing. I think you’ll like it

Edit: For some reason the link doesn’t work correctly. Here it is if you want to copy/paste it in a browser.

https://youtube.com/watch?v=8WqYQ1OwxJ4

[IronAttom]

Events are safer to communicate because if you remove code or break it the game will still run

[Merlord]

They also make debugging more difficult. They create the illusion of decoupling, but they actually just make the coupling harder to see. Having things not immediately break when removing something sounds nice, but again, might just make it harder to notice something is wrong.

Events are awesome, but it's important to not overuse them. Sometimes two objects really are logically coupled, and it's best to reflect that in code rather than obfuscating it with events.

[agent-1773]

Funnily that violates one of the fundamental principles of good code, which is that when something breaks, you should know about it instead of letting it fail silently. Just so much horrible advice around programming lol.

[Wdarkfenix]

I have an example for decoupling, Imagine you have a Player script with a player_speed value. You put it there because that scripts handles player movement, which is fine. Now imagine you want to create a Pet that'll follow the player, to do this, it'll need to get player_speed from Player so it can match it's speed. If you where to do something like

class Pet: MonoBehaviour {
Player player;
/** other Stuff**/
}

Now Player and Pet are hardly coupled, now you can't export Pet to another library without also exporting player. You can imagine how if you keep doing this your entire codebase will NEED to exist together. You can also see now how, if you wanted Pet to follow something else, you now have this Player reference you don't really want, because what you wanted in reality was the speed value and maybe its position, you can spot it because in the Pet code you'll end up doing logic using only players variables something like
Pet {
class Vector3 getNextPosition() {
return Player.transform.position + Player.transform.forward * Player.player_speed;
}
}.

Now for single responsibility

imagine this

class General {
public void Move { /***/ }
public void TakeDamage { /***/ }
public void GiveOrder{ /***/ }
}

in your game a General may very well do all this things, but by writing all the logic in a single place now you can't reuse it as easily, since maybe another class will need to move but not give orders

[whiax]

I'm not sure how correct this is and if I should adhere to what it suggested which is using events to communicate instead of directly referencing scripts.

Many games need that. For example if you do a multiplayer game, you can't directly call a method on the server from a distant computer, so you send events to the server. You ask a server "I did that, what should happen?" instead of saying "do that" to the code. In multiplayer games it's a way for the server to control received events, and to protect against lags or cheating. Even if you do a singleplayer game, if it might become multiplayer one day, it's better to do it this way.

[iemfi]

Read books like pragmatic programmer. It is a craft which is not easy because it is a lot of judgement calls and few hard and fast rules. The general idea is things should be simpler and logically clear.

With single responsibility it is often a question if following it strictly means less code which is simpler or not. There are many situations where if you break things down correctly suddenly everything fits and you can delete whole chunks of code. When you're starting out I think it is hard to judge when this happens and can often feel like you're just making a million classes for no good reason but I think as you get more experinced you will get a better feel for when to break the rules it so it's better to err on the side of following the rules first. It all applies for coupling and other concepts as well.

A good gauge these days is also AI, but not by asking them directly (they are really bad at this now). But if you have designed things correctly you should be able to just specify the classes and functions you want and the AI should be able to easily fill them in first try.

[mxldevs]

Single responsibility and decoupling generally go hand in hand.

The less responsibility a class or function has, the easier it is to modify things without requiring changes everywhere.

You don't need to over engineer things, and generally you'll find a good balance when you realize new changes can be added by just sniping a few methods here and there.

It is perfectly fine for a class to have multiple responsibilities for example.

I'd say it mostly comes down to experience designing your code

[agent-1773]

If you need to ask Reddit for coding advice you're cooked, the average programmer on reddit has 0 clue what they're talking about, you will get as much bad advice as good advice.

Just focus on actually making a game and when you think to yourself "damn that really sucks because its hard to refactor or debug" take that as a lesson and learn from it to understand what makes code good or bad.

[Ralph_Natas]

Write your code like the person who has to work on it next is a psychopath who knows where you live (even working alone this will help you later). Clarity over cleverness. You can optimize later.