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

Fixes errors pointing at LogError instead of project asset #86

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Faxmashine
Copy link

@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 Faxmashine requested a review from MerlinVR December 5, 2022 15:24
@MerlinVR
Copy link
Collaborator

MerlinVR commented Dec 5, 2022

🤔 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.

@Faxmashine
Copy link
Author

Faxmashine commented Dec 5, 2022 via email

@MerlinVR
Copy link
Collaborator

MerlinVR commented Dec 5, 2022

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

@Faxmashine
Copy link
Author

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

@MerlinVR
Copy link
Collaborator

MerlinVR commented Dec 5, 2022

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
Copy link
Collaborator

MerlinVR commented Dec 5, 2022

Unity_vVHYAt9oIe.mp4

To illustrate

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.

2 participants