Requesting TreeItem::removeItem

If you found a bug in our library or on our website, please report it in this section. In this forum you can also make concrete suggestions or feature requests.

Moderators: CEGUI MVP, CEGUI Team

tiktak
Just popping in
Just popping in
Posts: 4
Joined: Sun Jun 15, 2008 13:07
Contact:

Requesting TreeItem::removeItem

Postby tiktak » Sun Jun 15, 2008 13:38

I took notice of this project a short while ago (starting with 0.6.0 ^.^), and I am really impressed so far. But just now I wanted to implement a nice hierarchy browser using the CEGUI::Tree widget, and I hit a limitation: it seems TreeItem has currently no means of removing child items!

Lamentably, Tree and TreeItem do not have a unified interface with respect to their child items (sadly even for a generalized function using addItem() this also implies writing 2 functions with just different class names for the parameters / or possibly a template).
In any case, Tree has this remove functionality (Tree::removeItem() ) but it works only on TreeItems in the highest level, a call on a child-items will not remove anything (aside from the fact that this required some dirty const-removal casting and getOwnerWindow() abuse to even test this..).

Correct me if I am wrong and there is another, less obvious, way of removing child items! Otherwise, a TreeItem::removeItem(const TreeItem *item) would be really useful and needed for any decent tree implementation :)

User avatar
CrazyEddie
CEGUI Project Lead
Posts: 6760
Joined: Wed Jan 12, 2005 12:06
Location: England
Contact:

Postby CrazyEddie » Mon Jun 16, 2008 08:31

Hi, welcome, and thanks for raising this issue/request.

Somebody will investigate the possibilities shortly :)

CE.

tiktak
Just popping in
Just popping in
Posts: 4
Joined: Sun Jun 15, 2008 13:07
Contact:

Postby tiktak » Thu Jul 24, 2008 21:45

Like dewyatt pointed out, the code was very easy to read and I managed to add the functionality with the following patches (they seem to run fine in my cases (removing children with open child items, etc) but of course I may have missed some test cases).
The removing-all-items function (called resetList() and resetList_impl() in Tree) should be also easy to transfer (but resetList() seems like an imperfect name to me for child items, so I did not touch it ;) )

Functions are transferred from Tree, and i used the rationale comparing addItem() in both variants seemed to suggest.

src/elements/CEGUITreeItem.cpp

Code: Select all

186a187,214
>    void TreeItem::removeItem(const TreeItem* item)
>    {
>       if (item != 0)
>       {
>          Tree *parentWindow = (Tree *)getOwnerWindow();
>          LBItemList::iterator pos = std::find(d_listItems.begin(), d_listItems.end(), item);
>          if (pos != d_listItems.end())
>          {
>             (*pos)->setOwnerWindow(0);
>
>             d_listItems.erase(pos);
>
>             if (item == parentWindow->d_lastSelected)
>             {
>                parentWindow->d_lastSelected = 0;
>             }
>
>             if (item->isAutoDeleted())
>             {
>                delete item;
>             }
>
>             WindowEventArgs args(parentWindow);
>             parentWindow->onListContentsChanged(args);
>          }
>       }
>    }
>


include/elements/CEGUITreeItem.h

Code: Select all

479a480
>    void  removeItem(const TreeItem* item);


EDIT: btw, patches are against the 0.6.1 tree

User avatar
CrazyEddie
CEGUI Project Lead
Posts: 6760
Joined: Wed Jan 12, 2005 12:06
Location: England
Contact:

Postby CrazyEddie » Fri Jul 25, 2008 08:50

Hi,

Thanks for the patches. I'll probably add them over the weekend. I've actually marked the existing Tree as deprecated recently, although it will remain until a replacement is ready :)

CE.

tiktak
Just popping in
Just popping in
Posts: 4
Joined: Sun Jun 15, 2008 13:07
Contact:

Postby tiktak » Fri Jul 25, 2008 22:45

Thanks, I think a more consistent Tree implementation would be a real gain.
Another suggestion to make the current one more usable:

Move the const TreeItem* getItemAtPoint(const Point& pt) const from protected to public. It makes sense to export this function, for example to enable item-based popups or item-dependent context menu with the right mouse button (my use-case).

Far from wanting to save a deprecated class - but with those changes you can at least do most things you can do with a normal Gtk/Qt/whatever Tree :)

include/elements/CEGUITree.h

Code: Select all

491a492,502
>
>
>       /*!
>       \brief
>               Return the TreeItem under the given window local pixel co-ordinate.
>
>       \return
>               TreeItem that is under window pixel co-ordinate \a pt, or NULL if no
>               item is under that position.
>       */
>       TreeItem*       getItemAtPoint(const Point& pt) const;
637,645d647
<       /*!
<       \brief
<               Return the TreeItem under the given window local pixel co-ordinate.
<
<       \return
<               TreeItem that is under window pixel co-ordinate \a pt, or NULL if no
<               item is under that position.
<       */
<       TreeItem*       getItemAtPoint(const Point& pt) const;

