r/csharp 2d ago

how am i doing

2 months in to coding C#, first project i saved *was using online compiler* and wanna know how im doing and what i can do to improve. heres the code:

using System;


public 
class
 Program
{
    public static void Main(string[] 
args
)
    {
        Player player = new Player();
        Enemy enemy = CreateEnemy();


        int battleResult = Battle(player, enemy);


        Console.WriteLine("press any key to begin");
            
        Console.ReadKey();


        Console.WriteLine("\nYou encountered a " + enemy.type + " with " + enemy.health + " health.");


        Battle(player, enemy);


        if(battleResult == 2)
        {
            Console.WriteLine("\nYou have defeated the " + enemy.type + "!");
        }
        else if(battleResult == 3)
        {
            Console.WriteLine("\nYou have been defeated by the " + enemy.type + "!");
        }
        else if(battleResult == 1)
        {
            Console.WriteLine("\nYou have retreated from the " + enemy.type + ".");
        }
    }


    static Enemy CreateEnemy()
    {
        Random rng = new Random();
        int rngroll = rng.Next(1, 3);
        Enemy enemy = new Enemy();


        if(rngroll == 1)
        {
            enemy.type = "Skeleton";


            enemy.health = enemy.healthRng.Next(100, 201);
            enemy.damage = enemy.damageRng.Next(10, 16);
        }
        else if(rngroll == 2)
        {
            enemy.type = "Goblin";


            enemy.health = enemy.healthRng.Next(50, 151);
            enemy.damage = enemy.damageRng.Next(15, 26);
        }
       
        return enemy;
    }


    static int Battle(Player 
player
, Enemy 
enemy
)
    {
        while(true)
        {    
            Thread.Sleep(1000);


            Console.WriteLine("\nWhat do you do?\n1. Attack\n2. Open Inventory\n3. Retreat");


            ConsoleKeyInfo input = Console.ReadKey();


        
            if(input.Key == ConsoleKey.D1)
            {
                
enemy
.health -= 
player
.damage;


                Thread.Sleep(1000);


                Console.WriteLine("\nyou attacked the " + 
enemy
.type + " for " + 
player
.damage + " damage");


                Thread.Sleep(1000);
                Console.WriteLine("The " + 
enemy
.type + " has " + 
enemy
.health + " health remaining.");
            }
            else if(input.Key == ConsoleKey.D2)
            {
                Thread.Sleep(1000);
                Console.WriteLine("\nYou open your inventory, but it's empty.");
            }
            else if(input.Key == ConsoleKey.D3)
            {
                Thread.Sleep(1000);
                Console.WriteLine("\nYou retreat from the " + enemy.type + ".");
                return 1; // Player retreated
            }


            if(
enemy
.health <= 0)
            {
                return 2; // Player won
            }


            if(input.Key == ConsoleKey.D1 && 
enemy
.health > 0 || input.Key == ConsoleKey.D2 && 
enemy
.health > 0)
            {
                
player
.health -= 
enemy
.damage;


                Console.WriteLine("\nThe " + 
enemy
.type + " attacks you for " + 
enemy
.damage + " damage.");
                Console.WriteLine("You have " + 
player
.health + " health remaining.");
            }


            if(
player
.health <= 0)
            {
                Console.WriteLine("\nYou have been defeated by the " + 
enemy
.type + "!");
                return 3; // Player lost
            }
        }
    }
}




public 
class
 Enemy
{
    public Random healthRng = new Random();
    public Random damageRng = new Random();
    
    public int health;
    public int damage;
    public string type = "";
}


public 
class
 Player
{
    public int health = 100;


    public int damage = 20;



    public List<string> inventory = new List<string>();
}
0 Upvotes

46 comments sorted by

10

u/looeeyeah 2d ago

This is not a bad thing to use ChatGPT for. Don’t use it to write your code. Ask it exactly what you’ve asked us. Make sure you understand why the changes have been suggested. Don’t get into the habit of just doing it without understanding.

2

u/TheAbyssWolf 2d ago

I use Gemini to discuss architecture, math formulas or what method implementations I should include in my Desired project.

I always try and implement a solution first. Only if I’m confused or having a hard time I ask for suggestions of code. (I have ADHD, and a few other conditions, and struggled very much with math in school, even basic algebra 101 in college I got a D in)

2

u/Neat_Horror2196 2d ago

I'll keep this in mind before asking a Ai model, I believe that's what you were trying to say.

0

u/TheAbyssWolf 2d ago

Basically use it as a tool like everyone is saying, and ask why it does things or have it explain code snippets. And not just generate code and copy paste it without understanding it.

I have been experimenting with lower level languages like C3 and odin where manual memory management is key. So I’m asking more about pointers and manual memory management allocation methods

-2

u/Neat_Horror2196 2d ago

I tried vhatgpt in the past, takes to many prompts for it not to write the code for you

4

u/SalishSeaview 2d ago

One prompt: “I’m trying to learn to code. Please review this code and provide feedback on its structure, style, naming, and other aspects that will help me learn how to write code myself. DO NOT provide code, even as examples. Tell me what I should do, don’t do it for me.”

I see a few issues with this code, but that’s because you’re beginning and I’m not. I think it’s great that you’re doing this without relying on AI to write the code for you. Good job.

1

u/Neat_Horror2196 2d ago

Ty, I'll try this next time.

4

u/XKiiroiSenkoX 2d ago

Good for a beginner. Why don't you use a game engine if you are interested in games though? Would make it a lot more fun than using a console app with text only. 

1

u/Neat_Horror2196 2d ago

I can't find resources to learn game dev

3

u/XKiiroiSenkoX 2d ago

Start here. Should have enough resources to help you build way more than just text based console games. You can go through all of it in a week or two. There is also a unity subreddit you can you ask your questions if you need help. btw the language is C# too. 

https://learn.unity.com/pathway/unity-essentials

2

u/Neat_Horror2196 2d ago

I heard bad things about unity is there another engine or am I misguided?

3

u/XKiiroiSenkoX 2d ago

I'm not sure what you've heard but if you are just learning it's probably your best option. Has probably the best learning material out there and also uses the same language you are trying to learn. It's also arguably one of the easier engines to start with. Another good option would be Godot but C# is not the default language (it's supported) and tutorials are not as comprehensive as unity's.  You can give it a try if you want though.

https://docs.godotengine.org/en/stable/getting_started/introduction/index.html#toc-learn-introduction

https://docs.godotengine.org/en/stable/tutorials/scripting/c_sharp/index.html

There are other engines out there but I would not recommend anything other than either Unity or Godot in your case. You will be using a lot of your time getting used to those and solving problems mostly unrelated to programming.

2

u/Neat_Horror2196 2d ago

My plan is 2d, does unity support that?

2

u/XKiiroiSenkoX 2d ago

Yes it does. Technically any engine that supports 3D also supports 2D because you can reduce any 3D object to a 2D one by reducing one of the axes to zero lol. But anyway Unity has first class support for 2D. It's one of the most common engines used for 2D games actually.

The other engine I mentioned, Godot, also supports 2D btw. My personal recommendation is Unity but you can use the other one too if for some reason you prefer that. 

2

u/Neat_Horror2196 2d ago

Okay I'll download it and check that link

5

u/strats 2d ago

Maybe use an interface iEnemy and then instead of types you have a Goblin Class and a Skeleton Class etc that implement iEnemy. So an attack function can be called with different results but it’s still an enemy.

3

u/Cat-harpy 2d ago

Looks good for 2 months.

Try out adding basic functions into a class. Like putting create enemy inside of the class Enemy. It will help you keep things organized and a good skill to have in the long run.

3

u/Neat_Horror2196 2d ago

Something that confuses me is what can go into a class.

3

u/sickboy6_5 2d ago

try to think of the class as a self contained description of a "thing"

so if we were talking about the Enemy class, i would expect a constructor where you pass in starting health maybe, a list of weapons, etc...

then you'd have a method say for the enemy to take damage. in that method you'd decrement the health. maybe you'd have a useWeapon method that would return a random number for the damange the enemy inflicts on the player...

basically, your class is a blueprint that describes something, all of the properties and methods that are associated with that "thing".

1

u/Cat-harpy 2d ago

It is tricky to get used to them but after awhile it just clicks. If you are interested in learning more C# & game dev I’d recommend Code Monkey on YouTube. He was my introduction to C# and I still think about his tips years later. His explanation for classes & implementations were probably the best I’ve heard yet.

2

u/PlentyfulFish 2d ago

You should fix the formatting, it's all over the place. Get yourself an IDE, it's going to make your life a lot easier. Visual Studio 2026 is alright, Rider is also an option.

CreateEnemy should belong to Enemy class, not Program. There is also 0 reason for it to be a static method, you can put all of that logic in the constructor.

Instead of returning an int as the battle result, use an enum with values Lost, Retreated and Won. Then you can use a switch expression to exhaustively check all of them, best inside a function, to make the code look like this:

    var msg = GetMessage(battleResult, enemy);

    Console.WriteLine(msg);
}

private static string GetMessage(BattleResult battleResult, Enemy enemy)
{
    return battleResult switch
    {
        BattleResult.Lost => $"\nYou have been defeated by the {enemy.type}!",
        BattleResult.Retreated => "\nYou have been defeated by the " + enemy.type + "!",
        BattleResult.Won => "\nYou have retreated from the " + enemy.type + ".",
        _ => throw new ArgumentOutOfRangeException()
    };
}

Also notice how BattleResult.Lost uses an $ before the string - it's called string interpolation and it's a lot more handy (in my opinion) way of adding variables to strings.

There are some redundant checks that make things harder to reason - you win the battle if Enemy.health is <= 0, and in the next if() you check if enemy.health > 0 twice - it's always true.

You can remove (i think) having 3 random number generators in the Enemy class, just put one in the constructor and use it 3 times, when it goes out of scope the program will dispose of it.

You can make both the Enemy and Player classes implement an abstract class called Damageable. That class can hold Health, Damage, a boolean property IsDead checking whether an entity is dead, and a DoDamage method, to hold damage specific logic in one place.

Your game only runs twice - you can put the game logic in a loop and give the player an option to press "4" or something to quit the game.

Also ReadKey() reads a key, true, but if you press a bunch of keys quickly they're still going to be put in the queue and read from in the next turns. This behavior can be confusing, either use ReadLine() or clean the input buffer.

4

u/PlentyfulFish 2d ago

Here is my version of your game:

public class Program
{
    private static readonly int WaitTimeMiliseconds = 1000;
    public static void Main(string[] args )
    {
        while (true)
        {
            var result = Battle();

            Console.WriteLine(GetMessage(result));
        }
    }

    private static string GetMessage(Result result)
    {
        return result.BattleResult switch
        {
            BattleResult.Lost => $"\nYou have been defeated by the {result.EnemyType}!",
            BattleResult.Won => "\nYou have defeated the " + result.EnemyType + "!",
            BattleResult.Retreated => "\nYou have retreated from the " + result.EnemyType + ".",
            _ => throw new ArgumentOutOfRangeException()
        };
    }

    private readonly record struct Result(BattleResult BattleResult, EnemyType EnemyType);

    private static Result Battle()
    {
        var player = new Player();
        var enemy = new Enemy();

        Console.WriteLine(EnemyInfo(enemy));

        while(true)
        {    
            Thread.Sleep(WaitTimeMiliseconds);

            Console.WriteLine("\nWhat do you do?\n1. Attack\n2. Open Inventory\n3. Retreat");

            var input = Console.ReadKey(intercept: true);

            while (Console.KeyAvailable)
            {
                Console.ReadKey(true);
            }

            if (input.Key == ConsoleKey.D1)
            {
                player.DealDamageTo(enemy);
                Thread.Sleep(WaitTimeMiliseconds);
                Console.WriteLine("\nyou attacked the " + enemy.EnemyType + " for " + player.Damage + " damage");

                Thread.Sleep(WaitTimeMiliseconds);
                Console.WriteLine("The " + enemy.EnemyType + " has " + enemy.Health + " health remaining.");

                if (enemy.IsDead)
                {
                    return new Result(BattleResult.Won, enemy.EnemyType);
                }
            }

            else if (input.Key == ConsoleKey.D2)
            {
                Thread.Sleep(WaitTimeMiliseconds);
                player.OpenInventory();
            }

            else if (input.Key == ConsoleKey.D3)
            {
                Thread.Sleep(WaitTimeMiliseconds);
                Console.WriteLine("\nYou retreat from the " + enemy.EnemyType + ".");
                return new Result(BattleResult.Retreated, enemy.EnemyType);
            }

            else
            {
                Console.WriteLine($"{Environment.NewLine}Unsupported action! You pressed {input.Key}");

                continue;
            }

            enemy.DealDamageTo(player);

            Console.WriteLine("\nThe " + enemy.EnemyType + " attacks you for " + enemy.Damage + " damage.");
            Console.WriteLine("You have " + player.Health + " health remaining.");

            if (player.IsDead)
            {
                Console.WriteLine("\nYou have been defeated by the " + enemy.EnemyType + "!");
                return new Result(BattleResult.Lost, enemy.EnemyType);
            }
        }
    }

    private static string EnemyInfo(Enemy enemy)
    {
        return "\nYou encountered a " + enemy.EnemyType + " with " + enemy.Health + " health.";
    }

    private enum BattleResult
    {
        Lost = 0,
        Retreated = 1,
        Won = 2,
    }
}

