6
\$\begingroup\$

I wanted your opinion on the first RPG game I made in C#. What things can I improve on with the code?

Events:

static class Events
{
    public static void SpawnWeapon(string weapon_name)
    {
        Console.WriteLine("\nAfter that fight you looted a weapon from the enemy party: {0}", weapon_name);
        Player.EquipWeapon(weapon_name);
    }

    public static void LootHerbs(Player player)
    {
        Console.WriteLine("\nYou also managed to loot some herbs that you will use to cure you! You gained 3 HP.");
        player.HP += 3;
    }

    public static void LootSpecialHerbs(Player player)
    {
        Console.WriteLine("\nYou also managed to loot some special herbs that you will use to cure you! You gained 5 HP.");
        player.HP += 5;
    }

    public static void LootMedicine(Player player)
    {
        Console.WriteLine("\nYou looted some medicine that you will use to cure you! You gained 5 HP.");
        player.HP += 5;
    }

    public static void LootHighGradeMedicine(Player player)
    {
        Console.WriteLine("\nAnd you also managed to loot some high grade medicine that you will use to cure you! You gained 7 HP.");
        player.HP += 7;
    }

    public static void GodBlessing(Player player)
    {
        Console.WriteLine("\nAs you were walking towards the dragon nest, you suddenly hear a angelical voice behind you\n" +
            "and your wounds magically heal. You were blessed by a god to finish your journey! You gained 12 HP.");
        player.HP += 12;
    }
}

Battle:

static class Battle
{
    public static int crit_chance = 16; // 1 in 15 attacks is a crit. ( Yes, 15 ). 
    public static int attack_timer = 1500;

    public static bool isCrit()
    {
        Random rnd = new Random();
        int crit = rnd.Next(1, crit_chance);

        if (crit == 1)
        {
            return true;
        }
        return false;
    }

    public static bool isPlayerDead(Player player)
    {
        if (player.HP <= 0)
        {
            return true;
        }
        return false;
    }

    public static bool isMobDead(Mob mob)
    {
        if (mob.HP <= 0)
        {
            return true;
        }
        return false;
    }

    public static void PlayerAttack(Player player, Mob mob)
    {
        bool was_crit = false;

        int DamageInflicted()
        {
            int damage = player.Attack() - mob.Defense();
            if (damage <= 1)
            {
                damage = 1;
            }
            if (isCrit() == true)
            {
                was_crit = true;
                damage += damage;
            }
            return damage;
        }

        mob.HP -= DamageInflicted();

        if (was_crit == true)
        {
            Console.WriteLine("It was a critical hit! You dealt {0} damage.", DamageInflicted());
        }
        else
        {
            Console.WriteLine("You dealt {0} damage.", DamageInflicted());
        }
        Console.WriteLine("The enemy has {0} HP.", mob.HP);
    }

    public static void EnemyAttack(Player player, Mob mob)
    {
        bool was_crit = false;

        int DamageInflicted()
        {
            int damage = mob.Attack() - player.Defense();
            if (damage <= 0)
            {
                damage = 0;
            }
            if (isCrit() == true)
            {
                was_crit = true;
                damage += damage;
            }
            return damage;
        }

        player.HP -= DamageInflicted();

        if (was_crit == true)
        {
            Console.WriteLine("It was a critical hit! The enemy dealt {0} damage.", DamageInflicted());
        }
        else
        {
            Console.WriteLine("The enemy dealt {0} damage.", DamageInflicted());
        }
        Console.WriteLine("You have {0} HP.", player.HP);
    }

    public static void MakeBattle(Player player, Mob mob)
    {
        Console.WriteLine("You, {0}, will now fight the {1} {2}.", player.name, mob.specie, mob.name);
        mob.Stats();
        Console.WriteLine("Press Enter to begin the fight.");
        Console.ReadLine();
        Random rnd = new Random();
        int move = rnd.Next(1, 3);
        if (move == 1)
        {
            Console.WriteLine("You begin!");
            while (true)
            {
                Console.WriteLine();
                System.Threading.Thread.Sleep(attack_timer);
                PlayerAttack(player, mob);
                if (isMobDead(mob))
                {
                    Console.WriteLine("You killed the {0}!", mob.specie);
                    Console.ReadLine();
                    Console.Clear();
                    break;
                }

                Console.WriteLine();
                System.Threading.Thread.Sleep(attack_timer);
                EnemyAttack(player, mob);
                if (isPlayerDead(player))
                {
                    Console.WriteLine("You died! Game Over!");
                    break;
                }
            }
        }
        else
        {
            Console.WriteLine("The enemy will begin!");
            while (true)
            {
                Console.WriteLine();
                System.Threading.Thread.Sleep(attack_timer);
                EnemyAttack(player, mob);
                if (isPlayerDead(player))
                {
                    Console.WriteLine("You died! Game Over!");
                    break;
                }

                Console.WriteLine();
                System.Threading.Thread.Sleep(attack_timer);
                PlayerAttack(player, mob);
                if (isMobDead(mob))
                {
                    Console.WriteLine("You killed the {0}!", mob.specie);
                    Console.ReadLine();
                    Console.Clear();
                    break;
                }
            }
        }
    }
}

Mob:

class Mob
{
    public string specie;
    public string name;
    public int base_attack = 2;
    public int base_defense = 1;
    public int HP;

    public int mob_defense;
    public int mob_attack;

    public Mob(string aSpecie, string aName, int aHP, int aAttack, int aDefense)
    {
        specie = aSpecie;
        name = aName;
        HP = aHP;
        mob_attack = aAttack;
        mob_defense = aDefense;
    }

    public int Attack()
    {
        return (base_attack + mob_attack);
    }

    public int Defense()
    {
        return (base_defense + mob_defense);
    }

    public void Stats()
    {
        Console.WriteLine("The {0} {1}'s Stats:", specie, name);
        Console.WriteLine("HP: {0}", HP);
        Console.WriteLine("Attack: {0}", Attack());
        Console.WriteLine("Defense: {0}", Defense());
    }
}

Story:

static class Story
{
    public static void S_1_Bandits(Player player)
    {
        Console.WriteLine("You are " + player.name + ", who's on his way to kill the dragons which are destroying the kingdom.");
        Console.WriteLine("As you are on your way to the lairs of the dragons, you run into a couple of bandits.");
        Console.WriteLine("And they don't seem so friendly...");
        Console.ReadLine();
        Console.Clear();
    }

    public static void S_2_Knights()
    {
        Console.WriteLine("The bandits weren't much of a match for you. Well Done! You continue on to the dragons lair!");
        Console.WriteLine("However, a new movement has risen that wants to protect the dragons of the world.");
        Console.WriteLine("Many people have joined this movement, including some knights.");
        Console.WriteLine("And uh oh, 3 of them have found out about your quest...");
        Console.WriteLine("Maybe they're friendly?");
        Console.ReadLine();
        Console.WriteLine("Nope.");
        Console.ReadLine();
        Console.Clear();
    }

    public static void S_3_Dragons()
    {
        Console.WriteLine("With the knights defeated you continue on your journey!");
        Console.WriteLine("After a while you make it to the lair of dragons...");
        Console.WriteLine("It's hot and little smokey in there. You put your sight on the pair of dragons in the center of it.");
        Console.WriteLine("The time has come to end the dragons rampage!");
        Console.ReadLine();
        Console.Clear();
    }

    public static void S_End(Player player)
    {
        Console.WriteLine("You killed the dragons and saved the kingdom!");
        Console.WriteLine("Your name will be remembered forever and legends will be told about the hero {0} that defeated the evil dragons!", player.name);
        Console.WriteLine("Congrats!");
    }
}

Weapon:

class Weapon
{
    public string name;
    public int attack;
    public int defense;

    public static List<Weapon> weapon_list = new List<Weapon>();

    public Weapon(string aName, int aAttack, int aDefense)
    {
        name = aName;
        attack = aAttack;
        defense = aDefense;

    }

