-
Notifications
You must be signed in to change notification settings - Fork 214
Component Creator : Block component overwrite, fix issue with bad characters in component naming. #4391
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
base: dev
Are you sure you want to change the base?
Conversation
| QWidget& _widget; | ||
| }; | ||
|
|
||
| class TfValidIdentifierValidator : public QValidator |
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.
Any reason to call it with Tf prefix? At first I thought it was a pixar class.
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.
Myeah the idea was to make it clear it's a "tf valid identifier"... "valid identifier" on its own could mean anything i guess
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.
It's just confusing because I expect classes and functions beginning with Tf, Sdf, Usd to be Pixar stuff, but not very important.
pierrebai-adsk
left a comment
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.
Minor things, nothing blocking.
|
pr-build |
| std::filesystem::path location = { _locationEdit->text().toStdString() }; | ||
| location.append(_nameEdit->text().toStdString()); | ||
|
|
||
| if (std::filesystem::exists(location) && !std::filesystem::is_empty(location)) { |
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.
MayaUsd isn't C++17 because we have to support older versions of Maya. So you cannot use filesystem. Instead use gulrak. See other files for examples.
(cherry picked from commit e5b085ba5a35e1ee8c0476a78d62c69602390fca)
| if (MSVC) | ||
| target_compile_definitions(${PROJECT_NAME} | ||
| PRIVATE | ||
| QT_NO_FLOAT16_OPERATORS | ||
| ) | ||
| endif() |
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.
How exactly does this solve the problem? I think we should make this condition more specific to 2023 only.
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.
see https://github.com/qt/qtbase/blob/dev/dist/changes-5.13.1
* [QTBUG-72073] Added the QT_NO_FLOAT16_OPERATORS macro in order to work
around a Microsoft <= VS2017 compiler bug that is exposed when using
std::bitset along with any Qt header that includes <qfloat16.h>.
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.
Sure, i can test for the MSVC version i guess
Uh oh!
There was an error while loading. Please reload this page.