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

Add add_item method to Tree Node #708

Closed
BrodyEller opened this issue Apr 14, 2020 · 6 comments
Closed

Add add_item method to Tree Node #708

BrodyEller opened this issue Apr 14, 2020 · 6 comments

Comments

@BrodyEller
Copy link

BrodyEller commented Apr 14, 2020

Describe the project you are working on:
GUI based app where markers can be added and organized into a tree structure, displayed in a Tree node. Each marker contains some information that gets serialized to a json file on save. Originally I used an ItemList and array to store the markers but now I want the user to be able to organize them into groups using a Tree node.

Describe the problem or limitation you are having in your project:
Tree has the method create_item which creates and returns a TreeItem object. While this works well, I have extra information I want to store in each TreeItem. To do this, I'd like to subclass TreeItem using extends TreeItem, add my properties, and use the custom class almost exactly how I would a normal TreeItem. I cannot add a custom type to my Tree node however since create_item creates a TreeItem node for me, instead of allowing me to use a custom type.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Add another method to the Tree class called add_item which works very similar to create_item but has a parameter to pass in an instance of a custom type that extends TreeItem.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Something like this, may or may not work, I just altered the current create_item method real quick:

void Tree::add_item(TreeItem *p_item, TreeItem *p_parent, int p_idx) {

	ERR_FAIL_COND_V(blocked > 0, NULL);

	if (p_parent) {

		// Append or insert a new item to the given parent.
		ERR_FAIL_COND_V(!p_item, NULL);
		p_item->cells.resize(columns.size());

		TreeItem *prev = NULL;
		TreeItem *c = p_parent->children;
		int idx = 0;

		while (c) {
			if (idx++ == p_idx) {
				p_item->next = c;
				break;
			}
			prev = c;
			c = c->next;
		}

		if (prev)
			prev->next = p_item;
		else
			p_parent->children = p_item;
		p_item->parent = p_parent;

	} else {

		if (!root) {
			// No root exists, make the given item the new root.
			ERR_FAIL_COND_V(!p_item, NULL);
			p_item->cells.resize(columns.size());

			root = p_item;
		} else {
			// Root exists, append or insert to root.
			add_item(p_item, root, p_idx);
		}
	}
}

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Those who don't need this functionality can continue to use Tree just as they have before. It doesn't break any current projects.

Is there a reason why this should be core and not an add-on in the asset library?:
I'm not sure there is any workaround in pure GDScript and it is very simple to implement in the engine.

I'm willing to work on this and submit a PR, just wanted to make sure there was no GDScript solution and that this was a good way to go about this.

@BrodyEller
Copy link
Author

After trying to implement this, it seems that TreeItem can't be instantiated in GDScript since it is a virtual class. Either way, my issue can be solved using object metadata instead so closing.

@rilpires
Copy link

I understand that for your problem it had a workaround, but can we reopen this issue?
An add_item function would be useful and a complement for TreeItem::remove_child. What's the point of removing a TreeItem then?

I would like to move TreeItems and its children around between differente Trees. Without an TreeItem::add_item this is impossible, or am I missing something?

Anyway, TreeItem::remove_child documentation says:

Removes the given child TreeItem and all its children from the Tree. Note that it doesn't free the item from memory, so it can be reused later. To completely remove a TreeItem use Object.free().

so it can be reused later

How can I properly reuse it later then?

@YuriSizov
Copy link
Contributor

As for the new APIs, there is an approved PR that may interest you already: godotengine/godot#46773

@CsloudX
Copy link

CsloudX commented Nov 26, 2021

As for the new APIs, there is an approved PR that may interest you already: godotengine/godot#46773

I think it should be add_child and move_child like node's method.
LOOK:

|-TreeRoot
  |-NodeA
    |-Child
      |-AA
      |-BB
        |-ZZ
      |-CC
  |-NodeB

How can I move Child to as a NodeB's child by move_before and move_after, like this:

|-TreeRoot
  |-NodeA
  |-NodeB
    |-Child
      |-AA
      |-BB
        |-ZZ
      |-CC

@radlotus
Copy link

radlotus commented Sep 19, 2022

rilpires

The issue is still present. You can't reuse TreeItems after removing them from the Tree. You can't add them back to the Tree.

@Calinou
Copy link
Member

Calinou commented Sep 19, 2022

The issue is still present. You can't reuse TreeItems after removing them from the Tree. You can't add them back to the Tree.

Please open a new proposal based on the API used in 4.0.beta1, as this proposal refers to the old 3.x API (which won't get significant new features on its own, only backwards-compatible backports).

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

No branches or pull requests

6 participants