-
Notifications
You must be signed in to change notification settings - Fork 203
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
Include MeshShape class #1064
Include MeshShape class #1064
Conversation
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
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.
I've left two relatively minor comments. Once those are fixed, I'm happy with this.
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.
This is a minor point, but I kind of feel like this file should be in the objects
subdirectory. That is, it seems to be an object in its own right, and basically everything else that derives from Shape
is in the object subdirectory.
@ahcorde What do you think?
namespace Ogre | ||
{ | ||
class ManualObject; | ||
} |
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.
Instead of doing this, can we just #include
the appropriate Ogre header? We should also probably add includes for SceneNode, Vector3, SceneManager, etc in here as well.
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
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.
This looks good to me with green CI.
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Related with this issue: #763