-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
1177 allow identifying assets in code from their path #1337
1177 allow identifying assets in code from their path #1337
Conversation
https://github.com/GameDevTecnico/cubos into 1177-allow-identifying-assets-in-code-from-their-path
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (1/2)
uuids::uuid mId; ///< UUID of the asset. | ||
void* mRefCount; ///< Void pointer to avoid including `<atomic>` in the header. | ||
int mVersion; ///< Last known version of the asset. | ||
std::string path; ///< Path of the asset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
std::string path; ///< Path of the asset. | |
std::string mPath; ///< Path of the asset. |
asset.mId = mId; | ||
asset.mRefCount = mRefCount; | ||
asset.mVersion = mVersion; | ||
asset.path = path; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
asset.path = path; | |
asset.path = mPath; |
@@ -503,6 +512,22 @@ std::unique_lock<std::shared_mutex> Assets::lockWrite(const AnyAsset& handle) co | |||
abort(); | |||
} | |||
|
|||
std::string Assets::checkIdType(const std::string& id) const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
method checkIdType
can be made static
std::string Assets::checkIdType(const std::string& id) const | |
std::string Assets::checkIdType(const std::string& id) |
/// @brief Checks if the given ID is of the correct type. | ||
/// @param id ID to check. | ||
/// @return The ID as a string. | ||
std::string checkIdType(const std::string& id) const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
method checkIdType
can be made static
std::string checkIdType(const std::string& id) const; | |
static std::string checkIdType(const std::string& id) ; |
{ | ||
return "UUID"; | ||
} | ||
else if (id.find('/') != std::string::npos || id.find('\\') != std::string::npos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use else
after return
else if (id.find('/') != std::string::npos || id.find('\\') != std::string::npos) | |
if (id.find('/') != std::string::npos || id.find('\\') != std::string::npos) |
CUBOS_INFO("Assets::entry 1 Current ID type: {}", checkIdType(id)); | ||
if (Assets::checkIdType(id) == "Path") | ||
{ | ||
for (auto asset : mEntries) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loop variable is copied but only used as const reference; consider making it a const reference
for (auto asset : mEntries) | |
for (const auto& asset : mEntries) |
uuids::uuid uuid = uuids::uuid{}; | ||
if (Assets::checkIdType(id) == "Path") | ||
{ | ||
for (auto asset : mEntries) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loop variable is copied but only used as const reference; consider making it a const reference
for (auto asset : mEntries) | |
for (const auto& asset : mEntries) |
uuids::uuid mId; ///< UUID of the asset. | ||
void* mRefCount; ///< Void pointer to avoid including `<atomic>` in the header. | ||
int mVersion; ///< Last known version of the asset. | ||
std::string path; ///< Path of the asset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
std::string path; ///< Path of the asset. | |
std::string mPath; ///< Path of the asset. |
asset.mId = mId; | ||
asset.mRefCount = mRefCount; | ||
asset.mVersion = mVersion; | ||
asset.path = path; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
asset.path = path; | |
asset.mPath = mPath; |
uuids::uuid mId; ///< UUID of the asset. | ||
void* mRefCount; ///< Void pointer to avoid including `<atomic>` in the header. | ||
int mVersion; ///< Last known version of the asset. | ||
std::string path; ///< Path of the asset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
std::string path; ///< Path of the asset. | |
std::string mPath; ///< Path of the asset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (2/2)
asset.mId = mId; | ||
asset.mRefCount = mRefCount; | ||
asset.mVersion = mVersion; | ||
asset.path = path; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
asset.path = path; | |
asset.path = mPath; |
if (str.find('/') != std::string::npos || str.find('\\') != std::string::npos) | ||
{ | ||
idOrPath = str; | ||
path = str; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
path = str; | |
mPath = str; |
, mId(other.mId) | ||
, mRefCount(other.mRefCount) | ||
, mVersion(other.mVersion) | ||
, path(other.path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
, path(other.path) | |
, mPath(other.mPath) |
, mId(other.mId) | ||
, mRefCount(other.mRefCount) | ||
, mVersion(other.mVersion) | ||
, path(other.path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
, path(other.path) | |
, mPath(other.mPath) |
mId = other.mId; | ||
mRefCount = other.mRefCount; | ||
mVersion = other.mVersion; | ||
path = other.path; | ||
this->incRef(); | ||
return *this; | ||
} | ||
|
||
AnyAsset& AnyAsset::operator=(AnyAsset&& other) noexcept |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
AnyAsset& AnyAsset::operator=(AnyAsset&& other) noexcept | |
mPath = other.mPath; |
@@ -96,22 +109,27 @@ | |||
|
|||
int AnyAsset::getVersion() const | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
{ | |
mPath = other.mPath; |
} | ||
|
||
bool AnyAsset::isStrong() const | ||
{ | ||
return reflectedId == mId && mRefCount != nullptr; | ||
return uuids::uuid::from_string(idOrPath) == mId && mRefCount != nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid case style for private member path
return uuids::uuid::from_string(idOrPath) == mId && mRefCount != nullptr; | |
return mPath; |
{ | ||
this->incRef(); | ||
} | ||
|
||
AnyAsset::AnyAsset(AnyAsset&& other) noexcept | ||
: reflectedId(other.reflectedId) | ||
: idOrPath(other.idOrPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move constructor initializes class member by calling a copy constructor
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1337 +/- ##
==========================================
+ Coverage 36.18% 42.41% +6.23%
==========================================
Files 402 409 +7
Lines 32096 29284 -2812
==========================================
+ Hits 11614 12421 +807
+ Misses 20482 16863 -3619 ☔ View full report in Codecov by Sentry. |
Description
Changed the Assets::entry functions to search for assets using paths instead of uuids.
Changed the Assets::load to update the mID of the AnyAsset handle.
Checklist