• cookie_sabotage@sh.itjust.works
    link
    fedilink
    English
    arrow-up
    161
    arrow-down
    2
    ·
    9 months ago
    public class GameManager : MonoBehaviour
    {
        public bool EnableHighContrast;
        public bool PlayerWon;
        public float PlayerUnitsMoved;
        public int PlayerDeathCount;
        public float PlayerHealth;
    
        public void PlayerTakeDamage(float damage)
        {
            PlayerHealth -= damage;
            if (PlayerHealth < 0)
            {
                PlayerDieAndRespawn();
            }
        }
    
        public void PlayerDieAndRespawn()
        {
            return;
        }
    }
    

    I couldn’t contain myself.

    • Wise@feddit.uk
      link
      fedilink
      English
      arrow-up
      57
      ·
      9 months ago

      Should it be

      PlayerHealth <= 0
      

      ?

      Otherwise the player could have 0 health and not die? I’m sleep deprived so forgive me if I’m wrong

      • vithigar@lemmy.ca
        link
        fedilink
        arrow-up
        12
        ·
        edit-2
        9 months ago

        You are correct about it allowing you to have zero health and not die, but whether or not that’s the correct behavior will depend on the game. Off the top of my head I know that Street Fighter, some versions at least, let you cling to life at zero.

      • joshfaulkner@lemmy.world
        link
        fedilink
        arrow-up
        5
        ·
        edit-2
        9 months ago

        I know this is /c/Progammerhumor, but I wanted to pull on this thread a little bit for my own edification. I’m a Python guy and have been a while, but I’ve dabbled in other languages. The screenshot says “MonoBehaviour” which makes me assume this is mono or a .Net-like language (you know what happens when you assume).

        If your player health is a float, would mono or .Net have an issue comparing the float with integer zero “0”? I mean, it seems like floating point precision may make it impossible for it to ever “equal” integer zero, but it also seems like the code isn’t accounting for that precision error.

        Am I overthinking this?

        • herrvogel@lemmy.world
          link
          fedilink
          arrow-up
          7
          ·
          9 months ago

          Floating point errors are a product of how floating points work as a mathematical concept. So they’re independent of the programming language and can happen everywhere.

          In this case though, I doubt it’s a critical issue. So the player “died” when they actually had 0.000000000027 hp left or whatever. Who cares? Do you need to be that precise?

        • Melobol@lemmy.ml
          link
          fedilink
          arrow-up
          1
          ·
          9 months ago

          As a noob in unity and programming, my understanding is that MonoBehavior only means that this script has to be attached as a component to a game object to function. And has no other meaning - but correct me if I’m wrong please.

      • Randomocity@sh.itjust.works
        link
        fedilink
        English
        arrow-up
        1
        arrow-down
        5
        ·
        9 months ago

        This won’t work if you can ever take more than 1 damage. If you were at 1 and received 2 damage you would become invincible. You’d want to do less than or equal to.

      • TheOakTree@lemm.ee
        link
        fedilink
        arrow-up
        15
        ·
        9 months ago

        Well if you have a “down but not dead” condition then yes, you could escape a fight with 0 health (assuming you have teammates/pawns that can save you).

    • Sakychu@lemmy.world
      link
      fedilink
      arrow-up
      10
      ·
      9 months ago

      I called the takeDamage function and my player disappeared: send 'elp everything foobar

    • Ironfacebuster@lemmy.world
      link
      fedilink
      arrow-up
      5
      ·
      9 months ago

      Too readable, please make each name a paragraph describing its function and how it relates to the other variables/functions around it

  • BatrickPateman@feddit.de
    link
    fedilink
    arrow-up
    112
    ·
    9 months ago

    Great. Now that my code is self-documenting it is somehow also not legible?

    Make up your damn minds, peeps!

    • skulblaka@startrek.website
      link
      fedilink
      arrow-up
      39
      ·
      9 months ago

      I genuinely believe something like this is what some of my professors wanted me to submit back in school. I once got a couple points off a project for not having a clarifying comment on every single line of code. I got points off once for not comment-clarifying a fucking iterator variable. I wish I could see what they would have said if I turned in something like this. I have a weird feeling that this file would have received full marks.

      • maniclucky@lemmy.world
        link
        fedilink
        arrow-up
        12
        ·
        9 months ago

        Did you have my professor for intro to C? This guy was well known for failing people for plagiarism on projects where the task was basically “hello world”. And he disallowed using if/else for the first month of class.

        • OneCardboardBox@lemmy.sdf.org
          link
          fedilink
          English
          arrow-up
          6
          ·
          9 months ago

          Reminds me of an early Uni project where we had to operate on data in an array of 5 elements, but because “I didn’t teach it to everyone yet” we couldn’t use loops. It was going to be a tedious amount of copy-paste.

          I think I got around it by making a function called “not_loop” that applied a functor argument to each element of the array in serial. Professor forgot to ban that.

          • CileTheSane@lemmy.ca
            link
            fedilink
            arrow-up
            5
            ·
            9 months ago

            but because “I didn’t teach it to everyone yet” we couldn’t use loops.

            That is aggravating. “I didn’t teach the class the proper way to do this task, so you have to use the tedious way.” What is the logic behind that other than wasting everyone’s time?

            • skulblaka@startrek.website
              link
              fedilink
              arrow-up
              1
              ·
              9 months ago

              Teaching someone the wrong way to do something frequently makes the right way make way more sense. Someone who just copy/pasted 99 near identical if statements understands on a fundamental level when, why, and where you use a for loop much more than someone who just read in the textbook “a for loop is used to iterate elements in a collection”.

              • I Cast Fist@programming.dev
                link
                fedilink
                arrow-up
                3
                ·
                9 months ago

                Reminds me of a dude that wrote the equivalent of this in Visualg (a brazilian pseudocode language and program, meant solely for teaching programming)

                if
                  if
                    if
                      if
                        if (x < 10) then
                          print(x)
                        else
                      else
                    else
                  else
                else
                

                That the thing ran and didn’t complain about the amount of loose/needless if’s checking fuck all baffles my mind to this day.

              • CileTheSane@lemmy.ca
                link
                fedilink
                arrow-up
                1
                ·
                9 months ago

                And if I know the right way of doing it I already understand why it’s better because I want to use it in this situation. Making the students who already understand the lesson do it the wrong way is just a waste of their time.

  • hstde@feddit.de
    link
    fedilink
    arrow-up
    73
    arrow-down
    2
    ·
    9 months ago

    This is something that can easily get refactored, because the purpose of alia the variables is right there in the name. This is way better that spending three days to try to figure out what the purpose of var1 is.

    • jballs@sh.itjust.works
      link
      fedilink
      English
      arrow-up
      8
      arrow-down
      1
      ·
      9 months ago

      Nah, refactoring this would be a bitch. Your function name contains everything that happens in the function. Which means if you add something to it, you also have to change the name of the function. So CallThisWhenThePlayerTakesDamageAndIfThePlayerHealthIsLessThanZeroThenAlsoTheyDie would have to go to something like CallThisWhenThePlayerTakesDamageAndIfThePlayerHealthIsLessThanZeroThenAlsoTheyDieAndIncrementTheTotalDamageTakenCounter if you added something else.

  • tobogganablaze@lemmus.org
    link
    fedilink
    English
    arrow-up
    51
    arrow-down
    3
    ·
    edit-2
    9 months ago

    I mean, this is overdoing it a bit and the “thisVarMakesItSoThat” part is redundant, but other than that those are very descriptive property- and method names, which is not a bad thing.

    • rbits@lemm.ee
      link
      fedilink
      arrow-up
      11
      ·
      9 months ago

      It wouldn’t need to say HighContrastForAccessibilityPurposes though, it would ideally just be HighContrast, and the “for accessibility purposes” would be a comment, right?

      • deadcream@sopuli.xyz
        link
        fedilink
        arrow-up
        12
        arrow-down
        1
        ·
        edit-2
        9 months ago

        Variable names shouldn’t need comments, period. You don’t want to look it up every time this variable is used in code, just to understand what it holds. Of course there are always exceptions, but generally names should be descriptive enough to not need additional explanation.

        And context can also come from names of other things, e.g. name of a class / namespace that holds this variable. For example AccessibilitySettings.HighContrast, where AccessibilitySettings holds all options related to accessibility.

        • 4am@lemm.ee
          link
          fedilink
          arrow-up
          5
          ·
          9 months ago

          Yeah but “HighContrast” is enough; if you need to know the Why and not the What you can find a comment at the definition. There’s not need to carry the whole Wiki article everytime you need to use the variable.

          • Kushan@lemmy.world
            link
            fedilink
            English
            arrow-up
            5
            ·
            9 months ago

            I think the argument about “for accessibility” is missing the point a little bit and a common mistake most developers make.

            You should endeavour to make your interface accessible by default. You shouldn’t be thinking in terms of “okay here’s the design and here’s the design that’s accessible”, you should be considering accessibility in all of your designs.

            Now that’s usually a bit harder with games because you have styles and themes that you don’t want to detract from, but if your interface causes accessibility issues, it’s generally going to be bad for people that don’t have accessibility needs as well.

            Accessibility benefits everyone.

      • tobogganablaze@lemmus.org
        link
        fedilink
        English
        arrow-up
        6
        ·
        edit-2
        9 months ago

        Well the “Purposes” can definitly be dropped. I guess “HighContrast” would be enough if there is only a single high contrast setting, but if there are multiple then I think “HighContrastForAccessibility” would be totally fine.

  • JakenVeina@lemm.ee
    link
    fedilink
    English
    arrow-up
    23
    arrow-down
    2
    ·
    9 months ago

    I’ll take this over the more “classic” styles, when people seed to believe they were paying the compiler by the character.

  • vacuumoftalent@lemmy.world
    link
    fedilink
    arrow-up
    17
    arrow-down
    3
    ·
    9 months ago

    Looks ugly until you need to implement something and realize you’ve been blessed with a description of business logic.

    • BeigeAgenda@lemmy.ca
      link
      fedilink
      arrow-up
      5
      ·
      9 months ago

      Until they find out that the way to descriptive variables or functions needs to be extended with new business logic requiring renaming of functions again and again.

      I think maintaining code with this level of verbose naming, will be a pain over time. If they don’t let the naming slip, and then they could as well use cryptic 3 letter names.

  • warlaan@lemm.ee
    link
    fedilink
    arrow-up
    14
    ·
    edit-2
    9 months ago

    The real naming fail is calling the class “GameManager”, still my number one pet peeve. With a class name as vague as that you would have to add tons of information into the variable name. (Also the class name begs for unorganized code. I mean name one function or variable that you could not justify putting into the “GameManager” class. After all if it’s managing the game it could justifiably perform any process in the game and access any state in it.)

    Once you put the first bool into a class with a name like AccessibilitySettings, calling it something like HighContrast is completely sufficient.

    • Meansalladknifehands@lemm.ee
      link
      fedilink
      arrow-up
      6
      ·
      9 months ago

      We’ve all been guilty of these mistakes, naming stuff is hard, structuring your project is hard, learning the grains of a language takes time. But comments like these are golden nugets, some might read this and think “oh yeah, this makes sense” and rethink their whole methodology of naming and structure. You might have pushed someone reading your comment, to think more about these things.

    • Dragster39@feddit.de
      link
      fedilink
      arrow-up
      4
      ·
      9 months ago

      With a class name as vague as that you would have to add tons of information into the variable name.

      Technically they did exactly that.

    • Snazz@lemmy.world
      link
      fedilink
      arrow-up
      4
      ·
      9 months ago

      Unity actually gives any class with the name GameManager a special gear icon. You cant just forgo the cool gear icon!

      (Its not too terrible from an organizational standpoint because most of the scripts are attached to game objects. MonoBehavior is a component of GameObject. For instance, you’d never have player movement in the GameManager class, you would put it in the component class attached to the player character GameObject.)

      • warlaan@lemm.ee
        link
        fedilink
        arrow-up
        2
        ·
        edit-2
        9 months ago

        you’d never have player movement in the GameManager class

        You want to bet? (Source: I teach game programming on a college level.)

        But yeah, your comment about the gear icon is sadly more true than people may realize. Game developers do questionable things. => Engine developers cater to people. => Students argue that if something is supported it can’t be that bad. Sometimes it feels like fighting windmills.

    • dan@upvote.au
      link
      fedilink
      arrow-up
      2
      ·
      9 months ago

      “Manager” classes often end up like “God classes”, just like how “Utils” classes end up with a bunch of random stuff in them.

      • warlaan@lemm.ee
        link
        fedilink
        arrow-up
        1
        ·
        9 months ago

        At my first job I was working on an MMO and we had a DatabaseManager class with 10k+ lines of code. Less than the first 200 lines actually used any of the members of that class.