Automatic code formating

Programming
1

The current code formatting guide isn’t followed consistently. One thing that could improve this (but doesn’t fix naming and other issue like that) formating issue is using an automatic formating tool.

I’ve started using clang format with my engine and I think we could also use it for thrive to have the code formatting be consistent in the c++ files (it doesn’t work perfectly for angelscript). Here’s what I suggest the format configuration is:

Language: Cpp
AccessModifierOffset: '-4'
AlignAfterOpenBracket: DontAlign
AlignConsecutiveAssignments: 'false'
AlignConsecutiveDeclarations: 'false'
AlignEscapedNewlinesLeft: Left
AlignTrailingComments: 'false'
AllowAllParametersOfDeclarationOnNextLine: 'false'
AllowShortBlocksOnASingleLine: 'false'
AllowShortCaseLabelsOnASingleLine: 'true'
AllowShortFunctionsOnASingleLine: Empty
AllowShortIfStatementsOnASingleLine: 'false'
AllowShortLoopsOnASingleLine: 'false'
AlwaysBreakAfterReturnType: All
AlwaysBreakBeforeMultilineStrings: 'false'
AlwaysBreakTemplateDeclarations: 'false'
BinPackArguments: 'true'
BinPackParameters: 'false'
BreakBeforeBinaryOperators: None
# BreakBeforeBraces: Attach
BreakBeforeBraces: Custom
BraceWrapping:
    AfterClass: false
    AfterControlStatement: false
    AfterEnum: false
    AfterFunction: true
    AfterNamespace: false
    AfterStruct: false
    AfterUnion: false
    # AfterExternBlock: false
    BeforeCatch: false
    BeforeElse: false
    IndentBraces: false
    SplitEmptyFunction: true
    SplitEmptyRecord: false
    SplitEmptyNamespace: true
BreakBeforeInheritanceComma: 'false'
BreakBeforeTernaryOperators: 'false'
BreakConstructorInitializers: AfterColon
# Works fine even without this
# CommentPragmas: '^! \\'
CompactNamespaces: 'true'
ConstructorInitializerAllOnOneLineOrOnePerLine: 'false'
Cpp11BracedListStyle: 'true'
DerivePointerAlignment: 'false'
IndentCaseLabels: 'false'
IndentWidth: '4'
IndentWrappedFunctionNames: 'true'
KeepEmptyLinesAtTheStartOfBlocks: 'true'
MaxEmptyLinesToKeep: '3'
NamespaceIndentation: None
PointerAlignment: Left
ReflowComments: 'true'
SortIncludes: 'true'
SortUsingDeclarations: 'true'
SpaceAfterCStyleCast: 'false'
SpaceAfterTemplateKeyword: 'false'
SpaceBeforeAssignmentOperators: 'true'
SpaceBeforeParens: Never
# This is also a new setting not yet available
# SpaceBeforeRangeBasedForLoopColon: 'true'
SpaceInEmptyParentheses: 'false'
SpacesBeforeTrailingComments: '1'
SpacesInAngles: 'false'
SpacesInCStyleCastParentheses: 'false'
SpacesInParentheses: 'false'
SpacesInContainerLiterals: 'false'
SpacesInParentheses: 'false'
SpacesInSquareBrackets: 'false'
Standard: Cpp11
TabWidth: '4'
UseTab: Never

Here’s an explanation of the options: https://clang.llvm.org/docs/ClangFormatStyleOptions.html
Thoughts on these settings? I’m not sure what the BraceWrapping and SpaceBeforeParens options should be to match the old style guide as well as possible.

Here’s the membrane system formatted like that:

#include "membrane_system.h"


// #include "bullet/collision_system.h"
// #include "bullet/rigid_body_system.h"
// #include "engine/component_factory.h"
// #include "engine/engine.h"
// #include "engine/entity_filter.h"
// #include "engine/game_state.h"
// #include "engine/serialization.h"
// #include "game.h"
// #include "ogre/scene_node_system.h"
// #include "scripting/luajit.h"
// #include "util/make_unique.h"

#include <OgreMaterialManager.h>
#include <OgreMesh2.h>
#include <OgreMeshManager2.h>
// #include <OgreMaterial.h>
// #include <OgreTechnique.h>
// #include <OgreEntity.h>
#include <OgreRoot.h>
#include <OgreSceneManager.h>
#include <OgreSubMesh2.h>
// #include <stdexcept>

#include <atomic>

using namespace thrive;

////////////////////////////////////////////////////////////////////////////////
// Membrane Component
////////////////////////////////////////////////////////////////////////////////
static std::atomic<int> MembraneMeshNumber = {0};

MembraneComponent::MembraneComponent() : Leviathan::Component(TYPE)
{
    // Create the mesh for rendering us
    m_mesh = Ogre::MeshManager::getSingleton().createManual(
        "MembraneMesh_" + std::to_string(++MembraneMeshNumber),
        Ogre::ResourceGroupManager::DEFAULT_RESOURCE_GROUP_NAME);

    m_subMesh = m_mesh->createSubMesh();
}

MembraneComponent::~MembraneComponent()
{

    LEVIATHAN_ASSERT(!m_item, "MembraneComponent not released");

    Ogre::MeshManager::getSingleton().remove(m_mesh);
    m_mesh.reset();
    m_subMesh = nullptr;
}

void
    MembraneComponent::Release(Ogre::SceneManager* scene)
{

    if(m_item) {
        scene->destroyItem(m_item);
        m_item = nullptr;
    }
}
// ------------------------------------ //
Ogre::Vector3
    MembraneComponent::FindClosestOrganelles(Ogre::Vector3 target)
{
    // The distance we want the membrane to be from the organelles squared.
    double closestSoFar = 4;
    int closestIndex = -1;

    for(size_t i = 0, end = organellePositions.size(); i < end; i++) {
        double lenToObject = target.squaredDistance(organellePositions[i]);

        if(lenToObject < 4 && lenToObject < closestSoFar) {
            closestSoFar = lenToObject;

            closestIndex = i;
        }
    }

    if(closestIndex != -1)
        return (organellePositions[closestIndex]);
    else
        return Ogre::Vector3(0, 0, -1);
}

Ogre::Vector3
    MembraneComponent::GetMovement(Ogre::Vector3 target,
        Ogre::Vector3 closestOrganelle)
{
    double power = pow(2.7, (-target.distance(closestOrganelle)) / 10) / 50;

    return (Ogre::Vector3(closestOrganelle) - Ogre::Vector3(target)) * power;
}

Ogre::Vector3
    MembraneComponent::GetExternalOrganelle(double x, double y)
{

    float organelleAngle = Ogre::Math::ATan2(y, x).valueRadians();

    Ogre::Vector3 closestSoFar(0, 0, 0);
    float angleToClosest = Ogre::Math::TWO_PI;

    for(size_t i = 0, end = vertices2D.size(); i < end; i++) {
        if(Ogre::Math::Abs(Ogre::Math::ATan2(vertices2D[i].y, vertices2D[i].x)
                               .valueRadians() -
                           organelleAngle) < angleToClosest) {
            closestSoFar = Ogre::Vector3(vertices2D[i].x, vertices2D[i].y, 0);
            angleToClosest = Ogre::Math::Abs(
                Ogre::Math::ATan2(vertices2D[i].y, vertices2D[i].x)
                    .valueRadians() -
                organelleAngle);
        }
    }

    return closestSoFar;
}

bool
    MembraneComponent::contains(float x, float y)
{
    // if (x < -cellDimensions/2 || x > cellDimensions/2 || y <
    // -cellDimensions/2 || y > cellDimensions/2) return false;

    bool crosses = false;

    int n = vertices2D.size();
    for(int i = 0; i < n - 1; i++) {
        if((vertices2D[i].y <= y && y < vertices2D[i + 1].y) ||
            (vertices2D[i + 1].y <= y && y < vertices2D[i].y)) {
            if(x < (vertices2D[i + 1].x - vertices2D[i].x) *
                           (y - vertices2D[i].y) /
                           (vertices2D[i + 1].y - vertices2D[i].y) +
                       vertices2D[i].x) {
                crosses = !crosses;
            }
        }
    }

    return crosses;
}
// ------------------------------------ //
//! Should set the colour of the membrane once working
void
    MembraneComponent::setColour(const Float4& value)
{

    colour = value;

    // TODO: apply
    DEBUG_BREAK;
}
// ------------------------------------ //
void
    MembraneComponent::Update(Ogre::SceneManager* scene,
        Ogre::SceneNode* parentcomponentpos)
{
    // Skip if the mesh is already created //
    if(isInitialized)
        return;

    if(!isInitialized)
        Initialize();

    DrawMembrane();

    // 12 vertices added per index of vertices2D
    const auto bufferSize = vertices2D.size() * 12;

    if(!m_vertexBuffer) {

        Ogre::RenderSystem* renderSystem =
            Ogre::Root::getSingleton().getRenderSystem();
        Ogre::VaoManager* vaoManager = renderSystem->getVaoManager();

        Ogre::VertexElement2Vec vertexElements;
        vertexElements.push_back(
            Ogre::VertexElement2(Ogre::VET_FLOAT3, Ogre::VES_POSITION));
        vertexElements.push_back(Ogre::VertexElement2(
            Ogre::VET_FLOAT2, Ogre::VES_TEXTURE_COORDINATES));
        // vertexElements.push_back(Ogre::VertexElement2(Ogre::VET_FLOAT3,
        // Ogre::VES_NORMAL));

        m_vertexBuffer = vaoManager->createVertexBuffer(vertexElements,
            bufferSize, Ogre::BT_DYNAMIC_PERSISTENT, nullptr, false);

        Ogre::VertexBufferPackedVec vertexBuffers;
        vertexBuffers.push_back(m_vertexBuffer);

        // 1 to 1 index buffer mapping

        Ogre::uint16* indices =
            reinterpret_cast<Ogre::uint16*>(OGRE_MALLOC_SIMD(
                sizeof(Ogre::uint16) * bufferSize, Ogre::MEMCATEGORY_GEOMETRY));

        for(size_t i = 0; i < bufferSize; ++i) {

            indices[i] = static_cast<Ogre::uint16>(i);
        }

        // TODO: check if this is needed (when a 1 to 1 vertex and index mapping
        // is used)
        Ogre::IndexBufferPacked* indexBuffer = nullptr;

        try {
            indexBuffer =
                vaoManager->createIndexBuffer(Ogre::IndexBufferPacked::IT_16BIT,
                    bufferSize, Ogre::BT_IMMUTABLE,
                    // Could this be false like the vertex buffer to not keep a
                    // shadow buffer
                    indices, true);
        } catch(const Ogre::Exception& e) {

            // Avoid memory leak
            OGRE_FREE_SIMD(indices, Ogre::MEMCATEGORY_GEOMETRY);
            indexBuffer = nullptr;
            throw e;
        }

        Ogre::VertexArrayObject* vao = vaoManager->createVertexArrayObject(
            vertexBuffers, indexBuffer, Ogre::OT_TRIANGLE_LIST);

        m_subMesh->mVao[Ogre::VpNormal].push_back(vao);

        // This might be needed because we use a v2 mesh
        // Use the same geometry for shadow casting.
        m_subMesh->mVao[Ogre::VpShadow].push_back(vao);


        // Set the bounds to get frustum culling and LOD to work correctly.
        // TODO: make this more accurate
        m_mesh->_setBounds(
            Ogre::Aabb(Ogre::Vector3::ZERO, Ogre::Vector3::UNIT_SCALE * 50)
            /*, false*/);
        m_mesh->_setBoundingSphereRadius(50);

        // Set the membrane material //
        //  Ogre::MaterialPtr baseMaterial =
        //  Ogre::MaterialManager::getSingleton().getByName(
        //      "Membrane");

        // If this is uncommented material destruction should be added
        // to the destructor (and this probably moved to the
        // constructor)

        // Ogre::MaterialPtr materialPtr =
        // baseMaterial->clone(m_mesh->getName()); materialPtr->compile();
        // // This doesn't work anymore
        // Ogre::TextureUnitState* ptus = materialPtr->getTechnique(0)->
        //     getPass(0)->getTextureUnitState(0);
        // ptus->setColourOperationEx(Ogre::LBX_MODULATE, Ogre::LBS_MANUAL,
        // Ogre::LBS_TEXTURE,
        //     colour);
        // m_mesh->setMaterial(materialPtr);

        m_subMesh->setMaterialName("Membrane");

        // Use different material for testing to see the mesh better
        // m_subMesh->setMaterialName("Background");
    }

    // Map the buffer for writing //
    // DO NOT READ FROM THE MAPPED BUFFER
    MembraneVertex* RESTRICT_ALIAS meshVertices =
        reinterpret_cast<MembraneVertex * RESTRICT_ALIAS>(
            m_vertexBuffer->map(0, m_vertexBuffer->getNumElements()));


    // Update mesh data //

    // Creates a 3D prism from the 2D vertices.

    // All of these floats were originally doubles. But to have more
    // performance they are now floats
    float height = .1;

    size_t writeIndex = 0;

    for(size_t i = 0, end = vertices2D.size(); i < end; i++) {
        // Finds the UV coordinates be projecting onto a plane and stretching to
        // fit a circle.
        const float x = vertices2D[i].x;
        const float y = vertices2D[i].y;
        const float z = vertices2D[i].z;

        const float ray = x * x + y * y + z * z;

        const float t = Ogre::Math::Sqrt(ray) / (2.0 * ray);
        const float a = t * x;
        const float b = t * y;
        // const float c = t*z;

        const Ogre::Vector2 uv(a + 0.5, b + 0.5);

        const Ogre::Vector2 center(0.5, 0.5);
        const double currentRadians = 2.0 * 3.1416 * i / end;
        const double nextRadians = 2.0 * 3.1416 * (i + 1) / end;

        // y and z coordinates are swapped to match the Ogre up direction

        // Bottom (or top?) half first triangle
        meshVertices[writeIndex++] = {Ogre::Vector3(0, 0, 0), uv};

        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[(i + 1) % end].x,
                vertices2D[(i + 1) % end].z - height / 2,
                vertices2D[(i + 1) % end].y),
            uv};

        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[i % end].x,
                vertices2D[i % end].z - height / 2, vertices2D[i % end].y),
            uv};

        // Second triangle
        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[i % end].x,
                vertices2D[i % end].z + height / 2, vertices2D[i % end].y),
        };

        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[(i + 1) % end].x,
                vertices2D[(i + 1) % end].z + height / 2,
                vertices2D[(i + 1) % end].y),
            uv};

        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[(i + 1) % end].x,
                vertices2D[(i + 1) % end].z - height / 2,
                vertices2D[(i + 1) % end].y),
            uv};

        // This was originally a second loop
        // Top half first triangle
        // This seems to be the only one that is actually drawn to the screen,
        // at least with the current test membrane.
        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[i % end].x,
                vertices2D[i % end].z + height / 2, vertices2D[i % end].y),
            center +
                Ogre::Vector2(cos(currentRadians), sin(currentRadians)) / 2};

        meshVertices[writeIndex++] = {Ogre::Vector3(0, height / 2, 0), center};

        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[(i + 1) % end].x,
                vertices2D[(i + 1) % end].z + height / 2,
                vertices2D[(i + 1) % end].y),
            center + Ogre::Vector2(cos(nextRadians), sin(nextRadians)) / 2};

        // Second triangle
        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[i % end].x,
                vertices2D[i % end].z - height / 2, vertices2D[i % end].y),
            uv};

        meshVertices[writeIndex++] = {
            Ogre::Vector3(vertices2D[(i + 1) % end].x,
                vertices2D[(i + 1) % end].z - height / 2,
                vertices2D[(i + 1) % end].y),
            uv};

        meshVertices[writeIndex++] = {Ogre::Vector3(0, -height / 2, 0), uv};
    }

    // LOG_INFO("Write index is: " + std::to_string(writeIndex) + ", buffer
    // size: " +
    //     std::to_string(bufferSize));
    // This can be commented out when this works correctly, or maybe a
    // different macro for debug builds to include this check could
    // work, but it has to also work on linux
    LEVIATHAN_ASSERT(writeIndex == bufferSize, "Invalid array element math in "
                                               "fill vertex buffer");

    // Upload finished data to the gpu (unmap all needs to be used to
    // suppress warnings about destroying mapped buffers)
    m_vertexBuffer->unmap(Ogre::UO_UNMAP_ALL);

    // TODO: apply the current colour to the material instance

    if(!m_item) {
        // This needs the v2 mesh to contain data to work
        m_item = scene->createItem(m_mesh, Ogre::SCENE_DYNAMIC);
        parentcomponentpos->attachObject(m_item);
    }
}


void
    MembraneComponent::Initialize()
{

    for(Ogre::Vector3 pos : organellePositions) {
        if(abs(pos.x) + 1 > cellDimensions) {
            cellDimensions = abs(pos.x) + 1;
        }
        if(abs(pos.y) + 1 > cellDimensions) {
            cellDimensions = abs(pos.y) + 1;
        }
    }

    for(int i = 0; i < membraneResolution; i++) {
        vertices2D.emplace_back(
            -cellDimensions + 2 * cellDimensions / membraneResolution * i,
            -cellDimensions, 0);
    }
    for(int i = 0; i < membraneResolution; i++) {
        vertices2D.emplace_back(cellDimensions,
            -cellDimensions + 2 * cellDimensions / membraneResolution * i, 0);
    }
    for(int i = 0; i < membraneResolution; i++) {
        vertices2D.emplace_back(
            cellDimensions - 2 * cellDimensions / membraneResolution * i,
            cellDimensions, 0);
    }
    for(int i = 0; i < membraneResolution; i++) {
        vertices2D.emplace_back(-cellDimensions,
            cellDimensions - 2 * cellDimensions / membraneResolution * i, 0);
    }

    // Does this need to run 50*cellDimensions times. That seems to be
    // really high and probably causes some of the lag
    for(int i = 0; i < 50 * cellDimensions; i++) {
        DrawMembrane();
    }

    // Subdivide();


    isInitialized = true;
}
// ------------------------------------ //
void
    MembraneComponent::DrawMembrane()
{
    // Stores the temporary positions of the membrane.
    auto newPositions = vertices2D;

    // Loops through all the points in the membrane and relocates them as
    // necessary.
    for(size_t i = 0, end = newPositions.size(); i < end; i++) {
        Ogre::Vector3 closestOrganelle = FindClosestOrganelles(vertices2D[i]);
        if(closestOrganelle == Ogre::Vector3(0, 0, -1)) {
            newPositions[i] =
                (vertices2D[(end + i - 1) % end] + vertices2D[(i + 1) % end]) /
                2;
        } else {
            Ogre::Vector3 movementDirection =
                GetMovement(vertices2D[i], closestOrganelle);
            newPositions[i].x -= movementDirection.x;
            newPositions[i].y -= movementDirection.y;
        }
    }

    // Allows for the addition and deletion of points in the membrane.
    for(size_t i = 0; i < newPositions.size() - 1; i++) {
        // Check to see if the gap between two points in the membrane is too
        // big.
        if(newPositions[i].distance(
               newPositions[(i + 1) % newPositions.size()]) >
            cellDimensions / membraneResolution) {
            // Add an element after the ith term that is the average of the i
            // and i+1 term.
            auto it = newPositions.begin();
            Ogre::Vector3 tempPoint =
                (newPositions[(i + 1) % newPositions.size()] +
                    newPositions[i]) /
                2;
            newPositions.insert(it + i + 1, tempPoint);

            i++;
        }

        // Check to see if the gap between two points in the membrane is too
        // small.
        if(newPositions[(i + 1) % newPositions.size()].distance(
               newPositions[(i - 1) % newPositions.size()]) <
            cellDimensions / membraneResolution) {
            // Delete the ith term.
            auto it = newPositions.begin();
            newPositions.erase(it + i);
        }
    }

    vertices2D = newPositions;
}

void
    MembraneComponent::sendOrganelles(double x, double y)
{
    organellePositions.emplace_back(x, y, 0);
}

void
    MembraneComponent::clear()
{
    isInitialized = false;
    vertices2D.clear();

    if(m_item)
        m_item->detachFromParent();
}
2 Likes
2

I don’t think it’s really all that important (we’re not a large team by any stretch of the imagination), and i think our setup is already kinda complex for new people to join. Maybe have it as a suggested thing?

Edit: also the braces there are not consistent between function definitions and loops/ifs

3

I can write a script to format all the files with clang format. So if we make the clang format file the official format standard for thrive, I can then, after people make changes, clean up their code automatically.

4

You mean like having a github bot?

5

If I can be bothered I could make a bot that tells people on pull requests if their code has style issues, but initially it would be just done manually (or with the script).

6

They aren’t either in the currently prevalent style in the code, where functions have the brace on the next line and everything else has the brace on the same line. But if you prefer I could set that all braces are on the next line.

7

Isn’t the recommended style in thrive to have all the braces on the same line?
Most of the files i’ve seen seem to follow this

8

Except functions which should be split like this:

ReturnType
name(
    arg1,
    arg2
) const {

}
9

I meant in regards to braces, yeah the function parameters and return type are all over the place.
(then again i think the thrive style can be a little too cumbersome if the return type and parameters are short)