    public static void CreateWeapon( string name, int attack, int defense)
    {
        Weapon weapon = new Weapon(name, attack, defense);
        weapon_list.Add(weapon);
    }

    public static void CheckAllAvailableWeaponsStats()
    {
        Console.WriteLine("\nAll weapons in the game:");
        foreach (Weapon weapon in Weapon.weapon_list)
        {
            Console.Write("Name: {0}\nAttack: {1}\nDefense: {2}\n", weapon.name, weapon.attack, weapon.defense);
            Console.WriteLine("---------------------------");
        }
    }

    public static void CheckWeaponStats(Weapon weapon)
    {
        Console.Write("\nName: {0}\nAttack: {1}\nDefense: {2}\n", weapon.name, weapon.attack, weapon.defense);
    }

    public static void CompareWeaponStats(Weapon other_weapon, Weapon your_weapon)
    {
        if (your_weapon == Player.equipped_weapon)
        {
            Console.Write("Name: {0} | Equipped Weapon Name: {1}\nAttack: {2} | Equipped Weapon Attack: {3} \nDefense: {4} |" +
                " Equipped Weapon Defense: {5}\n", other_weapon.name, your_weapon.name, other_weapon.attack, your_weapon.attack, other_weapon.defense, your_weapon.defense);
        }
        else
        {
            Console.Write("Other Weapon Name: {0} | Your Weapon Name: {1}\nOther Weapon Attack: {2} | Your Weapon Attack: {3} \nOther Weapon Defense: {4} " +
                "| Your Weapon Defense: {5}\n", other_weapon.name, your_weapon.name, other_weapon.attack, your_weapon.attack, other_weapon.defense, your_weapon.defense);
        }
    }
}

Player:

class Player
{
    public static Weapon initial_sword = new Weapon("Initial Sword", 8, 4);

    public string name;
    public int HP;
    public int level = 0;
    public static Weapon equipped_weapon = initial_sword;

    public int base_attack = 4;
    public int base_defense = 2;

    public Player(string aName, int aHP)
    {
        name = aName;
        HP = aHP;
    }

    public int Attack()
    {
        return (base_attack + equipped_weapon.attack);
    }

    public int Defense()
    {
        return (base_defense + equipped_weapon.defense);
    }

    public void Stats()
    {
        Console.WriteLine("\n{0}'s Stats:", name);
        Console.WriteLine("HP: {0}", HP);
        Console.WriteLine("Attack: {0} ({1})", Attack(), base_attack);
        Console.WriteLine("Defense: {0} ({1})", Defense(), base_defense);
        Console.WriteLine("Equipped Weapon: {0}; AT: {1}; DF: {2}", equipped_weapon.name, equipped_weapon.attack, equipped_weapon.defense);
    }

    public static bool QuestionPrompt()
    {
        string[] yes_list = { "yes", "", "sure", "y", "yeah", "why not", "yes.", "y." };

        Console.Write("-->");
        string input = Console.ReadLine();
        string iinput = input.ToLower();
        foreach (string value in yes_list)
        {
            if (value.Equals(iinput))
            {
                return true;
            }
            else
            {
                continue;
            }
        }
        return false;
    }

    public static void ChangeWeaponByName(string new_weapon_name)
    {
        Weapon weapon_to_change = new Weapon("Test Weapon", 0, 0);

        bool WeaponExists()
        {
            foreach (Weapon weapon in Weapon.weapon_list)
            {
                if (weapon.name.ToLower() == new_weapon_name.ToLower())
                {
                    weapon_to_change = weapon;
                    return true;
                }
                else
                {
                    continue;
                }
            }
            return false;
        }

        if (WeaponExists() == true)
        {
            equipped_weapon = weapon_to_change;
        }
    }

    public static void ChangeWeapon(Weapon new_weapon)
    {
        equipped_weapon = new_weapon;
    }

    public static void EquipWeapon(string weapon_name)
    {
        Weapon weapon_to_equip = new Weapon("Test Weapon", 0, 0);

        bool WeaponExists()
        {
            foreach (Weapon weapon in Weapon.weapon_list)
            {
                if (weapon.name.ToLower() == weapon_name.ToLower())
                {
                    weapon_to_equip = weapon;
                    return true;
                }
                else
                {
                    continue;
                }
            }
            return false;
        }

        if (WeaponExists())
        {
            Console.WriteLine("\nComparison of both weapons stats:");
            Weapon.CompareWeaponStats(weapon_to_equip, Player.equipped_weapon);
            Console.WriteLine("Are you sure you want to equip this weapon?");
            if (QuestionPrompt() == true)
            {
                Console.WriteLine("You equipped the weapon!");
                ChangeWeapon(weapon_to_equip);
            }
            else
            {
                Console.WriteLine("You will continue with the same weapon, the new one was discarded.");
            }
        }
        else
        {
            Console.WriteLine("The weapon you want to equip doesn't exist!");
        }
    }

    public static void CheckEquippedWeapon()
    {
        Console.WriteLine("Equipped Weapon:");
        Weapon.CheckWeaponStats(equipped_weapon);
    }
}

Main:

    static void Main()
    {
        Player player = new Player("", 30);

        bool PlayerDied()
        {
            if (Battle.isPlayerDead(player) == true)
            {
                return true;
            }
            return false;
        }

        Weapon.CreateWeapon("Rusty Old Sword", 7, 3); // initial weapon
        Weapon.CreateWeapon("Iron Sword", 10, 5); // to give the player after he fights the bandits
        Weapon.CreateWeapon("Enchanted Silver Sword", 15, 7); // to give the player after he fights the knights

        Player.ChangeWeaponByName("rusty old sword");

        // the enemies
        Mob[] bandits =
        {
            new Mob("Bandit", "Rob", 10, 6, 2),
            new Mob("Bandit Leader", "Joe", 10, 7, 2)
        };
        Mob[] knights =
        {
            new Mob("Knight", "Rob", 12, 8, 4),
            new Mob("Knight", "John", 12, 9, 3),
            new Mob("Knight Captain", "Aaron", 14, 10, 4),
        };
        Mob[] dragons =
        {
            new Mob("Blue Dragon", "Jormungandr", 16, 10, 6),
            new Mob("Dragon Leader", "Helios", 18, 11, 6),
        };

        Console.WriteLine("What's your name, adventurer?");
        string response = Console.ReadLine();
        player.name = response;

        player.Stats();
        Console.WriteLine("\nPress anything to begin the game.");
        Console.ReadLine();
        Console.Clear();

        while (true)
        {
            Story.S_1_Bandits(player); // first part of the story
            foreach (Mob mob in bandits)
            {
                Battle.MakeBattle(player, mob);
                if (Battle.isPlayerDead(player) == true)
                {
                    break;
                }
            }

            if (PlayerDied() == true)
            {
                break;
            }

            Events.SpawnWeapon("Iron Sword");
            if (player.HP <= 16)
            {
                Events.LootSpecialHerbs(player);
            }
            else
            {
                Events.LootHerbs(player);
            }
            player.Stats();
            Console.WriteLine("\nPress Enter to continue.");
            Console.ReadLine();

            Console.Clear();

            Story.S_2_Knights(); // second part of the story
            foreach (Mob mob in knights)
            {
                Battle.MakeBattle(player, mob);
                if (Battle.isPlayerDead(player) == true)
                {
                    break;
                }
            }

            if (PlayerDied() == true)
            {
                break;
            }

            Events.SpawnWeapon("Enchanted Silver Sword");
            Events.LootMedicine(player);
            if (player.HP >= 15)
            {
                Events.LootHighGradeMedicine(player);
            }
            else if (player.HP <= 13)
            {
                Events.GodBlessing(player);
            }
            else
            {
                Events.LootMedicine(player);
            }
            player.Stats();
            Console.WriteLine("\nPress Enter to continue.");
            Console.ReadLine();

            Console.Clear();

            Story.S_3_Dragons(); // third part of the story
            foreach (Mob mob in dragons)
            {
                Battle.MakeBattle(player, mob);
                if (Battle.isPlayerDead(player) == true)
                {
                    break;
                }
            }

            if (PlayerDied() == true)
            {
                break;
            }

            Console.Clear();

            Story.S_End(player);

            break;
        }

        Console.ReadLine();
    }
}
\$\endgroup\$

closed as unclear what you're asking by Zeta, t3chb0t, Mast, Graipher, Quill Dec 25 '18 at 3:07

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    \$\begingroup\$ Welcome to Code Review. That's a lot of code at once and therefore is hard to review, as reviewers are currently missing the big-picture. They only know that this is some kind of RPG, but it's not clear whether it's a text-based adventure or how to play it. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does \$\endgroup\$ – Zeta Dec 22 '18 at 8:53
7
\$\begingroup\$

Valley Girl coding

The valley girls say "for sure, for sure". so does this:

if (PlayerDied() == true)

Instead:

if(PlayerDied())

And, not this:

public static bool isMobDead(Mob mob)
{
    if (mob.HP <= 0)
    {
        return true;
    }
    return false;
}

instead:

public static bool isMobDead(Mob mob) { return mob.HP <= 0; }

and not this:

if ( weapon.name.ToLower() == weapon_name.ToLower() ) {
   return true;
} else {
   continue;
}
return false;

instead:

return ( weapon.name.ToLower() == weapon_name.ToLower() );   

Misplaced properties

Battle.isPlayerDead(player)

isDead belongs in Player class, something like this:

public bool IsDead { get { return hitPoints <= 0; } }

If a class is "calculating" something about an object using only object-class's properties then it probably belongs in that class in the first place.

Why does a Weapon have it's own weapons? If the idea is for a collection of all weapon types (and associated properties) that exist in the game then make a Weapons (plural) class. And a Player should have Weapons too.


Nmng cnvntns

You know these are bad names: HP, isCrit(), etc. Here's why: Names should be in descriptive terms of the problem domain and thus convey context and meaning. Names should help a code reader who is not you understand the program.

Do not name things using implementation details. List<Weapon> weapon_list. Instead List<Weapon> weapons.


Make state variables

while (true)

While what is true?

Once you define what it means to keep going, let's say "combatEffective", then lots of imprecise and vague code is understandable:

while( combatEffective ) {
   . . .

   if( somePlayer.IsDead ) combatEffective = false;

   . . .
}

Any number of things might cause an ineffective player and now the code unambiguously defines/declares these situations. Also the code is made more self document-y.

MOST importantly rewrite so execution falls to the end of the while block and loops. As written, this code is very error prone when adding complexity.


Structure

The above change breaks the while code, you know that of course. But it would not given a structure instead of a single mass of linear code.

combatEffective = true;
while( combatEffective) {
   if( !StoryOne() )   { combatEffective = false; }
    . . .
   if( !StoryTwo() )   { combatEffective = false; }
   . . .
   if( !StoryThree() ) { combatEffective = false; }
}

Good structure creates appropriate level abstractions. That naturally makes code changealbe and understandable. Strive for appropriate abstractions at the main() level, class, method, and even individual control structure (if, while, switch, etc.) levels.


States are undefined

First, I assume HP means hit-points.... Where ever there is something like player.HP <= 13 that must have some meaning otherwise you just felt like writing 13 at that moment; so why not 5, 256, -43? Define these states using enums or constants or something. I like enums. So "13" must be some kind of limit and being under 13 must be some kind of combat effectiveness state or something.


Confusing Player weapons

Why or how is initial-sword different from any other sword a player may have (or any other weapon for that matter)? I don't see why the weapon that a player object is instantiated with is special or different than any weapon he may get during the game.

equiped_weapon is confusing. I think you mean the weapon he is currently wielding. The things in a player's possession are the things he is "equipt with." That is to say I am equipted with a sword whether it's in my hand or strapped to my back.

If a player comes standard issue with a sword then give him a sword in the constructor, then explicitly put it in his hand for use - just the way you'd do things every where else in the program

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.