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

I think the code copies the entire loaded mods list or something #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

art0007i
Copy link
Contributor

@art0007i art0007i commented Oct 5, 2024

not sure why it would do that actually

also update doc because it does something else than it says

I was also thinking that this method could be a property, but I think there is no way to easily do that without breaking compatibility with previous mods (although I'm not sure if there is any that use this function)

@XDelta XDelta added the enhancement New feature or request label Oct 6, 2024
@Nytra
Copy link

Nytra commented Nov 17, 2024

It probably gives you a new list so you can't modify the original one

@art0007i
Copy link
Contributor Author

It probably gives you a new list so you can't modify the original one

I thought so too but the return type is IEnumerable which cannot be modified by itself.

@art0007i
Copy link
Contributor Author

Alternatively I could change the return type of this method and use this function. I think it would make more sense to use this.

Changing a return type like this would mean that the change is source compatible but would require a recompile of every mod that used that function unfortunately

@XDelta
Copy link
Member

XDelta commented Nov 19, 2024

Would need to poke at more if there was further implications or breakage but the mod list isn't that large of an allocation and the few mods that call it don't do so too often. I have used a build just swapping it and the mods continued working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants