Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make RandomE support IReadOnlyList<T> #69

Closed
wants to merge 2 commits into from
Closed

Conversation

bluexo
Copy link

@bluexo bluexo commented Dec 15, 2020

This PR make RandomE support IReadOnlyList

GetReadOnlyRandom(this IReadOnlyList list);
GetReadOnlyRandom(this IReadOnlyList list, IReadOnlyList weights);

@BasmanovDaniil
Copy link
Member

Hello! Thanks for the contribution, good idea. A few years back I didn't want to reference LINQ library, to minimize dependencies and memory footprint. I think I might have used it in one of the examples anyway, I will need to investigate it a bit. The situation with Unity is completely different now, maybe I will just go with it. I will try to take a look at it this weekend, but might have to postpone it to after new year celebrations.

@BasmanovDaniil
Copy link
Member

Hey @bluexo! Finally was able to take a look at this idea.
Seems like it is a fairly common and complex issue: dotnet/runtime#31001
The simplest solution I see is to just replace IList with IReadOnlyList in the relevant methods, it shouldn't break much.
But I have some other ideas that might be just as valid, can you provide an example of your use case?

@bluexo
Copy link
Author

bluexo commented Jan 7, 2021

Hey @BasmanovDaniil! Thanks for your reply, I often use the List like this:

  public SomeClass : MonoBehaviour
  {
          public IReadonlyList<int> ListProperty => listField;
          [SerializeField]
          private List<int> listField;
  }

So , I don't want someone modify the private field.

BasmanovDaniil added a commit that referenced this pull request Jan 7, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants