Skip to content
New issue

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

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

Already on GitHub? Sign in 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants