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

[KK][GT] Cache temporary arrays in ChaControl.UpdateVisible #47

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

mosirnik
Copy link
Contributor

This patch caches the 1D and 2D temporary arrays created in UpdateVisible.

The transpiler is a little blunt, replacing all newarr instructions, so I included a check to ensure that we patch exactly 45 places. This should make it a little more robust against other plugins transpiling the same method.

I wrote some code (not included here) to check that all 45 arrays are indeed safe to cache.

This patch caches the 1D and 2D temporary arrays created in
UpdateVisible.

The transpiler is a little blunt, replacing all newarr instructions,
so I included a check to ensure that we patch exactly 45 places.
This should make it a little more robust against other plugins
transpiling the same method.
Copy link
Collaborator

@ManlyMarco ManlyMarco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If number of arrays is known why not use an array and id as index instead of a dictionary and id as key? It would eliminate the overhead.

@mosirnik
Copy link
Contributor Author

If number of arrays is known why not use an array and id as index instead of a dictionary and id as key? It would eliminate the overhead.

Good point, changed to use an array instead. (Let me know if you prefer squash + force-push rather than adding new commits)

@ManlyMarco
Copy link
Collaborator

I'll most likely squash merge anyways so you can just push separate commits.

@ManlyMarco ManlyMarco merged commit 3124e14 into IllusionMods:master Aug 30, 2023
ManlyMarco added a commit that referenced this pull request Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants