QueryEntityViewsAsArray null reference becase its size is changing during iteration.

[Darelbi]

Ok I found a really minor bug, but maybe it can be useful, maybe it was just a undocumented behaviour

Reproducing the bug:

        int count;
        var views = entityViewsDB.QueryEntityViewsAsArray< GeometrySourceView>(out count);

        for( int i=0; i< count; i++)
        {
            entitiesSink.RemoveEntity( views[i].id.ID); // null reference at some point
        }

When iterating a array of entities taken from the entityViewsDB if I remove them in a loop I get a null reference exception. I didn't investigated yed but I assume the array is resized as long as entities are removed. I expected this kind of stuff to happen only during "SubmitEntityViews" phase.

How I fixed it (basically copying the array locally)

        var views = entityViewsDB.QueryEntityViews< GeometrySourceView>().ToArray(); // linq copy

        for( int i=0; i< views.Count(); i++)
        {
            entitiesSink.RemoveEntity( views[i].id.ID);
        }

Also what's the difference between QueryEntityViews and QueryEntityViewsAsArray? when I should prefer one to the other?

In both cases the array is resized dynamically. Now, using "AsArray" allowed me to find the bug early because of null reference (but in the second code snippet, if I remove the ".ToArray()" I will stop getting the error (of course because "view.Count" changed accordingly) and noticing the bug become harder.

[sebas77]

The array version is to be used for performance critical code where you wouldn't remove the entity very likely :)

[sebas77]

Also you don't need the ToArray

[sebas77]

To avoid these problems I could remove the entities the next frame like I do for the adding

[Darelbi]

Without the ToArray not all entities are removed. Because even though the "Count" is updated, the index "i" will miss some entities. (just tested it)

[sebas77]

Yes of course. The solution would be to do i-- but it makes sense only if you assume how the entities are removed. I have already wrote a method to remove all the entities from a group, I could do one to remove all the entities built from a specific entity descriptor which is not what you are doing. Then again what is a entity? It's a collection of data with an unique id, therefore is probably wrong to identify a set of entities from a entity view type (beside different entity descriptors could generate the same entity view!) . You should know instead the list of entity ids to delete. If in your, maybe atypical, case you want to know the ids from the entity views, you should fill an array of ids first. It's more or less what you did with ToArray but it makes more sense.

…

On Sun, 18 Mar 2018, 23:11 Darelbi, ***@***.***> wrote: Without the ToonArray not all entities are removed. Because even though the "Count" is updated, the index "i" will miss some entities. (just tested it) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#18 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA5s48fcTfikcAsAD7acWp7-nr4JR0zNks5tfumcgaJpZM4SvLPX> .

[sebas77]

do you think this is still an issue @Darelbi ?

[Darelbi]

Nope Sorry didnt have time to respond. Il giovedì 22 marzo 2018, Sebastiano Mandalà <notifications@github.com> ha scritto:

…

do you think this is still an issue @Darelbi <https://github.com/darelbi> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADUeE2UQzheE-XqsVHka8VmkFbe1oxTVks5tg3BpgaJpZM4SvLPX> .