Ideas for refactoring/pull requests

[afritz1]

Maybe this could be formalized into a secondary to-do list or task list somewhere (and the to-do list in the Readme could be moved into an actual TODO.txt file). Some of these are things that new contributors could handle.

CMake is already set up for C++14. As @psi29a said, there are many C-ish things that could be replaced with their C++ equivalents (this also includes things like replacing for loops with range-based for, std::find, std::count, etc.).

Each change might require some considerations for the surrounding code, so when in doubt, just leave it alone. We do not want to introduce bugs because of "upgrades". You're free to ignore my suggestions -- these were just things I had in mind for getting rid of C/C++03 stuff.

• Change NULL to nullptr
• Change strcpy() to std::string() or std::copy()
• Change C arrays to std::array (or std::string in the case of char[])
• Change new/delete ptr to std::make_unique
• Change inconvenient type names to auto
• Add = default; to virtual constructors/destructors with { } body
• Replace trivial return true/return false if branches with single bool expression
• Change enum to enum class if not too invasive (i.e., no added static_casts)
• Derived classes might need virtual destructors (CellLoader, TextureConverter, ...)
• Remove (void) from C++ parameter lists (only relevant in C, not C++)
• Change Vector{2,3,4}& to const Vector{2,3,4}& where applicable
• Add more unit vectors to Vector{2,3,4} classes (UnitX, UnitY, UnitZ, etc.)
• Alphabetize forward declarations
• Move reusable/copy-pasted code into lambdas
• Change sprintf() to std::snprintf(), std::to_string(), or std::stringstream
• Add override to derived virtual methods
• Change typedef struct to struct
• Change iterator loops to range-based for loops
• Break lines around ~100 columns
• Replace atoi() with std::stoi()

[kcat]

A couple more ideas:

• Replace static-only classes with proper singletons (non-static members with a static getter; deleted copiers). I already did this for EngineSettings but there's still more classes like that.
• Change away from hungarian notation.

[kcat]

Also:

• Fix unused parameters/variables, and other warnings so we can build with -Wall -Wextra.

This can be a bit tricky since some parameters and variables maybe should be used but aren't because the code was in the middle of having worked on, while others may be vestigial and no longer necessary. In cases where we're not sure, we can just comment out the name:

void function(int /*maybe_should_use*/)
{
    int /*never_read*/ = some_init;
}

and leave some TODO or FIXME comments to mark it.

[psi29a]

I was actually working on these last night. ;)

[psi29a]

Feel free to help work on these! But avoid changing anything in thirdparty/

[psi29a]

I've created a lot of tickets... the idea behind it that people can pick them up, work on them and solve them. Should be low hanging fruit. :)

[afritz1]

Small question: are you guys okay with this in-place lambda syntax?

const int value = [...]()
{
    // Several variables and calculations.
    return result;
}();

I use it all the time because it's a very natural way for me to program and it keeps the surrounding scope clean. I haven't seen very much other code use it so I wasn't sure if it was a standard practice.

Edit: lambdas are implemented on the stack, so they are low/zero-overhead, and are more likely to be inlined because their definition is known at the callsite.

[psi29a]

I'm ok with anonymous functions only if they are one-shots.