Share a few lines of code that exemplify your programming style. Share innovations.

I'll start.

void UpdateAI()
{
   CPlayer* p;
 
    for(int i=0; i<PLAYERS; i++)
    {
        p = &g_player;
 
        if(!p->on)
            continue;
 
        if(!p->ai)
            continue;
 
        UpdateAI(p);
    }
}

Features:

- Use pointers with one, first letter of word of type (CPlayer* p, CUnit* u, CUnitType* t, CCamera* c, CClient* c, CEntity* e, CPacket* p) or purpose (position, radius)

Another example:

bool CollidesWithUnits(float x, float z, float hwx, float hwz, bool isUnit, CUnit* thisU, CUnit* ignore, bool checkPass)
{
  CUnit* u;
  CUnitType* t;
  float r;
  CVector3 p;
  CCamera* c;

  for(int i=0; i<UNITS; i++)
  {    
    u = &g_unit; // <--

    if(!u->on)
      continue;

    if(u == ignore)
      continue;

    if(u == thisU)
      continue;

    t = &g_unitType[u->type]; // <--

    //...

    c = &u->camera; // <--
    p = c->Position(); // <--
    r = t->radius; // <--

    //...
  }

  return false;
}

- Multiple levels of pointers retrieving something from other pointers

Instead of nesting if-statements,

bool CWidget::DropDown_prelbuttonup()
{
  if(ldown)
  {
    ldown = false;

    if(opened)
    {
      if(mousescroll)
      {
        mousescroll = false;
        return true;  // intercept mouse event
      }

      for(...)
      {
        // list item?
        if(...)
        {
          //....
          opened = false;
          return true;  // intercept mouse event
        }
      }

      // up button?
      if(...)
      {
        //...
        return true;
      }

      // down button?
      if(...)
      {
        //...
        return true;
      }

      opened = false;

      return true;  // intercept mouse event
    }

    return false;
  }

  return false;
}

I try to write

if(!...)
  return;

whenever I can, so that it looks like this:

bool CWidget::DropDown_prelbuttonup()
{
  if(!ldown)
    return false;
  
  ldown = false;

  if(!opened)
    return false;

  if(mousescroll)
  {
    mousescroll = false;
    return true;  // intercept mouse event
  }

  for(...)
  {
    // list item?
    if(...)
    {
      //....
      opened = false;
      return true;  // intercept mouse event
    }
  }

  // up button?
  if(...)
  {
    //...
    return true;
  }

  // down button?
  if(...)
  {
    //...
    return true;
  }

  opened = false;

  return true;  // intercept mouse event
}

This was a big paradigm shift for me, when I first thought of this. But now I do it all the time.

One critique. When declaring a pointer I get mental alarm bells if I don't see it initialized, so instead of:

CPlayer* p;

I prefer to see:

CPlayer* p = NULL; // Substitute with nullptr, 0, or whatever according to your preference/standard-followed/etc

This applies even if the rest of the code logic makes it impossible for the pointer to be uninitialized at the time it's first used; it may be impossible now but who's to say that it's going to remain impossible following months/years of maintenance?

There is almost universal consensus that abbreviating variable names is not a good idea. With minor exceptions for well understood idioms, such as loop counters, variables should be given descriptive names.

The type is the poorest name, as that is the one thing that we know for certain. The intended purpose is the least certain. Hopefully it is made obvious in the code that follows, but when that code contains a bug (or even just looks like it might), there is ambiguity between the desired and actual behaviour. We cannot rely 100% on the code to infer the intent.

For example, in your CollidesWithUnits() function, the parameters "hwx" and "hwz" and "checkPass" are obscure. Lacking other information, I would presume "x", "z" are a position of some sort. The parameter "isUnit" is surprising because the types involved seem to imply that these are all units. I presume "thisU" is the "current" unit, and you are trying to exclude self collisions. Maybe isUnit is related to thisU being perhaps optional? It is clear what "ignore" is doing, but it is not clear why there is some other unit being excluded from the collision.

Maybe with the full source of the function I would be able to infer the purpose of the parameters, both in the function itself and in the overall scope of the game.

Have you considered declaring your variables on the line they are first used? It would avoid the problem of having an uninitialised variable.

This block:

  for(...)
  {
    // list item?
    if(...)
    {
      //....
      opened = false;
      return true;  // intercept mouse event
    }
  }

Suggests usage of the for...if anti-pattern. Now, it may not be, it probably isn't, but it is something to be aware of and watch out for.

rip-off is correct about this:

Have you considered declaring your variables on the line they are first used? It would avoid the problem of having an uninitialised variable.

It's a good habit to get into, and also has the good practice of limiting your variable scope to the narrowest possible. If your first block of code, "p" does not need to exist outside of the loop, so it shouldn't. This kind of basic stuff will help make your code more robust too.

I saw 'CClassName'.
I stopped reading.

Dumbest naming convention ever and I hate everyone who does it.

There is almost universal consensus that abbreviating variable names is not a good idea. With minor exceptions for well understood idioms, such as loop counters, variables should be given descriptive names.

I have to disagree with that. One-letter variable names are fine if they are in a very small scope, because you can't forget what they are and what they're initialized to when they're declared just 2~3 lines above.

// this is fine
if (this->SomeCondition())
{
    SOME_DATA_STRUCT* d = this->GetDataStruct();
    this->DoThis(d);
    this->DoThat(d);
}


// but this is not fine
void MyClass::VeryLongMethod()
{
    const SOME_DATA_STRUCT* d = this->GetDataStruct();

    //
    // Lots of lines of code go here.
    //

    // WTF was d again!?
    this->DoThis(d);
    this->DoThat(d);
}

// this is fine
if (this->SomeCondition())
{
    SOME_DATA_STRUCT* d = this->GetDataStruct();
    this->DoThis(d);
    this->DoThat(d);
}

This is fine indeed - until a coupla years later you come back to the code and find that maintenance/evolution/whatever has added 200 extra lines of code between your "SOME_DATA_STRUCT* d = this->GetDataStruct();" and your "this->DoThis(d);". Then it stops being fine and you'll wish you'd given it a better name first time round.

That's kinda the point here. Code isn't static; you don't just write it once and then leave it be forever more, and even if you think you will, trust me - you won't. So you always write with the assumption that code is going to be revisited, worked over, bugfixed, have functionality added, and generally sprout all kinds of interesting tentacles in all kinds of interesting directions during it's lifetime, and that way you're protected when (and it is a "when", not an "if") that time does (and it is a "does", not a "may") come.

At the very least pick a name such that doing a search for all occurrances of that name in your codebase will give you what is genuinely all occurrances and nothing else.

I saw 'CClassName'.
I stopped reading.

Dumbest naming convention ever and I hate everyone who does it.

I agree.. but I didnt stop reading and found more than that :P

Seriously.. I dont understand the point of this thread.. the code posted is nothing revolutionary and, from so many points of view, obsolete, error prone and verbose.. I mean... why 2 if that end up in a continue? why oh why is this supposed to be any more clear than a boolean operation? Raw arrays everywhere with no sign of bounds check.. why would I want to declare a variable faaaaar away from where it is actually first assigned? anyway.. good luck :P

void UpdateAI()
{
  for(int i = 0; i < PlayerList.size(); ++i)
  {
    if(!PlayerList.IsAlive || !PlayerList.IsAi) {
      continue;
    }
    UpdateAI(PlayerList);
  }
}
 
void UpdateAI(TPlayer& aiPlayer);

I saw 'CClassName'.
I stopped reading.

Dumbest naming convention ever and I hate everyone who does it.

I couldn't agree more. Another thing i found stupid is why people use a signle character for a variable name. That code is unreadable except by the person who wrote it in the first place.