UdonSharp icon indicating copy to clipboard operation
UdonSharp copied to clipboard

Fixes errors pointing at LogError instead of project asset

Open Faxmashine opened this issue 2 years ago • 6 comments

@MerlinVR should weigh in before I proceed further.

This fix makes it easier to find U# program assets without a source C# asset. Clicking the error takes the user to the asset, instead of UdonSharpUtils.cs's LogError.

Faxmashine avatar Dec 05 '22 15:12 Faxmashine

🤔 the program asset was already passed as context to that error? Changing the text to be more specific is fine, but introducing an overload that takes U# program assets for the UdonSharpUtils.LogError() is redundant since one already exists for generic UnityEngine.Objects. It's just not great when you need to point to an asset with the context since usually people will have the Console window and Project window on different tabs in the same window so clicking the error only brings up the inspector for the asset and doesn't select it in the Project window. Also fyi AssetDatabase.FindAssets() gets pretty slow to call on large projects so it's usually avoided or cached. It also tends to... not work.

MerlinVR avatar Dec 05 '22 16:12 MerlinVR

Oh yeah, we should avoid needlessly finding assets twice, and the overload sucks.

If we don't want to add the proper context we should discard the context, as seeing the logger is useless (or misleading) for creators.

I think the content may get lost because there's some array cloning going on when all the assets are first retrieved.

On Mon, Dec 5, 2022, 17:37 Merlin @.***> wrote:

🤔 the program asset was already passed as context to that error? Changing the text to be more specific is fine, but introducing an overload that takes U# program assets for the UdonSharpUtils.LogError() is redundant since one already exists for generic UnityEngine.Objects. It's just not great when you need to point to an asset with the context since usually people will have the Console window and Project window on different tabs in the same window so clicking the error only brings up the inspector for the asset and doesn't select it in the Project window. Also fyi AssetDatabase.FindAssets() gets pretty slow to call on large projects so it's usually avoided or cached. It also tends to... not work.

— Reply to this email directly, view it on GitHub https://github.com/vrchat-community/UdonSharp/pull/86#issuecomment-1337688339, or unsubscribe https://github.com/notifications/unsubscribe-auth/A36FF4HYNO4CV3ZOQCRVRYDWLYK5NANCNFSM6AAAAAASUM5NJ4 . You are receiving this because you authored the thread.Message ID: @.***>

Faxmashine avatar Dec 05 '22 17:12 Faxmashine

It does point to the asset properly when I've tried it, is there some repro for it not working?

MerlinVR avatar Dec 05 '22 17:12 MerlinVR

Repro steps

  1. Create an UdonSharp project with the VCC
  2. Create an UdonSharp script (Project window -> right click -> U# script)
  3. Delete the C# script
  4. Double click either of the two errors

UdonSharpUtils opens when it shouldn't

Faxmashine avatar Dec 05 '22 22:12 Faxmashine

I can get it to repro sometimes with that, it looks more like Unity's Context only works if there aren't multiple errors pointing to the same thing in a row or something along those lines. Loading the asset doesn't improve it either so I'm assuming your fix just seemed to change because the behavior of Unity is inconsistent. I think this is just a Unity moment and we can't do much about it.

MerlinVR avatar Dec 05 '22 23:12 MerlinVR

https://user-images.githubusercontent.com/36685500/205764202-3a19c90d-8546-4417-9110-df7e534ed485.mp4

To illustrate

MerlinVR avatar Dec 05 '22 23:12 MerlinVR