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

Add Categories feature to Patch Container Classes #498

Merged
merged 2 commits into from
Nov 3, 2022

Conversation

rrazgriz
Copy link
Contributor

@rrazgriz rrazgriz commented Sep 7, 2022

Summary

This change adds the following features:

  • [HarmonyPatchCategory(string category)] attribute, applied to patch container classes to specify one category for a patch
  • Harmony.PatchCategories(string category) method, called to apply all patch container classes marked with a specific category
  • Harmony.PatchAllUncategorized() method, called to apply any patch container classes not marked with a specific category
  • Documentation for the above functionality (in markdown form - not sure if I have to do anything for page generation)

This functionality allows applying different sets of patches at different times when using annotated patch classes. This allows for easy conditional patch application, allowing things like delaying a patch until after a static initializer has run, or performing patches based on user configuration.

Notes

  • For simplicity, [HarmonyPatchCategory] can only be applied to patch container classes, not individual patch methods within a container. This avoids having to make a library-level choice about how individual patch method categorization overrides container categorization, which I believe would be an awkward design. Instead, users can control their categorization as needed to create conditional logic for what patches to apply and when.

  • Only one category can be applied to a patch. This simplifies the implementation and means that patching multiple different categories will never cause a patch container to be applied more than once.Users can use specific

  • This change does not change the behavior of Unpatch() or UnpatchAll(). Because of how those methods work, they act based on the methods that were patched, not what patches were used, so it would require extra data to be added to identify what patch categories were used to patch something. Additionally, unpatching seems to unpatch as a whole, so if you only wanted to unpatch a method's patches from one category, it would also unpatch any changes from another category. Future work could maybe change how Unpatching logic is done to allow partial unpatching (or unpatching and re-patching based on categories).

@pardeike
Copy link
Owner

pardeike commented Sep 7, 2022

Unfortunately, adding an extra argument to PatchAll will break existing compiled dlls that now try to use the previous version of that method. Instead we need to add an overload or a different named method.

The problem is not that code can be compiled without changes, it’s that a lot of games work with a Unity version that allows for resolving dlls at runtime without regard to their version which requires API stability or else you will get loading errors.

@rrazgriz
Copy link
Contributor Author

rrazgriz commented Sep 8, 2022

Sounds good. Given the need for API stability, I've broken out the uniquely named method PatchAllUncategorized(), which has the needed behavior, and leaves the functionality and interface for PatchAll() untouched.

Let me know what you think!

@pardeike
Copy link
Owner

pardeike commented Oct 4, 2022

I did not forget you, am just a tad too busy.

@pardeike
Copy link
Owner

pardeike commented Nov 3, 2022

Excellent work

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