Skip to content
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

Add move/insert to repeated protomodel & add shader and timeline editors #79

Merged
merged 3 commits into from
Dec 21, 2019
Merged

Conversation

fundies
Copy link
Contributor

@fundies fundies commented Dec 18, 2019

No description provided.

Copy link
Member

@JoshDreamland JoshDreamland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. (Feel free to merge as-is.)


StackedCodeWidget::StackedCodeWidget(QWidget* parent) : QStackedWidget(parent) {}

void StackedCodeWidget::newSource() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this happen a lot? Consider extracting a template that delegates methods to its type.

@@ -12,7 +12,7 @@ class PathEditor : public BaseEditor {

public:
explicit PathEditor(ProtoModelPtr model, QWidget* parent);
~PathEditor();
~PathEditor() override;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very positive style of change... it'd be nice if we could get clang-tidy checks for stuff like this. Are we building with -Wall?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope.

-- 'CMAKE_C_FLAGS_DEBUG': -g
-- 'CMAKE_C_FLAGS_MINSIZEREL': -Os -DNDEBUG
-- 'CMAKE_C_FLAGS_RELEASE': -O3 -DNDEBUG
-- 'CMAKE_C_FLAGS_RELWITHDEBINFO': -O2 -g -DNDEBUG
-- 'CMAKE_CXX_FLAGS_DEBUG': -g -fno-omit-frame-pointer -fsanitize=address
-- 'CMAKE_CXX_FLAGS_MINSIZEREL': -Os -DNDEBUG
-- 'CMAKE_CXX_FLAGS_RELEASE': -O3 -DNDEBUG
-- 'CMAKE_CXX_FLAGS_RELWITHDEBINFO': -O2 -g -DNDEBUG


cursorPositionLabel = new QLabel();
this->updateCursorPositionLabel(ui->roomPreview->widget()->cursor().pos());
assetNameLabel = new QLabel("obj_xxx");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a placeholder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that was there before. robert wrote that

@@ -114,7 +121,8 @@ void SoundEditor::on_loadButton_clicked() {
// QString newData = GetModelData(Sound::kDataFieldNumber).toString();
// TODO: Copy data into our egm and reset the path
// SetModelData(Sound::kDataFieldNumber, lastData);
} else qDebug() << "Failed to load gmx sound";
} else
qDebug() << "Failed to load gmx sound";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did clang-format do this? It should have put {} around this else clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it did but i dont think clang format puts brackets

case CppType::CPPTYPE_BOOL:
refl->SetBool(protobuf, field, value.toBool());
break;
case CppType::CPPTYPE_INT32: refl->SetInt32(protobuf, field, value.toInt()); break;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am struggling to believe that clang-format did this, but I don't mind it.


connect(ui->stepBox, QOverload<int>::of(&QSpinBox::valueChanged), [=](int value) { CheckDisableButtons(value); });

connect(ui->changeMomentButton, &QPushButton::pressed, [=]() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these lambdas are big enough to extract into members.

@JoshDreamland JoshDreamland merged commit 30340ac into enigma-dev:master Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants