-
-
Notifications
You must be signed in to change notification settings - Fork 470
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
Small cosmetic wind adjustments #2834
base: master
Are you sure you want to change the base?
Conversation
src/object/sprite_particle.hpp
Outdated
@@ -39,6 +39,8 @@ class SpriteParticle final : public GameObject | |||
int drawing_layer = LAYER_OBJECTS-1, bool notimeout = false, Color color = Color::WHITE); | |||
~SpriteParticle() override; | |||
|
|||
SpritePtr& get_sprite() { return sprite; } |
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.
Why is this a SpritePtr reference?
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.
Why not?
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.
You can make this const if you return SpritePtr
(which is std::unique_ptr<Sprite>
) instead of SpritePtr&
. It doesn't change anything.
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.
Maybe Sprite& get_sprite() { return *sprite; }
would be better?
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.
You can make this const if you return
SpritePtr
(which isstd::unique_ptr<Sprite>
) instead ofSpritePtr&
. It doesn't change anything.
Doing it this way creates many errors.
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.
"creates many errors"...WHAT errors?
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 a unique_ptr
, so copy issues I'd assume.
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.
Many copies of this:
"error C2280: 'std::unique_ptr<Sprite,std::default_delete>::unique_ptr(const std::unique_ptr<Sprite,std::default_delete> &)': attempting to reference a deleted function"
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.
Or maybe return a SpritePtr
instance by using std::unique_ptr::get()
This will fail if sprite ever is NULL (null dereference). |
@mrkubax10 Were you so certain that sprite could never be null that you deref'ed it without checking? |
If you return a pointer instead it will fail in other place. |
Not when there's suitable null checks. |
This PR makes the wind particles point in the proper direction now, as well as putting wind on a layer behind others so that you can actually grab other objects in the editor when there's wind in the level.