Sign in to follow this  
FlawleX

My first C# game - Feedback

Recommended Posts

Hello!

I just completed my first text based C# game! Well i still have to come up with good names for the boss attacks but oh well.
Anyways the game is in it's final stage and i hope to get some feedback here if anyone would take their time to even read the code.

It's a text based game so it's nothing fancy and its really repetetive. I just want to see if i have made any obvious dumb mistakes :)

The main file: (Note that im using different .cs files to store boss data and such so they are not included simply because they are so simple i don't think they need checking. and to be honest everything is the same after the first boss is dead so if you could give me feedback on the first bit i would be very happy!)

[url="http://pastebin.com/vVm0HtEk"]http://pastebin.com/vVm0HtEk[/url]

Share this post


Link to post
Share on other sites
A few suggestions from a skim:

1. I think there's a lot of data that would be best factored out somewhere, if only to make the code easier to read. This might just be a flat text file for the script, for example, where you can just look up "story_intro" and get the text starting "The Story: ..." back without it being embedded. It might be an interesting exercise to look at writing some code to auto format/decorate that text e.g. with your hashes down the sides for a specified console width.

2. I would prefer to use a single Console.WriteLine with the string split over multiple lines. If you had the auto-formatter that I mentioned, then you could just do something like Console.WriteLine(Resources.GetFormattedString("intro_story")). Other alternatives would be to use a verbatim string literal (prefix with @) or just concatenate them together with +. (I'm sure the compiler is smart enough to do these concatenations at compile time if the strings are known.)

3. There's quite a lot of duplication of structure (e.g. *UserDMG, *Damage, *Ability) and code (e.g. the big switch statement) - this would be better refactored into various classes and functions. This will make your code easier to read, as well as much easier to modify. Things seem a bit confusing, e.g. greenscale.greenscaleAttackPower. Why isn't this just greenscale.attackPower?

4. What is the purpose of the userInformation abstract class? (Why is it abstract?)

5. I find a naming convention that distinguishes types and variables (e.g. MyType myTypeInstance) easier to read. Personal preference, though!

Share this post


Link to post
Share on other sites
Hi,
the first obviouse mistake, without reading one line of code, is that it's just one file. This way it is almost impossible to get the structure if you haven't coded it yourself and even you will probably get problems if you look at it after one month not working on it.

It should be possible to find a way to seperate those over 2000 lines of code into multiple files. Also it would be nice if you would provide a runnable version, so that someone can run the code he is looking at and actually see what it is supposed to do.

Anyway, from what I've read so far there are no errors in what you are coding, it's just not "the best" way to do it. You should have a look into functions, and, if you want to be fancy and push your game to the next level, into state machines as well.
I think this will teach you the basics pretty well:
[url="http://www.programmingbasics.org/en/fsm.html"]http://www.programmi...org/en/fsm.html[/url]
I hope this helps you!

Share this post


Link to post
Share on other sites
[quote name='TheUnbeliever' timestamp='1318276281' post='4871180']
A few suggestions from a skim:

1. I think there's a lot of data that would be best factored out somewhere, if only to make the code easier to read. This might just be a flat text file for the script, for example, where you can just look up "story_intro" and get the text starting "The Story: ..." back without it being embedded. It might be an interesting exercise to look at writing some code to auto format/decorate that text e.g. with your hashes down the sides for a specified console width.

2. I would prefer to use a single Console.WriteLine with the string split over multiple lines. If you had the auto-formatter that I mentioned, then you could just do something like Console.WriteLine(Resources.GetFormattedString("intro_story")). Other alternatives would be to use a verbatim string literal (prefix with @) or just concatenate them together with +. (I'm sure the compiler is smart enough to do these concatenations at compile time if the strings are known.)

3. There's quite a lot of duplication of structure (e.g. *UserDMG, *Damage, *Ability) and code (e.g. the big switch statement) - this would be better refactored into various classes and functions. This will make your code easier to read, as well as much easier to modify. Things seem a bit confusing, e.g. greenscale.greenscaleAttackPower. Why isn't this just greenscale.attackPower?

4. What is the purpose of the userInformation abstract class? (Why is it abstract?)

5. I find a naming convention that distinguishes types and variables (e.g. MyType myTypeInstance) easier to read. Personal preference, though!
[/quote]

Thanks for all the feedback! I'll take a look into it and change stuff around!

Share this post


Link to post
Share on other sites
[quote name='bonus.2113' timestamp='1318276334' post='4871182']
Hi,
the first obviouse mistake, without reading one line of code, is that it's just one file. This way it is almost impossible to get the structure if you haven't coded it yourself and even you will probably get problems if you look at it after one month not working on it.

It should be possible to find a way to seperate those over 2000 lines of code into multiple files. Also it would be nice if you would provide a runnable version, so that someone can run the code he is looking at and actually see what it is supposed to do.

Anyway, from what I've read so far there are no errors in what you are coding, it's just not "the best" way to do it. You should have a look into functions, and, if you want to be fancy and push your game to the next level, into state machines as well.
I think this will teach you the basics pretty well:
[url="http://www.programmingbasics.org/en/fsm.html"]http://www.programmi...org/en/fsm.html[/url]
I hope this helps you!
[/quote]

Thanks alot! And yes indeed my code isn't easy to read. I'll make sure that is split the code more for my next project. Even i have problems navigating in the code :) But again it's my first project so im not that good at structuring everything correctly.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this