-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Remove unused hints #60458
Remove unused hints #60458
Conversation
I think that the enum values should be made continuous again, we can break compat in 4.0. |
PROPERTY_USAGE_NO_EDITOR = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_NETWORK, | ||
PROPERTY_USAGE_DEFAULT = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_EDITOR, | ||
PROPERTY_USAGE_DEFAULT_INTL = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_INTERNATIONALIZED, | ||
PROPERTY_USAGE_NO_EDITOR = PROPERTY_USAGE_STORAGE, |
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 just realized PROPERTY_USAGE_NO_EDITOR
is now basically an alias to PROPERTY_USAGE_STORAGE
. Should it be removed maybe?
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 think it works well to indicate that the property is hidden in the inspector and I fear doing the same with just storage is not going to be as clear.
865a5b7
to
93ee62c
Compare
core/object/object.h
Outdated
PROPERTY_USAGE_GROUP = 1 << 5, //used for grouping props in the editor | ||
PROPERTY_USAGE_CATEGORY = 1 << 6, | ||
PROPERTY_USAGE_SUBGROUP = 1 << 7, | ||
PROPERTY_USAGE_NO_INSTANCE_STATE = 8, |
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 implemented, but not used anywhere.
93ee62c
to
1ba0b37
Compare
3f72a54
to
0efd9f3
Compare
Maybe we can reorder some of them to be presented in a more organized manner? Since this is a clean-up PR anyway... |
If you have ideas for better order I'm open to suggestions. |
Yeah I've been meaning to get this thoroughly cleaned up/reordered/renamed but I can't muster the brain capacity to do a proposal for this. Related: #30203. |
Maybe we could merge this now and debate about ordering later? Rebasing is troublesome >_> |
We have until the next week and Remi returns anyway, I'll try to think of what we can improve with the order. |
0efd9f3
to
2f777b9
Compare
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.
Let's merge and do further cleanup later.
Thanks! |
PROPERTY_USAGE_DEFAULT = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_NETWORK, | ||
PROPERTY_USAGE_DEFAULT_INTL = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_NETWORK | PROPERTY_USAGE_INTERNATIONALIZED, | ||
PROPERTY_USAGE_NO_EDITOR = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_NETWORK, | ||
PROPERTY_USAGE_STORAGE = 1 << 1, |
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.
1 << 1
is 2
, not 1
. Should have started with 1 << 0
, but now it's too late to fix it.
No description provided.