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

[Editor] Merge duplicate entries in enum property inspector #96386

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Aug 31, 2024

This helps with confusion over how selecting a key with a duplicate value won't be selected as only the first entry with a particular value will be selected.

See:

@timothyqiu
Copy link
Member

Disabling duplicated enumerators feels wrong to me because the underneath value is, in fact, usable. Having to use a different name defeats the purpose of giving the value an alternate name in the first place.

Maybe it's better to show duplicated enumerators as one entry. For example, for this enum

enum TestEnum2 { TEST = 1, TEST2 = 0, TEST3 = 1 }

show these two options:

  • Test, Test3
  • Test2

@AThousandShips
Copy link
Member Author

Can try that, bit more convoluted code, but unsure if it's not more confusing, the underlying issue here is that people get confused when clicking Test 3 it activates Test 1 and there's no clear indicator of why

But will test that, it'll be a bunch of additional code but should be easy

@AThousandShips
Copy link
Member Author

Pushed an alternative solution, however not sure how enum keys work with RTL languages in the interface so it might be weird with RTL languages being used, but someone knowledgeable in that can probably answer that

@timothyqiu
Copy link
Member

not sure how enum keys work with RTL languages in the interface

Localization of enum dropdowns in the inspector is currently disabled. We have not implemented string extraction for hint strings in editor codebase yet.

For user-defined texts like this, I think it's better to leave them untranslated.

@AThousandShips
Copy link
Member Author

AThousandShips commented Aug 31, 2024

Was more like how ", " would work if the individual hints are in an RTL language, but I don't know how the dropdown even behaves if a key is in RTL anyway so might not be a concern, i.e. if you do جَزِيرَة, بُحَيْرَة, it seem that it renders the same way as this does here (at least to my browser, not set to Arabic, I just took two Arabic words as an example I don't speak Arabic)

But I think that's probably a future problem to evaluate for the editor interface and not critical here

@AThousandShips AThousandShips changed the title [Editor] Disable enum property items with duplicate values [Editor] Merge duplicate entries in enum property inspector Sep 12, 2024
This helps with confusion over how selecting a key with a duplicate
value won't be selected as only the first entry with a particular value will
be selected.
@akien-mga akien-mga modified the milestones: 4.x, 4.4 Sep 13, 2024
@akien-mga akien-mga merged commit ac652cf into godotengine:master Sep 13, 2024
20 checks passed
@AThousandShips AThousandShips deleted the enum_inspector_improve branch September 13, 2024 09:29
@akien-mga
Copy link
Member

Thanks!

@AThousandShips
Copy link
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants