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

[Asset] Valid default UrlReference prevent building #783

Open
Eideren opened this issue Jul 13, 2020 · 4 comments
Open

[Asset] Valid default UrlReference prevent building #783

Eideren opened this issue Jul 13, 2020 · 4 comments
Labels
area-Asset bug Something isn't working priority-Low Not important at this time. Should be deleted or reassessed after release work-estimate-S Good for first timers. A relatively quick item to fix, Max of 2x of XS

Comments

@Eideren
Copy link
Collaborator

Eideren commented Jul 13, 2020

Release Type: Github

Version: Latest

Platform(s): Any

Describe the bug
public UrlReference SomeRefs = new UrlReference( "MainScene" );
Prevents building the game and logs an error when loading a game in the editor even if the url provided is valid.

To Reproduce
Steps to reproduce the behavior:

  1. Spawn the 'New game' template
  2. Add public UrlReference SomeRefs = new UrlReference( "MainScene" ); to one of the scripts.(and using Stride.Core.Serialization;)
  3. Reload the assembly
  4. Check the log window / broken reference on the field.

Expected behavior
Providing a default implementation that has a valid url should work. And if an invalid one has to throw it should be more explicit.

Log and callstacks
urlerror.txt

Additional context
You will have to remove the url property from the scene file after saving a scene to force stride to process the default value for that field again, otherwise it'll just throw with an empty url issue.

Pinging @dfkeenan to see if he can fix this really fast or has some additional insight into this issue.

@Eideren Eideren added the bug Something isn't working label Jul 13, 2020
@dfkeenan
Copy link
Contributor

dfkeenan commented Jul 13, 2020

It's been along time since I looked at this. But I am pretty sure you should not be assigning it a value like that in code. It should only be set by the editor.

Options:

  • Make that clear in the documentation.
  • Possibly make the constructors non-public. I can't remember why I did it that way.
  • Change the serializers to not throw exception. Just ignore invalid UrlReferences/log warning messages.

The earliest I could look at this is the weekend.

@Eideren
Copy link
Collaborator Author

Eideren commented Jul 13, 2020

I completely understand why it shouldn't work for invalid paths but I don't see a reason not to support this for valid ones, is there a specific limitation that I'm not aware of ?

@dfkeenan
Copy link
Contributor

References are different to paths. The AttachedReferenceManager keeps track of what references what using a ConditionalWeakTable of objects and their reference information. The reference is used at serialization time so it knows what assets to bundle. The path is used to load it from the ContentManager at runtime. The serializer is erroring because that UrlReference has no "attached reference".

Game studio links the UrlReference to the "attached reference". I am not sure if it is possible to fix the missing "attached reference" at serialization time. It might also be possible that the asset in question has been pruned from the asset dependency graph by then. The asset dependency graph and serialization stuff is quite complex. I spent months on this on and off and I still didn't understand it fully by the time I got it to work.

@xen2
Copy link
Member

xen2 commented Jul 13, 2020

It's been a while, but I think I told @dfkeenan to rely on AttachedReferenceManager here:
#391 (comment)

Idea is that then it's properly serialized in asset file in format AssetId:AssetUrl (we want to keep asset id) and we could reuse existing path for AttachedReference (which handle asset moving, rename, etc.).

Not exactly sure what's best way to fix that yet, but could be either:

  1. make serializer little bit more accepting of case like this one and check if editor is happy with it on yaml save
  2. special pass in editor when creating object and/or serializing an object to scan for such UrlReference members, find their AssetId from url and create AttachedReference? (or null it + warning if not found?)
  3. there's probably some other way

@bmello4688 bmello4688 added priority-Low Not important at this time. Should be deleted or reassessed after release work-estimate-S Good for first timers. A relatively quick item to fix, Max of 2x of XS labels Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Asset bug Something isn't working priority-Low Not important at this time. Should be deleted or reassessed after release work-estimate-S Good for first timers. A relatively quick item to fix, Max of 2x of XS
Projects
None yet
Development

No branches or pull requests

4 participants