Had a bug caused by not knowing .Add() enforced unique items:
https://buckdesign.slack.com/archives/C03A8T87B6J/p1673543406173809
Expectation is that .Add() would work like it does for other collections and not enforce unique items. Personally, if I want to check whether the RuntimeSet already contains the item, I think it's more intuitive to write the Contains() check myself wherever I'm using it.
Thus, my preferred solution is to refactor out the .Contains() checks.
Other discussed solutions included:
- Adding a bool to RuntimeSet that allows you to turn on or off whether that particular set enforce unique items.
- Adding a parameter to RuntimeSet.Add(bool enforceUnique) that allows the user to choose whether the item gets added if the setalready contains it or not
Some more Slack thoughts from Nick:
The contains check is part of the original repo we forked from. I think the idea is that many things (managers, individual GO instances, etc) could all be trying to add to the runtime set, and it's doing its own internal verification. But yeah, could use a "safety off" kind of checkbox.
Like, for example in some hypothetical game, if a pooler with a bunch of GOs is instantiating enemies and we need a list of enemies in the game, you can just call Add() / Remove() on the enemy class itself without worry of previous instances being there, etc.
Just sort of a convenience thing. But not helpful here haha
Had a bug caused by not knowing .Add() enforced unique items:
https://buckdesign.slack.com/archives/C03A8T87B6J/p1673543406173809
Expectation is that .Add() would work like it does for other collections and not enforce unique items. Personally, if I want to check whether the RuntimeSet already contains the item, I think it's more intuitive to write the Contains() check myself wherever I'm using it.
Thus, my preferred solution is to refactor out the .Contains() checks.
Other discussed solutions included:
Some more Slack thoughts from Nick:
The contains check is part of the original repo we forked from. I think the idea is that many things (managers, individual GO instances, etc) could all be trying to add to the runtime set, and it's doing its own internal verification. But yeah, could use a "safety off" kind of checkbox.
Like, for example in some hypothetical game, if a pooler with a bunch of GOs is instantiating enemies and we need a list of enemies in the game, you can just call Add() / Remove() on the enemy class itself without worry of previous instances being there, etc.
Just sort of a convenience thing. But not helpful here haha