-
Notifications
You must be signed in to change notification settings - Fork 8
Add inspector transpiler #122
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
base: master
Are you sure you want to change the base?
Conversation
|
It doesn't respect the Enabled state yet, wasn't sure how to do that |
MonkeyLoader.Resonite.Integration/UI/Inspectors/CustomInspectorInjector.cs
Outdated
Show resolved
Hide resolved
|
It doesn't rely on counting the branches anymore |
|
It now respects the Enabled state |
|
I think this now 100% matches the behavior of the old patch |
MonkeyLoader.Resonite.Integration/UI/Inspectors/CustomInspectorInjector.cs
Outdated
Show resolved
Hide resolved
Banane9
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments - I'd also prefer this to be an optional thing, at least for the start, I think. As in, have a toggle to use replacement instead of transpiler 🤔
Also let me know whether you think the history of the commits should be kept or whether it should be squashed
MonkeyLoader.Resonite.Integration/UI/Inspectors/CustomInspectorInjector.cs
Outdated
Show resolved
Hide resolved
MonkeyLoader.Resonite.Integration/UI/Inspectors/CustomInspectorInjector.cs
Outdated
Show resolved
Hide resolved
MonkeyLoader.Resonite.Integration/UI/Inspectors/CustomInspectorInjector.cs
Outdated
Show resolved
Hide resolved
Would be a bit odd to have both a prefix and transpiler, is that even possible? Also I don't mind what happens to the commits. Probably fine to squash. |
well, a prefix would just skip the transpiled content of the method, so it would be possible to have both indeed. It's mainly that restoring the vanilla behavior becomes harder when a transpiler is involved. We could do patch / unpatch with enabled / disabled, but that's also kinda funky 😩 |
|
The transpiler respects the Enabled state, so if its disabled, skip the prefix and then the original should run via the transpiler then just have a toggle for if you want to use prefix replacement or transpiler |
What I'm thinking about is the transpiler breaking the method so it doesn't work anymore at all after an update - so disabling it would have to be an unpatch 🤔 |
|
The solution to that would be to make the transpiler very robust so it is very unlikely to break after an update.. The same thing could happen to the prefix. |
Yea... though at least in my scenario, the prefix and transpiler break in different ways. The transpiler could break the method from working entirely (if not made properly), while the prefix just stays stuck on the old implementation, but doesn't "break" anything in that sense. Of course that all goes out of the window when any of the referenced methods change so they're not found anymore. |
|
If the transpiler IL code is invalid it won't patch anyway. |
Closes #120