1

I am currently making a simple console based RPG dungeon game, I am still fairly new to C#, I took a visual basic course in school, and I have played around with unity for about a month.

My problem is, that I am programming the first battle that the player will encounter. I have constructed a few weapons, and I want to be able to call the persons current weapon with a separate string, for example, My current weapon is the dagger, or the object wp1, I want my "weapon" to be attached to wp1 in some way, so that I can do something like, Console.WriteLine("Damage: " + weapon.dmg); rather than hardcoding wp1.dmg, so that later in the game making process, when the player has the opportunity to purchase a better weapon, I can do it with variables, for example, the player now has the shortsword (wp2)

private string weapon = "wp2"; ... Console.WriteLine("Damage: " + weapon.dmg);

I have tried to simply put, String weapon = wp1;

then call weapon.dmg, but this doesn't work because it thinks i'm trying to call weapon.dmg, and not wp1.dmg


//players base damage
int damage = 0;

//players strength attribute
int strength = 0;

//weapon constructor
class wp
        {
            public int dmg;
            public int cost;
            public string name;

            public wp(int d, int c, string n)
            {
                dmg = d;
                cost = c;
                name = n;
            }
        }

//three weapons that are constructed
wp wp1 = new wp(1, 25, "dg");
wp wp2 = new wp(3, 100, "ss");
wp wp3 = new wp(5, 250, "ls");

//the current weapon string
public string weapon = "wp1";

void attack()
{
   //calculates damage based off of the players damage, strength, and weapon
   int newDamage = damage * strength + (weapon.dmg);
}

Expected result: the program should use the player's current weapon's damage value

Actual result, the program tries to find the dmg value of the weapon, but that is not possible because "weapon" is just a string, so it throws an error

5
  • 5
    Don't store the current weapon as a string.. store it as the wp type. Commented Nov 1, 2019 at 19:42
  • 1
    ...then pass the current weapon (wp) object to the Attack method so the current weapon is used each time Commented Nov 1, 2019 at 19:43
  • 3
    Today is a great day as a beginner to learn good habits. Do not fall prey to "primitive obsession", particularly with strings. Beginners have a tendency to get in the habit of using basic types like strings for everything; don't. You're getting errors because this is the wrong thing to do. Commented Nov 1, 2019 at 19:45
  • 5
    While we're looking at your code: also get in the habit of naming things properly. You will not die younger if you type class Weapon instead of class wp. Classes and methods should be CasedLikeThis. Never make a public field; use a public property instead. Learn good habits now so you don't have to break them later. Commented Nov 1, 2019 at 19:46
  • 2
    Also, don't fall into bad naming habits. Give things descriptive names. wp should be Weapon. dmg should be Damage. Pretend you're reading someone else's code and see if you can reason about what things are based on their names. If you can't, you've chosen a bad name. (edit) Yeah, what Eric Lippert said. Commented Nov 1, 2019 at 19:47

2 Answers 2

6

As I said in a comment above, get into good habits now, as a beginner and you won't have to break those habits later. Let's see how we might design your system. Start off with a well-designed class hierarchy. Let's say we have three kinds of weapons: swords, daggers, and clubs. The stuff they have in common goes into an abstract base class:

abstract class Weapon
{

What do they have in common? Damage, cost, and a name. So make abstract, read-only properties for those:

    public abstract int Damage { get; }
    public abstract decimal Cost { get; }
    public abstract string Name { get; }
}

Now make some derived classes. Are you planning on extending them further? If not, seal them:

sealed class Sword : Weapon 
{
    public override int Damage => 10;
    public override decimal Cost => 12.5m;
    public override string Name => "normal sword";
}

And so on.

Now do the same for player. Let's say that we can change a player's weapon, but not their name. So Name should be a read-only property, and Weapon should be a read-write property:

sealed class Player
{
  public string Name { get; private set; }
  public Weapon Weapon { get; set; }
  public int Strength { get; private set; }
  public int BaseDamage { get; private set; }
  public Player(string name, Weapon weapon, int strength, int baseDamage)
  {
    this.Name = name;
    this.Weapon = weapon;
    this.Strength = strength;
    this.BaseDamage = baseDamage;
  }
}

Now we can make some weapons:

Weapon weapon1 = new Sword();
Weapon weapon2 = new Dagger();
Weapon weapon3 = new Club();
Player fighter = new Player("Bob", weapon3, 5, 10);

Or, better:

var weapons = new List<Weapon> { new Sword(), new Dagger(), new Club() };
// weapons is indexed 0, 1, 2.
Player fighter = new Player("Bob", weapons[2], 5, 10);

And now if you have a Player in hand:

static void Attack(Player p)
{
  int damage = p.BaseDamage * p.Strength + p.Weapon.Damage;
  string text = $"Player {p.Name} attacks with {p.Weapon.Name}";

No strings for referencing objects! Do not use strings for anything except text. References to objects should be references to objects, not strings.

Now, for advanced players only, there are times when you do need to look something up by a string. The way you do that in C# is:

var d = new Dictionary<string, Weapon> {
  { "weapon1", new Sword() },
  { "weapon2", new Dagger() },
  { "weapon3", new Club() } 
};
Weapon w = d["weapon1"]; // w is a Sword.

But do not do this by default. That's not the normal way to refer to something in C#.

Sign up to request clarification or add additional context in comments.

3 Comments

I think the dictionary's definitely an improvement over magic indices but if you're not yet an "advanced player" mapping a variable to the index number by something useful like Dagger would be better still.
Thank you very much for all of your help, I will be sure to try this stuff out.
I appreciate your answer! Your advice helped me improve, and I've been working on my skills with object oriented programming for quite some time now.
-1

one way to achieve what you want:

if(weapon=="wp1")
 //use wp1 object here.

But a Better way would be to put your 3 wp's into a list or array (since you literally hardcoded a list of 3 objects). Then wp1 would be at

wpArray[0];

1 Comment

You can access a weapons dmg from an array of wp. wpArray[0].dmg

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.