Improve PatchCategory/UnpatchCategory#704
Conversation
|
@codex Please do a code review on the suggested changes. Make sure you do an extensive risk analysis of this approach with the goal to find situations and use cases where this leads to errors and wrong behaviour. Be creative in thinking of all weird scenarios this could end up being run in. We are talking games, mods and any other weird runtime situations here that currently work fine with Harmony and that would break if we introduce caching. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
2 participants
The current implementation used in Harmony scans, for each call, every type in the assembly.
This implementation creates a cache for all categories on the first call for each assembly (using a ConditionalWeakTable per Assembly)
The cache for an assembly is basically a Dictionary<string, List>, where string is the category and List is the list of types that have the attribute applied.
Considering the caches are keyed to a specific assembly; I think things like dynamic types shouldn't cause any issues.
I'm unsure of where to put the static AssemblyCachedCategories. It's currently a static field on Harmony; but maybe it should go to the HarmonySharedState instead?