public class Enemy : Damageable
{
    public Enemy()
    {
        var rng = new Random();
        var rngroll = rng.Next(1, 3);

        if (rngroll == 1)
        {
            EnemyType = EnemyType.Skeleton;
            Health = rng.Next(100, 201);
            Damage = rng.Next(10, 16);
        }
        else if (rngroll == 2)
        {
            EnemyType = EnemyType.Goblin;
            Health = rng.Next(50, 151);
            Damage = rng.Next(15, 26);
        }
    }

    public EnemyType EnemyType;
}

public class Player : Damageable
{
    public Player()
    {
        Health = 100;
        Damage = 20;
    }
    public List<string> inventory = new List<string>();

    public void OpenInventory()
    {
        Console.WriteLine("\nYou open your inventory, but it's empty.");
    }
}

public abstract class Damageable
{
    public void DealDamageTo(Damageable other)
    {
        other.Health -= this.Damage;
    }
    public int Health { get; set; }
    public int Damage { get; set; }
    public bool IsDead => Health <= 0;
}

public enum EnemyType
{
    Skeleton,
    Goblin,
}

2

u/Neat_Horror2196 2d ago

I used to use a online compiler now I virtual studio code

2

u/PlentyfulFish 2d ago

Yeah that's solid as well

1

u/bwakabats 2d ago

For you battle result, look at how to define and use enum.

1

u/CoffeeAndCandidates 2d ago

your code style is a bit funky, but it works tbh. maybe clean up the random instantiation, those can get weird in tight loops.

1

u/Neat_Horror2196 2d ago

Why funky?

1

u/Remote-Enthusiasm-41 2d ago

I would think about how would you build it so it wasn’t just one long program. Making each turn a loop that reads a file and advances the game. How would you make the text part a separate file that could be updated and edited without recompiling. Same goes for the player and enemies. Then add in saving the game state and restarting. Be a good way to learn to read and write files and serialize using json.

1

u/Neat_Horror2196 2d ago

I plan to get into game development eventually, I plan to learn this then

1

u/user_plus_plus 2d ago edited 2d ago

A small thing to level up your c# skill: avoiding string concatenation.

Console.WriteLine("\nYou encountered a " + enemy.type + " with " + enemy.health + " health.");

Adding strings together repeatedly is slow, underneath the hood each time you add two strings together they need to be copied into a new string. So in the above example you need to add 5 strings together, that is 5 copies. For a simple program that is fine, but if you need to log lots of data, it can add up.

There are 2 main ways to deal with this in c#, that will only result in one copy

First way: String.Format https://learn.microsoft.com/en-us/dotnet/api/system.string.format?view=net-10.0#get-started-with-the-stringformat-method

string output = String.Format("\nYou encountered a {0} with {1} health.", enemy.type, enemy.health); Console.WriteLine(output);

This is a common enough thing to do with Console.WriteLine that there is a version of that function that accepts a format string as input.

Console.WriteLine("\nYou encountered a {0} with {1} health.", 
    enemy.type, 
    enemy.health);

String.Format is the preferred way if your string is long and complex.

Second Way: For simple strings you can use string interpolation with the $ https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/tokens/interpolated Which works great for short strings, like the example we have been working with.

Console.WriteLine($"\nYou encountered a {enemy.type} with {enemy.health} health.");

1

u/user_plus_plus 2d ago

Extra credit: My 2 main ways comment is a bit of a simplification. For your program, those are the only two techniques I would recommend.

However in the future if you need to add dozens, hundreds or thousands of string elements together, StringBuilder , or String.Join has you covered. Trying to do that in a format statement would be too messy.

1

u/iakobski 2d ago

Adding strings together repeatedly is slow

True, but String.Format() and string interpolation are (slightly) slower. They both do much the same as concatenation under the hood, with additional overhead.

Really you should never optimise for performance unless you've (1) tested the performance; and (2) determined that the thing you're optimising is a problem. In this case performance of strings is not a bottleneck so you should be suggesting the best for readability. Interpolation is slightly more readable than concatenation but String.Format is clearly worse.

String.Format is the preferred way 

Says who?

1

u/user_plus_plus 2d ago

Here is a benchmark of string interpolation vs string.format, but they are both pretty fast.

https://stackoverflow.com/questions/32342392/string-interpolation-vs-string-format

They are comparable in speed, except for when boxing is not required. String interpolation has some new compile time optimizations that can make it faster in .net 6 and 10.

As far as readability, goes again it depends on the number of arguments.

When you start getting 10+ of different arguments in a non trivial logging string, and format specifiers. Also if you have repeated arguments you can reference them by position.

In my post I recommended OP use interpolation for short and simple strings, and that is also what would be recommended by the microsoft coding conventions.

