-
Notifications
You must be signed in to change notification settings - Fork 15
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
[Feature Request] adding a weaver attribute for logging methods #21
Comments
Hey Loic, Yeah this is a feature that makes sense to have. Right now I only have a few sample plugins available just to help people design their own plugins. I am sure this could be something that can be added to the default ones. There are just a few things to take into account.
|
@ByronMayne thanks for your response. 1) Multiple return paths.I'm agree with inserting instructions before every return. 2) IEnumeratorI think we can do something like [MethodLogger(Yield = true)]
public IEnumerator<int> Foo()
{
for(int i = 0; i < 10; i++)
{
yield return i;
}
} 3) Function Return Typespublic Action Foo()
{
return Bar;
} In case of callback returned ignore the behavior of logging? 4) Exception Logging & handlingI think we can propose to the user also an option: [MethodLogger(ExceptionLogging= true)]
public void Foo()
{
throw new System.NotImplementedException();
} |
@ByronMayne also at the time i'm on unity 2019.2 do you have plan to update the library? I can't add an assembly to the weaver settings |
@ByronMayne I think the generated IL code always results in one ret instruction. When multiple return paths are found, it just branches to the return after popping (if needed) from the stack. Furthermore, IEnumerators are kind of a weird thing. You can either yield return, where the generated IL code produces a nested class in which you want to inject the code or a normal return, whereas it should be treated as normal function. I'm posting this here to say that I've created a Decorator component and attribute which you can override and pretty much does what this issue is about - and some more. It's a bit messy so I don't know if I'll get time to wrap it and send it here. My only issue is that I always handle IEnumerator functions as normal functions, ignoring if there's a yield in there. I do not know if I'll find time to fix that. On a further note, I fixed the problem where you couldn't add assemblies but I had to change some stuff. I didn't implement it to work differently for older Unity versions. All I did was ALWAYS apply the modified properties (but not run the AssemblyUtility.DirtyAllScripts) and made a cache to be able to revert the modified properties if you want to (if you haven't called AssemblyUtility.DirtyAllScripts). |
Hey Salokinsomisa, Even though I have written this tool that write and manipulates IL I am still not very good with it. There are so many case I just don't know of so I get a bit wary. I am trying to come up with a more elegant way to write unit tests to make sure I also don't break anything. I am currently in the works of a refactor for Weaver because of some of the old design decisions I took as well as this branch being unable to support newer versions of Unity. In this I am also rewriting all the default plugins to make the code much cleaner and fix a few small bugs. If you want to post your code I could use it as a reference for what you were doing. One feature I am adding which I am really excited for is the recursive flag for methods. For example |
Hello! The fix for the assemblies is scattered throughout the source code ( I removed many unnecessary calls to AssemblyUtility.DirtyAllScripts because if you had many assemblies it could take even up to 7 seconds (for me) to load them. Furthermore I populate the assemblies only once when the WeaverSettings editor is enabled for the first time. I did that because when hitting play a domain reload happens and adding 7 seconds to the time it takes to load the game EACH time you hit play is massive. Apart from that, the DecoratorComponent that does what this issue is all about is seperated into two files. What parts of the code do you want? Both the fixes and the Decorator or one of the two? |
Hey guys, I know this thread is a few years old but I am very interested in such an attribute (I just want to log the type on which the method was called)! I'm on Unity 2021. |
Sorry to bump again, but I am still super interested! |
I am no longer working on this repository as I can't due to NDA. If you would like toe add the feature I can accept PRs |
I think it will be good to add a weaver like that :
The attribute
The use case:
The text was updated successfully, but these errors were encountered: