pr fixes

src/adera/Machines/Rocket.cpp

@@ -33,6 +33,7 @@
 #include "adera/Shaders/Phong.h"
 #include "adera/Shaders/PlumeShader.h"
 #include "osp/Resource/blueprints.h"
+#include "osp/PhysicsConstants.h"
 #include "adera/SysExhaustPlume.h"
 #include "adera/Plume.h"
 #include <Magnum/Trade/MeshData.h>
@@ -102,24 +103,27 @@ void SysMachineRocket::update_physics(ActiveScene& rScene)
         }
 
         // Check for nonzero throttle, continue otherwise
-        WireData *throttle = machine.m_wiThrottle.connected_value();
-        wiretype::Percent* percent = nullptr;
-        if (throttle)
+        WireData *pThrottle = machine.m_wiThrottle.connected_value();
+        wiretype::Percent* pPercent = nullptr;
+        if (pThrottle != nullptr)
         {
             using wiretype::Percent;
-            percent = std::get_if<Percent>(throttle);
-            if (!(percent->m_value > 0.0f))
+            pPercent = std::get_if<Percent>(pThrottle);
+            if ((pPercent == nullptr) || !(pPercent->m_value > 0.0f))
             {
                 continue;
             }
         }
-        else { continue; }
+        else
+        {
+            continue;
+        }
 
         // Check for adequate resource inputs
         bool fail = false;
         for (auto& resource : machine.m_resourceLines)
         {
-            MachineContainer* src = m_scene.reg_try_get<MachineContainer>(resource.m_sourceEnt);
+            auto const* src = m_scene.reg_try_get<MachineContainer>(resource.m_sourceEnt);
             if (!src)
             {
                 std::cout << "Error: no source found\n";
@@ -132,7 +136,10 @@ void SysMachineRocket::update_physics(ActiveScene& rScene)
                 break;
             }
         }
-        if (fail) { continue; }
+        if (fail)
+        {
+            continue;
+        }
 
         // Perform physics calculation
 
@@ -147,46 +154,38 @@ void SysMachineRocket::update_physics(ActiveScene& rScene)
 
         }
 
-        if (WireData *pThrottle = machine.m_wiThrottle.connected_value();
-            pThrottle != nullptr)
-        {
-            wiretype::Percent *pPercent = std::get_if<wiretype::Percent>(pThrottle);
-            if (pPercent == nullptr) { continue; }
-
-            float thrustMag = machine.m_params.m_maxThrust * percent->m_value;
-
-            Matrix4 relTransform = pRbAncestor->m_relTransform;
-
-            /* Compute thrust force
-             * Thrust force is defined to be along +Z by convention.
-             * Obtains thrust vector in rigidbody space
-             */
-            Vector3 thrustDir = relTransform.transformVector(Vector3{0.0f, 0.0f, 1.0f});
-            // Take thrust in rigidbody space and apply to RB in world space
-            Vector3 thrust = thrustMag * thrustDir;
-            Vector3 worldThrust = rCompTf.m_transform.transformVector(thrust);
-            SysPhysics_t::body_apply_force(rCompRb, worldThrust);
-
-            // Obtain point where thrust is applied relative to RB CoM
-            Vector3 location = relTransform.translation();
-            // Compute worldspace torque from engine location, thrust vector
-            Vector3 torque = Magnum::Math::cross(location, thrust);
-            Vector3 worldTorque = rCompTf.m_transform.transformVector(torque);
-            SysPhysics_t::body_apply_torque(rCompRb, worldTorque);
-
+        Matrix4 relTransform = pRbAncestor->m_relTransform;
+
+        /* Compute thrust force
+            * Thrust force is defined to be along +Z by convention.
+            * Obtains thrust vector in rigidbody space
+            */
+        Vector3 thrustDir = relTransform.transformVector(Vector3{0.0f, 0.0f, 1.0f});
+        float thrustMag = machine.m_params.m_maxThrust * pPercent->m_value;
+        // Take thrust in rigidbody space and apply to RB in world space
+        Vector3 thrust = thrustMag * thrustDir;
+        Vector3 worldThrust = rCompTf.m_transform.transformVector(thrust);
+        SysPhysics_t::body_apply_force(rCompRb, worldThrust);
+
+        // Obtain point where thrust is applied relative to RB CoM
+        Vector3 location = relTransform.translation();
+        // Compute worldspace torque from engine location, thrust vector
+        Vector3 torque = Magnum::Math::cross(location, thrust);
+        Vector3 worldTorque = rCompTf.m_transform.transformVector(torque);
+        SysPhysics_t::body_apply_torque(rCompRb, worldTorque);
 
-            // Perform resource consumption calculation
-            float massFlowRateTot = thrustMag / (9.81f * machine.m_params.m_specImpulse);
-            for (auto const& resource : machine.m_resourceLines)
-            {
-                float massFlowRate = massFlowRateTot * resource.m_massRateFraction;
-                float massFlow = massFlowRate * m_scene.get_time_delta_fixed();
-                uint64_t required = resource.m_type->resource_quantity(massFlow);
-                auto* src = m_scene.reg_try_get<MachineContainer>(resource.m_sourceEnt);
-                uint64_t consumed = src->request_contents(required);
-                std::cout << "consumed " << consumed << " units of fuel, "
-                    << src->check_contents().m_quantity << " remaining\n";
-            }
+        // Perform resource consumption calculation
+        float massFlowRateTot = thrustMag /
+            (phys::constants::g_0 * machine.m_params.m_specImpulse);
+        for (auto const& resource : machine.m_resourceLines)
+        {
+            float massFlowRate = massFlowRateTot * resource.m_massRateFraction;
+            float massFlow = massFlowRate * m_scene.get_time_delta_fixed();
+            uint64_t required = resource.m_type->resource_quantity(massFlow);
+            auto* src = m_scene.reg_try_get<MachineContainer>(resource.m_sourceEnt);
+            uint64_t consumed = src->request_contents(required);
+            std::cout << "consumed " << consumed << " units of fuel, "
+                << src->check_contents().m_quantity << " remaining\n";
         }
     }
 }

src/adera/ShipResources.cpp

@@ -89,9 +89,7 @@ uint64_t MachineContainer::request_contents(uint64_t quantity)
 {
     if (quantity > m_contents.m_quantity)
     {
-        uint64_t remainder = m_contents.m_quantity;
-        m_contents.m_quantity = 0;
-        return remainder;
+        return std::exchange(m_contents.m_quantity, 0);
     }
 
     m_contents.m_quantity -= quantity;

src/osp/Active/SysVehicle.cpp

@@ -124,7 +124,7 @@ StatusActivated SysVehicle::activate_sat(ActiveScene &scene,
 
         PrototypePart &proto = *partDepends;
 
-        auto [partEntity, machineMapping] = this->part_instantiate(proto, partBp,
+        auto const& [partEntity, machineMapping] = this->part_instantiate(proto, partBp,
             vehicleEnt);
 
         vehicleComp.m_parts.push_back(partEntity);
@@ -225,6 +225,7 @@ void SysVehicle::add_machines_to_object(ActiveEnt partEnt, ActiveEnt objEnt,
     std::vector<unsigned> const& machineIndices)
 {
     if (machineIndices.empty()) { return; }
+
     auto &compMachines =
         m_scene.get_registry().get_or_emplace<ACompMachines>(partEnt);
 
@@ -243,17 +244,16 @@ void SysVehicle::add_machines_to_object(ActiveEnt partEnt, ActiveEnt objEnt,
         }
 
         // Instantiate the machine for the object
-        Machine& machine = sysMachine->second->instantiate(objEnt,
-            protoMachine, bpMachine);
+        sysMachine->second->instantiate(objEnt, protoMachine, bpMachine);
 
         // Add a PartMachine definition to the root part's ACompMachines
         compMachines.m_machines.emplace_back(objEnt, sysMachine);
     }
 }
 
 void SysVehicle::part_instantiate_machines(ActiveEnt partEnt,
-    std::vector<MachineDef> machineMapping,
-    PrototypePart& part, BlueprintPart& partBP)
+    std::vector<MachineDef> const& machineMapping,
+    PrototypePart const& part, BlueprintPart const& partBP)
 {
     std::vector<PrototypeMachine> const& protoMachines = part.get_machines();
     std::vector<BlueprintMachine> const& bpMachines = partBP.m_machines;
@@ -274,10 +274,11 @@ std::pair<ActiveEnt, std::vector<SysVehicle::MachineDef>> SysVehicle::part_insta
      * that are created to represent the objects that own them
      */
     std::vector<MachineDef> machineMapping;
+    machineMapping.reserve(prototypes.size());
 
     //std::cout << "size: " << newEntities.size() << "\n";
 
-    for (unsigned i = 0; i < prototypes.size(); i++)
+    for (size_t i = 0; i < prototypes.size(); i++)
     {
         PrototypeObject const& currentPrototype = prototypes[i];
         ActiveEnt parentEnt;
@@ -335,6 +336,7 @@ std::pair<ActiveEnt, std::vector<SysVehicle::MachineDef>> SysVehicle::part_insta
             }
 
             std::vector<DependRes<Texture2D>> textureResources;
+            textureResources.reserve(drawable.m_textures.size());
             for (unsigned i = 0; i < drawable.m_textures.size(); i++)
             {
                 unsigned texID = drawable.m_textures[i];

src/osp/Active/SysVehicle.h

@@ -118,8 +118,12 @@ class SysVehicle : public IDynamicSystem, public IActivator
 
 private:
 
-    // Stores the association between a PrototypeObj entity and the indices of
-    // its owned machines in the PrototypePart array
+    /* Stores the association between a PrototypeObj entity and the indices of
+     * its owned machines in the PrototypePart array. Stores a const reference
+     * to a vector because the vector is filled with PrototypePart machine data
+     * by part_instantiate() and consumed by part_instantiate_machines(), all
+     * within the activate() function scope within which the index data is stable.
+     */
     struct MachineDef
     {
         ActiveEnt m_machineOwner;
@@ -168,8 +172,8 @@ class SysVehicle : public IDynamicSystem, public IActivator
      * @param partBP [in] The blueprint configs of the part being created
      */
     void part_instantiate_machines(ActiveEnt partEnt,
-        std::vector<MachineDef> machineMapping,
-        PrototypePart& part, BlueprintPart& partBP);
+        std::vector<MachineDef> const& machineMapping,
+        PrototypePart const& part, BlueprintPart const& partBP);
 
     /**
      * Create a Physical Part from a PrototypePart and put it in the world

src/osp/PhysicsConstants.h

@@ -0,0 +1,32 @@
+/**
+ * Open Space Program
+ * Copyright © 2019-2020 Open Space Program Project
+ *
+ * MIT License
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+#pragma once
+
+namespace osp::phys::constants
+{
+
+constexpr float g_0 = 9.80665f;
+
+} // namespace osp::phys

src/osp/Resource/AssetImporter.cpp

@@ -79,19 +79,20 @@ void osp::AssetImporter::load_sturdy_file(std::string_view filepath, Package& pk
     gltfImporter.close();
 }
 
-void AssetImporter::load_machines(tinygltf::Value const& extras,
-    std::vector<PrototypeMachine>& machineArray,
-    std::vector<unsigned>& machineIndexArray)
+std::vector<unsigned> AssetImporter::load_machines(tinygltf::Value const& extras,
+    std::vector<PrototypeMachine>& machineArray)
 {
     if (!extras.Has("machines"))
     {
         std::cout << "Error: no machines found!\n";
-        return;
+        return {};
     }
     tinygltf::Value const& machines = extras.Get("machines");
     std::cout << "JSON machines!\n";
     auto const& machArray = machines.Get<tinygltf::Value::Array>();
 
+    std::vector<unsigned> machineIndices;
+    machineIndices.reserve(machArray.size());
     // Loop through machine configs
     // machArray looks like:
     // [
@@ -145,9 +146,16 @@ void AssetImporter::load_machines(tinygltf::Value const& extras,
             }
         }
 
-        machineIndexArray.push_back(machineArray.size());
+        machineIndices.emplace_back(machineArray.size());
         machineArray.emplace_back(std::move(machine));
+        /* TODO: eventually it would be nice to pre-allocate the PrototypePart's
+         * machine array, but for now it doesn't seem worth it to pre-traverse
+         * the whole part tree just for this purpose. If we do end up with a
+         * more involved loading scheme, we can change this then.
+         */
     }
+
+    return machineIndices;
 }
 
 void osp::AssetImporter::load_part(TinyGltfImporter& gltfImporter,
@@ -160,6 +168,9 @@ void osp::AssetImporter::load_part(TinyGltfImporter& gltfImporter,
     PrototypePart part;
     proto_add_obj_recurse(gltfImporter, pkg, resPrefix, part, 0, id);
 
+    // TODO: remove if the machine array is pre-allocated
+    part.get_machines().shrink_to_fit();
+
     // Parse extra properties
     tinygltf::Value const& extras = static_cast<tinygltf::Node const*>(
         gltfImporter.object3D(id)->importerState())->extras;
@@ -484,7 +495,7 @@ void AssetImporter::proto_add_obj_recurse(TinyGltfImporter& gltfImporter,
         *static_cast<tinygltf::Node const*>(childData->importerState());
     if (node.extras.Has("machines"))
     {
-        load_machines(node.extras, part.get_machines(), obj.m_machineIndices);
+        obj.m_machineIndices = load_machines(node.extras, part.get_machines());
     }
 
     UnsignedInt objIndex = protoObjects.size();

src/osp/Resource/AssetImporter.h

@@ -104,13 +104,12 @@ PluginManager;
      *
      * @param extras            [in] An extras node from a glTF file
      * @param machineArray      [out] A machine array from a PrototypePart
-     * @param machineIndexArray [out] An PrototypeObject machineIndexArray which
-     *                                stores the indices of the machineArray
-     *                                elements which belong to it
+     * 
+     * @return A machineIndexArray which is used by PrototypeObjects to store
+     * the indices of the machineArray elements which belong to it
      */
-    static void load_machines(tinygltf::Value const& extras,
-        std::vector<PrototypeMachine>& machineArray,
-        std::vector<unsigned>& machineIndexArray);
+    static std::vector<unsigned> load_machines(tinygltf::Value const& extras,
+        std::vector<PrototypeMachine>& machineArray);
 
     /**
      * Load only associated config files, and add resource paths to the package

src/osp/Resource/blueprints.cpp

@@ -58,12 +58,15 @@ BlueprintPart& BlueprintVehicle::add_part(
 
     // Create and default initialize object blueprint machines
     size_t numMachines = prototype->get_machines().size();
-    std::vector<BlueprintMachine> machineBPs(numMachines, BlueprintMachine{});
+    std::vector<BlueprintMachine> machineBPs;
+    machineBPs.resize(numMachines);
 
     BlueprintPart blueprint
     {
         partIndex,
-        translation, rotation, scale,
+        translation,
+        rotation,
+        scale,
         std::move(machineBPs)
     };
 

src/test_application/main.cpp

@@ -324,18 +324,18 @@ void debug_print_machines()
     }
 
     ActiveScene& scene = g_ospMagnum->get_scenes().begin()->second;
-    auto view = scene.get_registry().view<osp::active::ACompMachines>();
+    auto view = scene.get_registry().view<ACompMachines>();
 
     for (ActiveEnt ent : view)
     {
         ACompHierarchy& hier = scene.reg_get<ACompHierarchy>(ent);
         std::cout << "[" << int(ent) << "]: " << hier.m_name << "\n";
 
         auto& machines = scene.reg_get<ACompMachines>(ent);
-        for (auto mach : machines.m_machines)
+        for (ACompMachines::PartMachine const& mach : machines.m_machines)
         {
             ActiveEnt machEnt = mach.m_partEnt;
-            std::string sysName = mach.m_system->first;
+            std::string const& sysName = mach.m_system->first;
             std::cout << "  ->[" << int(machEnt) << "]: " << sysName << "\n";
         }
     }