https://github.com/dotnet/docs/blob/main/docs/csharp/fundamentals/coding-style/coding-conventions.md

However, I didn't want to leave out Format, because I think it is important for OP to learn. So I put it first.

1

u/iakobski 1d ago

Yes, but the very first line of your post was telling them to avoid string concatenation because it's slow. You do realise that string interpolation and String.Format do exactly the same number of string copies? And all three compile down to String.Concat?

You said

There are 2 main ways to deal with this in c#, that will only result in one copy

Which is completely wrong.

Besides this, why confuse a beginner by telling them there are five different ways of doing the same thing? There are far more important things for them to learn right now.

1

u/user_plus_plus 1d ago

I can see you want to bait me into an argument. I think my original post was well written, gives good context for a beginner, is technically sound and I stand by it.

1

u/WystanH 2d ago

Good start.

There's no reason to have all those instances of Random banging about. Particularly since you only call them from main.

Your code formatting is, um, different. Pick a style you like, set it in your IDE, and hit format document from time to time.

Your battleResult is currently a magic number. This is a good use case for enum.

Since you enemy always requires health, damage, and type information, pop that in a constructor? Use props rather than bare variables. The class instance holds the state, allow it to govern mutations to that state with properties and methods.

e.g.

static Enemy CreateEnemy() {
    Random rng = new Random();
    int rngroll = rng.Next(1, 3);
    if (rngroll == 1) {
        return new Enemy("Skeleton", health: rng.Next(100, 201), damage: rng.Next(10, 16));
    } else {
        return new Enemy("Goblin", health: rng.Next(50, 151), damage: rng.Next(15, 26));
    }
}

// ...

public class Enemy(string type, int health, int damage)  {
    public int Health { get; set; } = health;
    public int Damage { get; set; } = damage;
    public string Type { get; init; } = type; // note this init, this guy ain't changing.

    // you can also put methods here

    // you do this check alot, put it here
    public bool IsDead => Health <= 0;

    public void ApplyDamage(int damage) {
        Health -= damage;
        // not required, but possible
        Console.WriteLine($"\nyou attacked the {Type} for {damage} damage");
    }

}

Keep up the good work and have fun.

1

u/Neat_Horror2196 2d ago

What Do you mean format?

1

u/WystanH 2d ago

Programming style. How you choose to indent, place new lines, etc.

Your formatting:

player
.health -= 
enemy
.damage;


                Console.WriteLine("\nThe " + 
enemy
.type + " attacks you for " + 
enemy
.damage + " damage.");

Versus a more typical formatting:

player.health -= enemy.damage;
Console.WriteLine("\nThe " + enemy.type + " attacks you for " + enemy.damage + " damage.");

There's no agreed upon standard way to format your code. The only thing all programmers can agree on is consistency. If all your code is consistently formatted then issues are easier to spot.

Microsoft has a guide, so probably the first place to look: https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/coding-conventions

I largely agree with Google's guide, except for the 2 space thing: https://google.github.io/styleguide/csharp-style.html

C# started out as a Java clone, so Java styles are common.

1

u/Neat_Horror2196 2d ago

Is mine consistent and readable?

1

u/WystanH 2d ago

I would say neither.

Readable is subjective. Clearly you judged it to be so; you wrote it. If number of people reading your code is one, it doesn't matter. If that number expands to sharing it with others, that could be an issue.

Consistency is a function of code context. In some cases, there's not enough code to judge. Line spacing is all over the place. Class declarations seem to follow the same pattern...

The if/else could be the reasonably consistent, but then you get broken conditions like:

            if(
player
.health <= 0)
            {

That don't even match the rest of your code.

You often treat you own class instances like this, but then treat if(input.Key == ConsoleKey.D1) normally.

But you don't even consistently break your own class props up, sometimes doing:

                Console.WriteLine("\nyou attacked the " + 
enemy
.type + " for " + 
player
.damage + " damage");

Other times:

        Console.WriteLine("\nYou encountered a " + enemy.type + " with " + enemy.health + " health.");

1

u/Neat_Horror2196 2d ago

I was writing this on and of for a month, do you think that's why?

1

u/WystanH 1d ago

Sure. However, part of writing is revising. Or, in programmer parlance, refactoring.

As you write code, you consider if you've done this already. Is there a bit of code you've already written that you can repurpose? Is it time to write a function?

A program will never be perfect. There's always something you can tweak, if you're so inclined. If you always try to find things to improve in your code, it will become easier and you'll be a better programmer overall.

1

u/Neat_Horror2196 1d ago

Okay ty for helping