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

Rework asset editors dependencies #2167

Merged
merged 24 commits into from
Mar 4, 2024

Conversation

Kryptos-FR
Copy link
Member

PR Details

Description

Rework the relationship between assets and their editing counter parts.

The idea is to break the dependencies between what can be cross-platform (e.g. view models, previews) and what is UI-specific (editor views, preview views). On top of that, asset view model where directly dependent on their corresponding editor view model. This didn't make sense from an architecture point of view: asset view models should exist outside of an editing context ; they could for instance be reused to write an stand-alone previewer app. On the other hand, the editor view model knows which asset view model it is editing so a dependency in that direction makes sense.

Related Issue

Motivation and Context

As part of the editor rewrite, since it involves touching a lot of files, it is better to merge it into master before starting the biggest part of the rewrite. Otherwise, we could have a lot of conflicts as the xplat-editor branch will diverge from master. With that said, from this point we should only allow minor changes to the editor such as bug fixes, and make sure they are reported periodically to the xplat-editor branch.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • Can't be sure that nothing is breaking, needs testing.
    • Runtime untouched, only editor changes.

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

@Kryptos-FR Kryptos-FR changed the title Rework asset editors Rework asset editors dependencies Feb 29, 2024
@Kryptos-FR Kryptos-FR self-assigned this Feb 29, 2024
@Eideren
Copy link
Collaborator

Eideren commented Mar 1, 2024

Editor prevents interaction (windows ding) after closing the initial loading window when it produced a warning. Sounds like the modal window is not properly closed maybe ?
Repro:

  • Open topdown rpg - wait for it to load,
  • Close editor
  • Restart editor
  • Open the template you just created
  • Notice the warnings
  • Press close on the opening session modal

This only occurs when running through VS so that's likely the XAML debugger blocking stuff again, not sure that's really related to your changes

@Eideren
Copy link
Collaborator

Eideren commented Mar 1, 2024

Looks like sprite sheets are broken, opening ParticleButtons in the UI particle sample opens the ui for a split second then closes it

@Kryptos-FR
Copy link
Member Author

Looks like sprite sheets are broken, opening ParticleButtons in the UI particle sample opens the ui for a split second then closes it

That's a nice bug actually. For some reason, we were returning false in the Initialize() method for the SpriteSheetEditorViewModel.

public override Task<bool> Initialize()
{
Cache = new SpriteEditorImageCache();
Initialized?.Invoke(this, EventArgs.Empty);
return Task.FromResult(false);
}

But so far, we didn't actually care about the boolean value. Returning true now obviously fixes it.

@Kryptos-FR Kryptos-FR force-pushed the feature/xplat/assets-editor branch from df4ce49 to e558a3e Compare March 2, 2024 19:20
@Kryptos-FR Kryptos-FR requested a review from xen2 March 2, 2024 19:22
@Kryptos-FR Kryptos-FR requested review from Eideren and xen2 and removed request for xen2 March 2, 2024 19:22
@Kryptos-FR Kryptos-FR marked this pull request as ready for review March 2, 2024 19:32
@Eideren
Copy link
Collaborator

Eideren commented Mar 3, 2024

There's this screen that shows up after double clicking on a scene, right before it's loaded in and replaced by the scene view
image
In the previous version this screen would be showing up instead:
image
So it sounds like that loading screen never runs in your latest ?

@Kryptos-FR Kryptos-FR merged commit f9f1af7 into stride3d:master Mar 4, 2024
4 checks passed
@Kryptos-FR Kryptos-FR deleted the feature/xplat/assets-editor branch March 4, 2024 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants