Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7778dc7
test(asset-back): add more AssetLocation unit tests
Thyodas Mar 18, 2025
f144a68
test(asset-back): add tests for AssetRef
Thyodas Mar 18, 2025
f00d6f6
feat(asset-back): improve return value for AssetCatalog::getAssetsView
Thyodas Mar 20, 2025
b62422b
feat(asset-back): add global macro for testing NEXO_TESTING
Thyodas Mar 20, 2025
0d9fa54
test(asset-back): add tests for AssetCatalog
Thyodas Mar 20, 2025
bed3511
ci(asset-back): use Ninja as generator for cmake presets
Thyodas Mar 20, 2025
fabc85e
refactor(asset-back): set AssetCatalog constructor as default
Thyodas Mar 20, 2025
66cc9e4
test(asset-back): add AssetCatalogTest constructor and destructor
Thyodas Mar 20, 2025
92a6e3e
fix(asset-back): global macro not set on MSVC
Thyodas Mar 20, 2025
8d7c753
refactor(asset-back): remove NEXO_TESTING macro, difficult to compile
Thyodas Mar 20, 2025
1df38e5
feat(asset-back): share some CLion config
Thyodas Mar 20, 2025
e371215
feat(asset-back): use Mock class for AssetCatalog
Thyodas Mar 20, 2025
e719245
test(asset-back): add tests for AssetCatalog singleton
Thyodas Mar 20, 2025
ffbd8f2
feat(asset-back): add JSON serialization for AssetType enum
Thyodas Mar 20, 2025
6bedba1
feat(asset-back): rename genUniqueDependencyName to genUniqueDependen…
Thyodas Mar 20, 2025
9e3fc44
test(asset-back): add AssetImporterContext tests
Thyodas Mar 20, 2025
9b09e77
test(asset-back): use const for references in AssetImporterContext tests
Thyodas Mar 21, 2025
7fb3190
test(asset-back): fix leak in AssetImporterContext.test.cpp
Thyodas Mar 21, 2025
19e7dd3
fix(asset-back): mark importAssetUsingImporter and importAssetTryImpo…
Thyodas Mar 22, 2025
5f41255
refactor(asset-back): enhance AssetImporter interface and improve par…
Thyodas Mar 22, 2025
128fc69
refactor(asset-back): replace gtest_add_tests with gtest_discover_tes…
Thyodas Mar 22, 2025
7712ae1
test(asset-back): add AssetImporter.test.cpp and change link visibili…
Thyodas Mar 22, 2025
853c06f
refactor(asset-back): improve const correctness and simplify unregist…
Thyodas Mar 25, 2025
e6375fe
test(asset-back): add tests for multiple importer types and incompati…
Thyodas Mar 25, 2025
0f01219
test(asset-back): enhance test expectations with proper ordering in A…
Thyodas Mar 25, 2025
9cdd0c2
test(asset-back): ensure proper cleanup by deleting mock importer in …
Thyodas Mar 25, 2025
75f5d4c
test(asset-back): add EXPECT for invalidating assetRef
Thyodas Mar 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ src/client/main.cpp
# Covers JetBrains IDEs: IntelliJ, RubyMine, PhpStorm, AppCode, PyCharm, CLion, Android Studio, WebStorm and Rider
# Reference: https://intellij-support.jetbrains.com/hc/en-us/articles/206544839

**/.idea/
.idea/*
!cmake.xml
!sonarlint.xml

# User-specific stuff
.idea/**/workspace.xml
Expand Down Expand Up @@ -150,7 +152,6 @@ B-CPP-500_rtype.pdf
*.log

# Jetbrains IDEs
.idea/
build/

