/agdg/ - Amateur Game Development General

I didn't go over all your code, but from taking a glance:

> tabs v. spaces

As much as I like pressing the tab key, you should be expanding those to spaces. Whatever editor you're using should have this feature. Also, use an indentation width of 4 spaces please. 8 is too much. 2 should be fine for some other things (like HTML), but I think standard stuff for C# is 4 spaces for each tab.

> `int x; int x2;`

Avoid this as much as possible. I can't really tell what the second variable is for. Give it a better name.

> implicit private

I try to avoid this, explicit state whether a variable is private or public.

> `Rigidbody rb;`

Try `private Rigidbody rigidbody` instead. I try to avoid variable names under 3-4 characters. I know that it's a lot more typing, but it's much better for others who have to use your code. It's find if you want to do something like `Collider` -> `Col`. Since from `Col` I know that that might be for.

> Capitalization

Generally, public fields/variables will be CamelCase with the first letter capitalized in C#, whereas private members are camelCase with the first letter lowercased.

> Code comments

Where the heck are most of them? I cannot figure out what your code is doing from a quick glace because of this. You don't need to bomb on the comments (like I see a lot of people do). A line or two for a block of code should be fine. Also comment what your variables mean at the top of your class definition.

> "b-but well written code can be the comments/documentation..."

Don't even start thinking like that. It's a terrible habbit to get into. I've worked before at many other companies with massive codebases that are sparsely commented and documented. After 8+ years of operation, they wonder why it's so hard for new hires to start.

As a general rule of thumb, try to write code as if others will have to read and use it as well. If you do some cleanup like I have listed here, I'd be willing to take another, longer, look at it.

>>23483

thanks anon, i'll rewrite it after some sleep :)

>>23483

>Where the heck are most of them? I cannot figure out what your code is doing from a quick glace because of this.

>>"b-but well written code can be the comments/documentation..."

>Don't even start thinking like that. It's a terrible habbit to get into.

Naah

You can't tell what the code does at a glance because it has three methods per 700 lines of code. Only thing you should comment is what this module/class does and maybe (maybe) some public methods if there are any caveats in using them (which probably shouldn't be there).

For example I'd split Start() into calls to GetGameObjects() and ResetParameters(), Update() into GetInput(), CalculateAirTime(), ... uhh, it's kind hard to tell what's going on because of the short variables but you get the idea. Most of the code can be self-documented.

>>23496

Self documented code is one of the most autistic programming styles out there. Document your shit.

>>23508

Just like everything else, there should be balance. Where possible, you should opt for clarity, even when sacrificing a little bit of performance.

Using tons of comments is generally a good thing, but there is a point where it is too much.

If you have a section that wouldn't be immediately obvious to a freshman programmer who is just looking at the code for the first time, you need a comment, that includes context. If to understand a portion of code, you have to have all the surrounding code committed to memory, add a comment. You should be able to come back with amnesia, look at any section of code, and know what it does and why.

Verbose, clear, and well-written comments are a rare art to stumble on, but god are they refreshing when you find them.

>>23711

1. Add a better Readme file (please tell me what I'll need to compile it).

>>23480

>a hundred fucking global variables and repeated calls

anon, have you ever considered using structs and arrays

ie

GameObject pwcm;
 
        GameObject PCstep;
        GameObject PCbase;
        GameObject PCbase2;
        GameObject PCbase3;
        GameObject Indicator;
        GameObject PChar;
        GameObject PCA;
        GameObject PCam;
        GameObject jumptext;
        GameObject playerMC;
        GameObject tgo;
        GameObject PAxis2;
        GameObject Axis2Rotation;
        GameObject Axis2baserotation;
        GameObject Slide_PS;
        GameObject OBwalljump;

and instead just storing that as an array/dict, so you can loop through it instead of stupid shit like 20 handwritten find() calls in your start() function

also what

if (leftStickAngle2 > 316 || leftStickAngle2 < 44 || leftStickAngle2 > 46 && leftStickAngle2 < 134 || leftStickAngle2 > 136 && leftStickAngle2 < 224 || leftStickAngle2 > 226 && leftStickAngle2 < 314) {

break this shit up and make it readable god damn

if (leftStickAngle2 > 316 || 
     leftStickAngle2 < 44 ||
     leftStickAngle2 > 46 && leftStickAngle2 < 134 || 
     leftStickAngle2 > 136 && leftStickAngle2 < 224 || 
     leftStickAngle2 > 226 && leftStickAngle2 < 314) {

and why the fuck are you putting so many newlines between everything, it makes it impossible to follow the structure of the code

if ( allowjump == true && grounded == true && distanceToGround > gdval2 && groundhitbool == true ){

anon do we need to bring out the cs grad meme

conditionals evaluate to true/false

a boolean evaluates to true/false

thus

if (allowjump) is already a check for if allowjump is true. Writing it explicitly is just line noise

finally

how much of that massive fucking global declaration actually needs to be global

if it's local to some particular calculation and doesn't need to be stored permanently, then group it with the calculation. ie declare a function.

Anon, when you're copying and pasting 50 lines to modify an algorithm, you've fucked up. You should be copying like 3-5 lines, because you've grouped all the major actions into separate functions

and shit like this

 jt.text = "rvmag2 " + rvMag2.ToString ()
                + "\ntan angle 2" + stickAngleTan2.ToString ()
                + "\nbase run speed2:" + baserunspeed2.ToString ()
                + "\ngrounded " + grounded.ToString ()
                //+ "\ny velocity  " + playerfallvelocity.ToString ()
                                + "\nvelmag " + velMag.ToString ()
                                + "\nvelmag2 " + velMag2.ToString ()
                         + "\n jumpintervaltimer" + jumpintervaltimer.ToString()
                                + "\nstickangle2 " + leftStickAngle2.ToString ()
                                + "\nmovetime " + movingtimer.ToString ()

should just be using a string formatting method, whatever the hell C# has for it

and

if ( stickmoving == true ){
                                                transform.rotation = Quaternion.FromToRotation( Vector3.up, groundhit.normal )* Quaternion.AngleAxis( -xrotation3, Vector3.up )   * Quaternion.AngleAxis( leftStickAngle2 , Vector3.up );
                                        } else {
                                                transform.rotation = Quaternion.FromToRotation( Vector3.up, groundhit.normal )* Quaternion.AngleAxis( -xrotation2, Vector3.up )   * Quaternion.AngleAxis( leftStickAngle2 , Vector3.up );

should be

xrotate;
if stickmoving {
  xrotate = -xrotation3;
}
else { 
  xrotate = -xrotation2;
}
transform.rotation = Quaternion.FromToRotation( Vector3.up, groundhit.normal )* Quaternion.AngleAxis( xrotate, Vector3.up )   * Quaternion.AngleAxis( leftStickAngle2 , Vector3.up );

And thus it actually becomes clear what the fuck you're changing

>>23508

>Using tons of comments is generally a good thing, but there is a point where it is too much.

A comment should be used to explain what isn't apparent in the code. IE, if you do a weird regex because of some cli tool has a stupid output behavior, then you should be commenting that.

but commenting a simple regex like /\/(.+?)\// would be a nuisance, because it's pretty apparent what you're doing.

The entire function of code clarity is derived from line noise. The purpose of a comment is to reduce the impact of line noise. The purpose of "self-documentation" is to reduce line noise.

It's not about making your code obvious to the novice programmer. It's about making your code concise, clean and to the point.

There is however a difference between comments and documentation. Describing the function purpose above its declaration, that's documentation. And that's when you're writing for the freshman, who has no idea what the hell you're trying to do and why.

But not in comments. In comments, you make individual actions better understood (but even more ideally, those individual actions wouldn't necessitate comments)

>>23480

I don't know much about C# but can you create header files like you would C so you don't have a billion global variables in the main game file?

Also listen to this guy: >>23728

He's absolutely right, manipulating a struct is 100 times clearer, more maintainable, and more modular than what your doing now.

>>23711

>Terribly broken work.

You know how people put those "Hang in there!" pictures on their office wall? And it's supposed to inspire them to do better or face down a harsh time?

Well I can say nothing about the effectiveness of that, it really seems like a sort of mental band-aid for covering up a larger problem.

But I'm pretty sure smearing: "I'm a piece of shit" over your wall in feces isn't helping you either. Sort of like stabbing yourself.

Stop stabbing yourself.

>>23480 test 

>>23750

Not really. You can do a partial class and put the variables in another file, but they will still be a apart of that class.

>>23759

Try this:

public void onAction(String name,boolean isPressed, float tpf)
    {
        
        
     if ((Boolean) player.getUserData("alive"))
     {
           var control = player.getControl(PlayerControl.class);          // It's best practices not to use `var` IMO, but I don't know what your type is here.
           control.up = false;
           control.down = false;
           control.left = false;
           control.right = false;

        if (name.equals("up") && (!lastDir.equals("down"))) {
           control.up = true;
           lastDir="up";
        } else if (name.equals("down") && (!lastDir.equals("up"))) {
           control.down = true;
           lastDir="down";
        } else if (name.equals("left")&& (!lastDir.equals("right"))) {
           control.left = true;
           lastDir="left";
        } else if (name.equals("right")&& (!lastDir.equals("left"))) {
            control.right = true;
           lastDir="right";
        }  
      }
    }

Also, don't use use strings for directions. You should use instead an Enumeration or a 2D vector:

public enum Direction
{
    None,
    Up,
    Down,
    Left,
    Right
};

// Cartesian vectors:
public static class Direction
{
    public static readonly Vector2D None = Vector2D(0, 0);
    public static readonly Vector2D Up = Vector2D(0, 1);
    public static readonly Vector2D Down = Vector2D(0, -1);
    public static readonly Vector2D Left = Vector2D(-1, 0);
    public static readonly Vector2D right = Vector2D(1, 0);
};

I'm a bit more partial to the use of vectors, since they can be re-used more.

>>23759

I'm assuming player.getControl(PlayerControl.class) can be passed by reference like this, dunno C#.

I'm also assuming name is just a string and not some special bullshit I'm unaware of

and if you really wanted to, you could have name/lastDir be numbers. ie 0 = up, 1 = right, 2 = down, 3 = left. (clockwise). And then you'd have if's like

if (name = 0 && lastDir != 2) {}

also I'm not sure if you can do

thing.up = thing.down = thing.left = thing.right = false;

in C#.

           thing = player.getControl(PlayerControl.class);
           thing.up = false;
           thing.down = false;
           thing.left = false;
           thing.right = false;

if (name.equals("up") && (!lastDir.equals("down"))) {
           lastDir = name;
           thing.up = true;    
        } else if (name.equals("down") && (!lastDir.equals("down")&& (!lastDir.equals("up"))) {
            lastDir = name;
            thing.down = true;
        } else if (name.equals("left")&& (!lastDir.equals("right") && (!lastDir.equals("left"))) {
            lastDir = name;
            thing.left = true;
        } else if (name.equals("right")&& (!lastDir.equals("left") && (!lastDir.equals("right"))) {
            lastDir = name;
            thing.right = true;    
        }  

>>23762

you triple nigger

Hi thread. I'm thinking about starting up a "Code Review General," thread for things like these. Can I get some help with the guidelines/OP?

>>23784

Kind of pointless to have a "general", given that the problems and solutions tend to be very similar..

For example, we quite literally independently arrived at the same conclusion for clarity

>>23763

>>23762

And that's, basically how the whole "general" would go. The problem of clarity is rarely specific to any individual piece of code.

Instead of just getting "feedback" on your code, what you really need is a decent set of code to reference. Either from literature (Programming Pearls, Art of Unix Programming), Production code (experience in enterprise or open-source [Git is particularly known for being a good reference]) or active experience gained over time.

Otherwise, you should also be referring to people like Erik Naggum and Linus, who while "rude" are exceptionally efficient at keeping to the point (in english, and C/Lisp respectively), or things like Spolksy's blog and the c2 wiki for useful concepts (Don't Repeat Yourself, refactor the lowest hanging fruit, etc) and keeping them in mind as you fix your shit.