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

Adding support for list property types #4002

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

bjorn
Copy link
Member

@bjorn bjorn commented Jul 10, 2024

Implementation so far:

  • Added 'list' as option to the Add Property dialog, for adding a QVariantList.
  • Adjusted the CustomPropertiesHelper to create sub-properties that display the values in the list (maybe should rather be done by the VariantPropertyManager).
    • It is not done by VariantPropertyManager because it couldn't delegate class member creation back to the CustomPropertiesHelper. If VariantPropertyManager should handle lists, then it would need to handle classes as well.
  • Added support for saving and loading list properties to TMX and JSON formats.

image

  • Added a tool button with menu that allows to add new list items.

image

ToDo:

  • Ability to add items to a list
  • Ability to remove items from a list
  • Editing list items
  • Reordering list items

Resolves #1493

* Added 'list' as option to the Add Property dialog, for adding a
  QVariantList.

* Adjusted the CustomPropertiesHelper to create sub-properties that
  display the values in the list (maybe should rather be done by the
  VariantPropertyManager).

* Added support for saving and loading list properties to TMX and JSON
  formats.
* VariantPropertyManager now displays the number of list items.

* VariantEditorFactory now instantiates a ListEdit widget when a list
  value is selected, which features a button to add an item to the list
  (currently just "0" integer values).
* There is now menu alongside the menu to add new values:

  - Choosing from the menu adds a value of the selected type
  - Clicking the button now adds the same type as the last item, or
    spawns the menu (for empty lists)

* Saving in JSON format now supports lists of lists (not yet supported
  for loading)

* When list values change, it no longer deletes and re-creates all
  sub-properties, but instead reuses the existing items. This should be
  faster, but also avoids affecting the scroll position of the view.

* Some code moved around to share code between Add Property dialog and
  the new menu for adding list items.
Also switched from "value" to "item" elements.
In case there is no editor widget. Also avoids a warning about trying to
add nullptr as widget.
* When changing a class member of a class that is either directly or
  indirectly part of a list, it is now emitted as a change of the
  top-most list property. This is because there is currently no more
  specific way to apply such changes.

* The above is now also taken into account by the Reset action.

* List properties are now re-created as necessary when their type
  changes.

* Class members are now correctly marked as modified when the class is
  a list value.
@bjorn
Copy link
Member Author

bjorn commented Jul 18, 2024

Urgh, turned out the "Ability to add items to a list" is only mostly working. When just pressing the "+ Add" button instead of using the menu, it is not possible to add items of the same custom enum or class type. This is because the values are at that point converted to their "display value", which means a class is just a map and an enum is just a number, and only the CustomPropertiesHelper knows the associated type.

Unfortunately, VariantEditorFactory::createEditor is currently unable
to check whether the value is part of a list or not.
@@ -196,12 +203,14 @@ QWidget *VariantEditorFactory::createEditor(QtVariantPropertyManager *manager,
if (!editor)
editor = QtVariantEditorFactory::createEditor(manager, property, parent);

if (type == QMetaType::QColor || type == VariantPropertyManager::displayObjectRefTypeId() || property->isModified()) {
if (true || type == QMetaType::QColor || type == VariantPropertyManager::displayObjectRefTypeId() || property->isModified()) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem here is the if (true of course. It should be replaced with a check on whether the property is a value in a list (so whether its parent is a list property). Unfortunately, we don't have access to the parent property (they're private, and in theory there could even be multiple in this properties framework).

If we find a way to make this work, it should of course also only determine the visibility of the "Delete" button, and not affect the visibility of the "Reset" button.

@bjorn bjorn mentioned this pull request Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Being able to have an array custom property
1 participant