vcpkg*/
Expand Down
10 changes: 10 additions & 0 deletions .idea/cmake.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions .idea/sonarlint.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"description": "Default configuration with vcpkg",
"hidden": false,
"binaryDir": "${sourceDir}/build",
"generator": "Ninja",
"cacheVariables": {
"CMAKE_TOOLCHAIN_FILE": {
"type": "FILEPATH",
Expand Down
46 changes: 37 additions & 9 deletions engine/src/assets/Asset.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "AssetLocation.hpp"
#include "AssetRef.hpp"
#include "json.hpp"

namespace nexo::assets {

Expand All @@ -46,21 +47,46 @@ namespace nexo::assets {
_COUNT
};



/**
* @brief Array of asset type names
* @note The order of the array must match the order of the AssetType enum.
*/
const std::array<std::string, static_cast<int>(AssetType::_COUNT)> AssetTypeNames = {

"Texture",
"Model",
"Sound",
"Music",
"Font",
"Shader",
"Script"
constexpr const char *AssetTypeNames[] = {
"UNKNOWN",
"TEXTURE",
"MODEL",
"SOUND",
"MUSIC",
"FONT",
"SHADER",
"SCRIPT"
};

static_assert(
static_cast<int>(AssetType::_COUNT) == std::size(AssetTypeNames),
"AssetTypeNames array size must match AssetType enum size"
);

constexpr const char *getAssetTypeName(AssetType type) {
return AssetTypeNames[static_cast<int>(type)];
}

inline void to_json(nlohmann::json& j, AssetType type) {
j = getAssetTypeName(type);
}

inline void from_json(const nlohmann::json& j, AssetType& type) {
for (int i = 0; i < static_cast<int>(AssetType::_COUNT); ++i) {
if (j == AssetTypeNames[i]) {
type = static_cast<AssetType>(i);
return;
}
}
type = AssetType::UNKNOWN;
}
Comment thread
Thyodas marked this conversation as resolved.

/**
* @brief Asset ID type
* @note This is a UUID that uniquely identifies an asset. Alias of boost::uuids::uuid.
Expand Down Expand Up @@ -142,6 +168,8 @@ namespace nexo::assets {

friend class AssetRef<TAssetData>;
public:
static constexpr AssetType TYPE = TAssetType;

virtual ~Asset() override
{
delete data;
Expand Down
13 changes: 1 addition & 12 deletions engine/src/assets/AssetCatalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@
#include <Path.hpp>

namespace nexo::assets {
AssetCatalog::AssetCatalog()
{
}

void AssetCatalog::deleteAsset(AssetID id)
{
Expand Down Expand Up @@ -61,19 +58,11 @@ namespace nexo::assets {
return assets;
}

std::ranges::view auto AssetCatalog::getAssetsView() const
{
return m_assets
| std::views::values
| std::views::transform([](const auto& asset) {
return GenericAssetRef(asset);
});
}

GenericAssetRef AssetCatalog::registerAsset(const AssetLocation& location, IAsset* asset)
{
if (!asset)
return GenericAssetRef::null();
// TODO: implement error handling if already exists (once we have the folder tree)
Comment thread
Thyodas marked this conversation as resolved.
auto shared_ptr = std::shared_ptr<IAsset>(asset);
shared_ptr->m_metadata.location = location;
if (shared_ptr->m_metadata.id.is_nil())
Expand Down
13 changes: 10 additions & 3 deletions engine/src/assets/AssetCatalog.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ namespace nexo::assets {
* @brief Singleton class that holds all the assets in the engine.
*/
class AssetCatalog {
private:
protected:
// Singleton: private constructor and destructor
AssetCatalog();
AssetCatalog() = default;
~AssetCatalog() = default;

public:
Expand Down Expand Up @@ -82,7 +82,14 @@ namespace nexo::assets {
* @brief Get all assets in the catalog as a view.
* @return A view of all assets in the catalog.
*/
[[nodiscard]] std::ranges::view auto getAssetsView() const;
[[nodiscard]] auto getAssetsView() const
{
return m_assets
| std::views::values
| std::views::transform([](const auto& asset) {
return GenericAssetRef(asset);
});
}

/**
* @brief Get all assets of a specific type in the catalog.
Expand Down
6 changes: 3 additions & 3 deletions engine/src/assets/AssetImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace nexo::assets {
}

GenericAssetRef AssetImporter::importAssetUsingImporter(const AssetLocation& location,
const ImporterInputVariant& inputVariant, AssetImporterBase* importer)
const ImporterInputVariant& inputVariant, AssetImporterBase* importer) const
{
AssetImporterContext* ctx = m_customCtx;
AssetImporterContext ctxOnStack;
Expand All @@ -59,7 +59,7 @@ namespace nexo::assets {


importer->import(*ctx);
auto asset = ctx->getMainAsset();
const auto asset = ctx->getMainAsset();
if (!asset)
return GenericAssetRef::null();
if (asset->getID().is_nil())
Expand All @@ -71,7 +71,7 @@ namespace nexo::assets {
}

GenericAssetRef AssetImporter::importAssetTryImporters(const AssetLocation& location,
const ImporterInputVariant& inputVariant, const std::vector<AssetImporterBase*>& importers)
const ImporterInputVariant& inputVariant, const std::vector<AssetImporterBase*>& importers) const
{
std::vector<AssetImporterBase *> untriedImporters;
for (const auto& importer : importers) {
Expand Down
25 changes: 16 additions & 9 deletions engine/src/assets/AssetImporter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ namespace nexo::assets {
requires std::derived_from<AssetType, IAsset>
AssetRef<AssetType> importAsset(const AssetLocation& location, const ImporterInputVariant& inputVariant);
GenericAssetRef importAssetAuto(const AssetLocation& location, const ImporterInputVariant& inputVariant);
GenericAssetRef importAssetUsingImporter(const AssetLocation& location, const ImporterInputVariant& inputVariant, AssetImporterBase *importer);
GenericAssetRef importAssetUsingImporter(const AssetLocation& location, const ImporterInputVariant& inputVariant, AssetImporterBase *importer) const;
GenericAssetRef importAssetTryImporters(const AssetLocation& location, const ImporterInputVariant& inputVariant, const std::vector<AssetImporterBase *>&
importers);
importers) const;

/**
* @brief Get all registered importers for an asset type
Expand Down Expand Up @@ -97,12 +97,20 @@ namespace nexo::assets {

void clearCustomContext() { m_customCtx = nullptr; }

AssetImporterContext *getCustomContext() const { return m_customCtx; }
[[nodiscard]] AssetImporterContext *getCustomContext() const { return m_customCtx; }

void setParameters(const json& params);


private:
protected:

/**
* @brief Protected constructor for custom importers
* @note Used currently by unit tests
*/
explicit AssetImporter(AssetImporterContext *ctx) : m_customCtx(ctx)
{
}

/**
* @brief Register an importer for a specific asset type
Expand All @@ -122,7 +130,7 @@ namespace nexo::assets {
*
* @tparam AssetType The type of asset the importer can handle
* @param importer The importer instance to register
* @param priority Optional priority value (higher values = higher priority)
* @param priority Optional priority value (higher values = higher priority, if equal then insertion order)
*/
template<typename AssetType>
requires std::derived_from<AssetType, IAsset>
Expand Down Expand Up @@ -166,7 +174,7 @@ namespace nexo::assets {
}

template<typename AssetType> requires std::derived_from<AssetType, IAsset>
void AssetImporter::registerImporter(AssetImporterBase *importer, int priority)
void AssetImporter::registerImporter(AssetImporterBase *importer, const int priority)
{
const auto typeIdx = std::type_index(typeid(AssetType));

Expand All @@ -181,7 +189,7 @@ namespace nexo::assets {
auto& importersDetailsVec = m_importersDetails[typeIdx];

size_t i = 0;
for (; i < importersVec.size() && importersDetailsVec[i].priority < priority; ++i);
for (; i < importersVec.size() && priority <= importersDetailsVec[i].priority ; ++i);
importersVec.insert(importersVec.begin() + static_cast<long>(i), importer);
importersDetailsVec.insert(importersDetailsVec.begin() + static_cast<long>(i), {priority});
}
Expand Down Expand Up @@ -217,7 +225,6 @@ namespace nexo::assets {
void AssetImporter::unregisterAllImportersForType()
{
const auto typeIdx = std::type_index(typeid(AssetType));
m_importers.erase(typeIdx);
m_importersDetails.erase(typeIdx);
unregisterAllImportersForType(typeIdx);
}
} // namespace nexo::assets
19 changes: 11 additions & 8 deletions engine/src/assets/AssetImporterContext.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ namespace nexo::assets {

template <typename AssetType>
requires std::derived_from<AssetType, IAsset>
AssetLocation genUniqueDependencyName();
AssetLocation genUniqueDependencyLocation();

static AssetName formatUniqueName(const std::string& name, const AssetType type, unsigned int id)
{
return AssetName(std::format("{}_{}{}", name, getAssetTypeName(type), id));
}


private:
Expand All @@ -99,19 +104,17 @@ namespace nexo::assets {

template <typename AssetType>
requires std::derived_from<AssetType, IAsset>
AssetLocation AssetImporterContext::genUniqueDependencyName()
AssetLocation AssetImporterContext::genUniqueDependencyLocation()
{
auto depLoc = AssetLocation(
std::format("{}_{}{}", location.getFullLocation(), AssetTypeNames[AssetType::getType()], ++m_depUniqueId)
);
auto depLoc = AssetLocation(location.getFullLocation());
depLoc.setName(formatUniqueName(location.getName().data(), AssetType::TYPE, ++m_depUniqueId));

if (!AssetCatalog::getInstance().getAsset(depLoc))
return depLoc;

// If the location already exists, we need to generate a new one
auto name = std::string(location.getName());
while (AssetCatalog::getInstance().getAsset(depLoc)) {
std::string newName = name + std::to_string(++m_depUniqueId);
depLoc.setName(newName);
depLoc.setName(formatUniqueName(location.getName().data(), AssetType::TYPE, ++m_depUniqueId));
if (m_depUniqueId > ASSET_MAX_DEPENDENCIES) {
// Prevent infinite loop
LOG(NEXO_ERROR, "Failed to generate unique name for asset: {}: couldn't find unique id", depLoc.getFullLocation());
Expand Down
12 changes: 9 additions & 3 deletions engine/src/assets/AssetLocation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,18 @@ namespace nexo::assets {
return *this;
}

AssetLocation& setPackName(const std::optional<std::reference_wrapper<const AssetPackName>>& packName)
AssetLocation& setPackName(const AssetPackName& packName)
{
_packName = packName;
return *this;
}

AssetLocation& clearPackName()
{
_packName.reset();
return *this;
}

/**
* @brief Get the asset's name
* @return The asset's AssetName
Expand Down Expand Up @@ -120,9 +126,9 @@ namespace nexo::assets {
_path = extractedPath;
}

bool operator==(const AssetLocation& asset_location) const
bool operator==(const AssetLocation& assetLocation) const
{
return _name == asset_location._name && _packName == asset_location._packName && _path == asset_location._path;
return _name == assetLocation._name && _packName == assetLocation._packName && _path == assetLocation._path;
}

bool operator==(const std::string& fullLocation) const
Expand Down
18 changes: 9 additions & 9 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,20 @@ include(${CMAKE_CURRENT_LIST_DIR}/renderer/CMakeLists.txt)
include(${CMAKE_CURRENT_LIST_DIR}/ecs/CMakeLists.txt)

# Add tests
gtest_add_tests(TARGET engine_tests
TEST_LIST engineTestsList
gtest_discover_tests(engine_tests
TEST_LIST engineTestsList
)
gtest_add_tests(TARGET common_tests
TEST_LIST commonTestsList
gtest_discover_tests(common_tests
TEST_LIST commonTestsList
)
gtest_add_tests(TARGET renderer_tests
TEST_LIST rendererTestsList
gtest_discover_tests(renderer_tests
TEST_LIST rendererTestsList
)
gtest_add_tests(TARGET ecs_tests
TEST_LIST ecsTestsList
gtest_discover_tests(ecs_tests
TEST_LIST ecsTestsList
)

# Core engine tests
# Core engine tests
set_tests_properties(${engineTestsList} PROPERTIES LABELS "engine")
# Common tests
set_tests_properties(${commonTestsList} PROPERTIES LABELS "common")
Expand Down
8 changes: 6 additions & 2 deletions tests/engine/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,14 @@ add_executable(engine_tests
${BASEDIR}/scene/Scene.test.cpp
${BASEDIR}/scene/SceneManager.test.cpp
${BASEDIR}/components/Camera.test.cpp
${BASEDIR}/assets/AssetName.test.cpp
${BASEDIR}/assets/AssetLocation.test.cpp
${BASEDIR}/assets/AssetCatalog.test.cpp
${BASEDIR}/assets/AssetName.test.cpp
${BASEDIR}/assets/AssetRef.test.cpp
${BASEDIR}/assets/AssetImporterContext.test.cpp
${BASEDIR}/assets/AssetImporter.test.cpp
# Add other engine test files here
)

# Link gtest and engine (renderer) libraries
target_link_libraries(engine_tests GTest::gtest GTest::gmock nexoRenderer)
target_link_libraries(engine_tests PRIVATE GTest::gtest GTest::gmock nexoRenderer)
Loading