User avatar
CrazyEddie
CEGUI Project Lead
Posts: 6760
Joined: Wed Jan 12, 2005 12:06
Location: England
Contact:

Postby CrazyEddie » Mon Jul 28, 2008 08:51

Hi,

Thanks again :)

I did not get around to committing the patches over the weekend, but will do it now (well, in a few minutes or so ;) ).

CE.

User avatar
CrazyEddie
CEGUI Project Lead
Posts: 6760
Joined: Wed Jan 12, 2005 12:06
Location: England
Contact:

Postby CrazyEddie » Mon Jul 28, 2008 10:18

Ok, I have committed these patches now. The only thing is that the second patch got put into trunk as opposed to branches/v0-6, this is because the change would break the binary interface (at least for some compilers) which I'm trying very hard to avoid on the v0-6 line of code :)

Btw, if you could make any future patches in unified format, that would be excellent :)

CE.

tiktak
Just popping in
Just popping in
Posts: 4
Joined: Sun Jun 15, 2008 13:07
Contact:

Postby tiktak » Wed Oct 08, 2008 03:42

Yeah... way to followup on an old thread - sorry in advance :D

I made another patch for the "old" (=0.6.1) Tree, which fixes the setItemSelectState(bool) function (in the Trunk, the check whether the item is in the Tree is non-recursive, hence the setItemSelectState fails on non-firstlevel sub-children with an Exception).

This could now be properly used to select a TreeItem programmatically (e.g. search result highlighting, or highlighting an item clicked by mouse button different from the left one).

I think it should not break ABI since I just added a new protected function and changed some actual code. If you reject it for the trunk - I will just use it as my own private patched version ;)

<3 ,
tiktak

P.S. The existing provision that only visible items be considered for selection is still valid after this patch. And, I have not had anyone proofread this yet (SORRY) but it seemed to work great for me.

Code: Select all

--- src/elements/CEGUITree.cpp.orig   2008-09-17 04:56:45.000000000 +0200
+++ src/elements/CEGUITree.cpp   2008-09-17 06:08:53.000000000 +0200
@@ -550,16 +550,38 @@
    *************************************************************************/
    void Tree::setItemSelectState(TreeItem* item, bool state)
    {
-      LBItemList::iterator pos = std::find(d_listItems.begin(), d_listItems.end(), item);
-
-      if (pos != d_listItems.end())
-      {
-         setItemSelectState(std::distance(d_listItems.begin(), pos), state);
+      bool found = containsOpenItemRecursive(d_listItems, item);
+      if (found) {
+         TreeEventArgs args(this);
+         args.treeItem = item;
+         if (state && !d_multiselect) {
+            clearAllSelections_impl();
+         }
+         item->setSelected(state);
+         d_lastSelected = item->isSelected() ? item : 0;
+         onSelectionChanged(args);
+      } else {
+         throw InvalidRequestException((utf8*)"Tree::setItemSelectState - the specified TreeItem is not attached to this Tree or not visible.");
       }
-      else
+   }
+
+   bool Tree::containsOpenItemRecursive(const LBItemList& itemList, TreeItem* item) {
+      size_t itemCount = itemList.size();
+      for (size_t index = 0; index < itemCount; ++index)
       {
-         throw InvalidRequestException((utf8*)"Tree::setItemSelectState - the specified TreeItem is not attached to this Tree.");
+         if (itemList[index] == item) {
+            return true;
+         }
+         if (itemList[index]->getItemCount() > 0)
+         {
+            if (itemList[index]->getIsOpen())
+            {
+               bool found = containsOpenItemRecursive(itemList[index]->getItemList(), item);
+               if (found) return true;
+            }
+         }
       }
+      return false;
    }
 
 
--- include/elements/CEGUITree.h.orig   2008-09-17 04:57:00.000000000 +0200
+++ include/elements/CEGUITree.h   2008-09-17 04:47:40.000000000 +0200
@@ -589,6 +589,13 @@
    /*************************************************************************
       Implementation Functions
    *************************************************************************/
+   
+   /*!
+   \brief
+      Checks if a tree item is visible (searches sub-items)
+   */
+   bool containsOpenItemRecursive(const LBItemList& itemList, TreeItem* item);
+   
    /*!
    \brief
       Add list box specific events

User avatar
CrazyEddie
CEGUI Project Lead
Posts: 6760
Joined: Wed Jan 12, 2005 12:06
Location: England
Contact:

Postby CrazyEddie » Thu Oct 09, 2008 08:02

Hi,

Thanks for the new patch :) I'll look it over at the weekend with a view to committing it to the v0-6 branch of code. You're right that the changes made here should not affect the ABI.

CE.

User avatar
CrazyEddie
CEGUI Project Lead
Posts: 6760
Joined: Wed Jan 12, 2005 12:06
Location: England
Contact:

Postby CrazyEddie » Sun Nov 09, 2008 17:09

Hi,

I finally applied the latest patch in branches/v0-6 at rev. 1844. It should get merged into trunk in a few days.

Thanks again :)

CE.


Return to “Bug Reports, Suggestions, Feature Requests”

Who is online

Users browsing this forum: No registered users and 10 guests