Skip to content
This repository has been archived by the owner on Aug 22, 2018. It is now read-only.

GameSystemCollection.Remove doesn't remove item from underlying updateableGameSystems and drawableGameSystems lists #666

Open
guygodin opened this issue Apr 13, 2018 · 4 comments

Comments

@guygodin
Copy link

guygodin commented Apr 13, 2018

The AddGameSystem call specifies a ProfilingKey as the KeyValuePair.Value while the Remove call specifies null as the value; therefore the item doesn't get removed.

var key = new KeyValuePair<IUpdateable, ProfilingKey>(updateableSystem, null);

var key = new KeyValuePair<IDrawable, ProfilingKey>(drawableSystem, null);

@Kryptos-FR
Copy link

First of all, congrats for the #666 issue 😈

Secondly, could you update the description and provide a link to the related file(s) in the sources?

@guygodin
Copy link
Author

Woah wicked! Links added to issue.

@stefnotch
Copy link

stefnotch commented Apr 13, 2018

More info regarding this issue:

The AddGameSystem call specifies a ProfilingKey as the KeyValuePair.Value

gameSystemKey = new KeyValuePair<T, ProfilingKey>(gameSystemKey.Key, new ProfilingKey(parentProfilingKey, gameSystem.GetType().Name));

Also, according to this answer, "[A KeyValuePair] is a struct. This means it uses the default value equality. This simply compares the values of the fields to test for equality." So, this is almost certainly a bug.

Lastly,updateableGameSystems is a normal C# list instead of a seemingly more fitting dictionary?

@guygodin
Copy link
Author

It should be a list since it is iterated over a lot (iterating over lists performs better than dictionaries). The solution is to iterate over the items of the list in the Remove method and check for KeyValuePair.Key equality like it’s done elsewhere in my opinion.

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

No branches or pull requests

3 participants