-
Notifications
You must be signed in to change notification settings - Fork 832
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
Keep registration order as order for context menu #163
base: master
Are you sure you want to change the base?
Conversation
@@ -57,11 +60,16 @@ class NODE_EDITOR_PUBLIC DataModelRegistry | |||
|
|||
QString const name = uniqueModel->name(); | |||
|
|||
if (_registeredModels.count(name) == 0) | |||
if (!_registeredModels.count(name)) |
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 might find this less readable, but I've seen people treating the count
method on std::map
or std::set
as "contains", which is why I wrote is as !_registeredModels.count(name)
here. It might not be desirable, if so, it's easy enough to change this back
examples/calculator This is the registration code (edited to make this comment smaller): auto ret = std::make_shared<DataModelRegistry>();
ret->registerModel<NumberSourceDataModel>("Sources");
ret->registerModel<NumberDisplayDataModel>("Displays");
ret->registerModel<AdditionModel>("Operators");
ret->registerModel<SubtractionModel>("Operators");
ret->registerModel<MultiplicationModel>("Operators");
ret->registerModel<DivisionModel>("Operators");
ret->registerModel<ModuloModel>("Operators"); Before this PR: After this PR: The categories can still be sorted after this PR by calling |
3edf358
to
15a270d
Compare
15a270d
to
a6e4ba4
Compare
…y useless in this case
Just found that version 2.1.3 still not support manually context menu settings. |
Implements #147
I kept
_categories
and_registeredModelsCategory
as-is. Someone could've called those functions and stored the variables, and removing them, but especially replacing it with a new data-type, would break their code (replacing could silently break in some cases). My ideal would've been to just change thecategories()
function to return thestd::vector
and to change theregisteredModelsCategoryAssociation
to return a separate map type that preserves insertion order (similar to LinkedHashMap in Java; I would've just combinedstd::vector
andstd::unordered_map
).If removing / changing
_categories
or_registeredModelsCategory
is desired, I'll make the change and update this PR.Note: if we do remove
categories()
, such that_categories
becomes inaccessible through any external functions, it'd be best to change its type fromstd::set
tostd::unordered_set