Question about SparseSet insert else case

[notinmybackyaard]

Hello leudz,

I am using Shipyard 0.6.2 and have a question about it. In the sparse_set/mod.rs file, there is an insert function in the SparseSet. In the code, under what circumstances does the else statement get executed? Is this scenario considered normal? To track if there is a case where the insert function is called but data is not added, I inserted panic code, and although I'm not exactly sure about the scenario, the panic occurred at that point, which is why I'm asking.

impl<T: Component> SparseSet<T> {
    /// Inserts `value` in the `SparseSet`.
    ///
    /// # Tracking
    ///
    /// In case `entity` had a component of this type, the new component will be considered `modified`.  
    /// In all other cases it'll be considered `inserted`.
    pub(crate) fn insert(&mut self, entity: EntityId, value: T, current: u64) -> Option<T> {
        self.sparse.allocate_at(entity);

        // at this point there can't be nothing at the sparse index
        let sparse_entity = unsafe { self.sparse.get_mut_unchecked(entity) };

        let old_component;

        if sparse_entity.is_dead() {
            *sparse_entity =
                EntityId::new_from_index_and_gen(self.dense.len() as u64, entity.gen());

            if T::Tracking::track_insertion() {
                self.insertion_data.push(current);
            }
            if T::Tracking::track_modification() {
                self.modification_data.push(0);
            }

            self.dense.push(entity);
            self.data.push(value);

            old_component = None;
        } else if entity.gen() >= sparse_entity.gen() {
            let old_data = unsafe {
                core::mem::replace(self.data.get_unchecked_mut(sparse_entity.uindex()), value)
            };

            if entity.gen() == sparse_entity.gen() {
                old_component = Some(old_data);
            } else {
                old_component = None;
            }

            sparse_entity.copy_gen(entity);

            let dense_entity = unsafe { self.dense.get_unchecked_mut(sparse_entity.uindex()) };

            if T::Tracking::track_modification() {
                unsafe {
                    *self
                        .modification_data
                        .get_unchecked_mut(sparse_entity.uindex()) = current;
                }
            }

            dense_entity.copy_index_gen(entity);
        } else {
            // Here!!
            old_component = None;
        }

        old_component
    }
}

[leudz]

Hi, I definitely wouldn't say it's normal to get to the else block.

In theory you should almost only enter the if block. You add an entity or component and there is no component in the storage. The else if block can be entered in normal scenarios if you override a component, entity.gen() == sparse_entity.gen(). This block can also trigger for a less normal execution when entity.gen() > sparse_entity.gen(), that would mean you didn't deleted all components of an entity. It's still okay.

The else block is accessed when you have an entity1, you delete entity1 and create a new entity2. Then you add component T to entity2 and then try to add component T to entity1.

---

Of course it could be a bug in shipyard, hopefully not.

[notinmybackyaard]

Okay, I'll check my whole logic, thx!