-
Notifications
You must be signed in to change notification settings - Fork 93
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
Support removing the actor, light, or model from the root #1492
base: sdf14
Are you sure you want to change the base?
Conversation
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
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.
looks good; I suggested some small, optional improvements to the test
sdf::Actor actor1; | ||
actor1.SetName("actor1"); | ||
root.SetActor(actor1); | ||
EXPECT_NE(nullptr, root.Actor()); |
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.
EXPECT_NE(nullptr, root.Actor()); | |
EXPECT_NE(nullptr, root.Actor()); | |
EXPECT_EQ(nullptr, root.Light()); | |
EXPECT_EQ(nullptr, root.Model()); |
let's check all three pointers, expecting at most one to be non-null
root.SetActor(actor1); | ||
EXPECT_NE(nullptr, root.Actor()); | ||
root.ClearActorLightModel(); | ||
EXPECT_EQ(nullptr, root.Actor()); |
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.
EXPECT_EQ(nullptr, root.Actor()); | |
EXPECT_EQ(nullptr, root.Actor()); | |
EXPECT_EQ(nullptr, root.Light()); | |
EXPECT_EQ(nullptr, root.Model()); |
root.SetModel(model1); | ||
EXPECT_NE(nullptr, root.Model()); | ||
root.ClearActorLightModel(); | ||
EXPECT_EQ(nullptr, root.Model()); |
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.
EXPECT_EQ(nullptr, root.Model()); | |
EXPECT_EQ(nullptr, root.Actor()); | |
EXPECT_EQ(nullptr, root.Light()); | |
EXPECT_EQ(nullptr, root.Model()); |
root.SetLight(light1); | ||
EXPECT_NE(nullptr, root.Light()); | ||
root.ClearActorLightModel(); | ||
EXPECT_EQ(nullptr, root.Light()); |
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.
EXPECT_EQ(nullptr, root.Light()); | |
EXPECT_EQ(nullptr, root.Actor()); | |
EXPECT_EQ(nullptr, root.Light()); | |
EXPECT_EQ(nullptr, root.Model()); |
sdf::Light light1; | ||
light1.SetName("light1"); | ||
root.SetLight(light1); | ||
EXPECT_NE(nullptr, root.Light()); |
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.
EXPECT_NE(nullptr, root.Light()); | |
EXPECT_EQ(nullptr, root.Actor()); | |
EXPECT_NE(nullptr, root.Light()); | |
EXPECT_EQ(nullptr, root.Model()); |
sdf::Model model1; | ||
model1.SetName("model1"); | ||
root.SetModel(model1); | ||
EXPECT_NE(nullptr, root.Model()); |
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.
EXPECT_NE(nullptr, root.Model()); | |
EXPECT_EQ(nullptr, root.Actor()); | |
EXPECT_EQ(nullptr, root.Light()); | |
EXPECT_NE(nullptr, root.Model()); |
🎉 New feature
Summary
The Root object didn't have a mechanism to clear/remove the actor, light, or model. This PR adds a
Clear
function that similar to the otherClear
functions.Test it
Tests have been added.
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.