UmbraLuminosa / sickle_ui

A widget library built on top of bevy_ui.
Apache License 2.0
205 stars 24 forks source link

- Renamed variables, removed unused logic, and commented code. #6

Closed danec020 closed 3 months ago

danec020 commented 3 months ago

Added comments and renamed variable to the previous changes to make it more understandable. I found using the longer variable names made it much easier to follow the flow of the logic due to the finicky nature of dealing with children's children of parent's children ect...

This part was removed from IF statement to see how many sized_zones are children of the tab's parent but could be used later if you need to propagate other type of widgets up to the parent of the removed zone?

//If nothing is found then the docking_zone can be removed. NOTE: This doesn't mean
//there aren't children inside that are of a different type than sized_zones.
if child_zones_count.len() == 0 {
    if let Ok(tz_parents_parent) = q_parent.get(tab_zone_parent_id) {
        let tz_parents_parent = tz_parents_parent.get();
        //Find out what index the tab's parent is so we can remove it (Contains all Tab elements)
        let removal_index = q_children
            .get(tz_parents_parent)
            .unwrap()
            .iter()
            .position(|child| *child == tab_zone_parent_id)
            .unwrap();

        Now that we have the index that is going to be removed, move it's children up.
        Earlier we only looked for children of sized_zones under this parent, not other types.
        for child in tz_parent_children {
            //Ignore the tab_zone that is going to be removed, it is no longer needed.
            if *child == tab_zone.zone { continue; }

            //Put the children at the index this zone use to be in
            //*NOTE: q_sized_zones in this example has already be searched on line 84 and
            //we have determined child cound is 0, therefore this can never happen. But
            //this example may be useful if you were looking for something OTHER than a
            //zone to throw these children in? 
            if q_sized_zones.get(*child).is_ok() {
                commands
                    .entity(tz_parents_parent)
                    .insert_children(removal_index, &[*child]);
            }
        }
    }
    commands.entity(tab_zone_parent_id).despawn_recursive();
    despawn_zone = false;
}
danec020 commented 3 months ago

The new changes submitted should address all the concerns mentioned by @eidloi.

NOTE: There is a sliced_test.rs and simple_test.rs example that I have added to my fork that can be used to test this edge case. When merging, these files can be ignored but may be helpful when confirming.

Now all remaining children who are not related to the requested zone will shift up if necessary. This creates it's own issues since the UI layout will possibly change. The result will be a sliced zone that has a sliced zone child containing only a label. When this happens the child will be removed and the label will shift up to the parent sliced zone. This seems like the logical solution but may need to be reconsidered?

Also with these changes, removed zones will grab their SizedZoneResizeHandleContainer children so they don't clutter up the hierarchy after the zone is despawned.

danec020 commented 3 months ago

No problem. Sorry to have wasted your time.

eidloi commented 3 months ago

No problem. Sorry to have wasted your time.

You didn't waste it mate! Your research is valuable, and will contribute to the overall fix!