Repair projectwatcher #2106

IXLLEGACYIXL · 2024-01-14T17:22:49Z

PR Details

Repairs Projectwatcher to behave correctly on reload

Description

The Problem is when you have Project A#1 references Project B#1, now you edit B#1 , B#1 gets reloaded and becomes B#2 but A#1 is dependent on it, A#1 has STILL B#1 types so the stamp of the roslyn workspace doesnt fit as #1 != #2 so the reference serializers die as the type dictionary is searched for it but type(BClass#1) isnt in the dictionary of registered serialziers, it has type(BClass#2) in it.

Now the proplem is in the ProjectWatcher

Get the Project which changed
Get its Dependencies
Register all of them to reload

First i changed the workspace to use solution as we need the dependency tree of the solution,
Then find all dependents on the project and sum them up in the list for the broadcast

Now lets send the list through the broadcast with the 2 projects A#2 and B#2
The only receiver is the debugger view model, it waits for events incomming

Now the problem with the debugger view model

it waits until an event comes
when an event comes consume it and block stride AND itself ( IDispatcher of stride blocks everything )
wait until button pressed

so B#2 and A#2 are getting sent through the broadcast, B#2 gets sent to the debugger view model, now it blcoks everything and waits for reload... now you press reload and you reload B#2... but what about A#2 ? its still in the queue

thats why i just wait until the project watcher sets the list of changes as public property and empty it then
this way A#2 and B#2 get reloaded when pressed

Problem with my solution:

Time dependency, on first occurence of an assembly change it locks itself again
its not good with multiple project changes .. it would be better to have a stack or something and pop and add changes
The reload button should be triggered when focus is set on the stride window, this way it gets blocked at the latest point possible, unity does the same

Related Issue

fixes #2081
and partially #2107
fixes #2125
fixes #2022
fixes #2054
fixes #2123 ( cant reproduce it )

Now what

I think the broadcasting is useless now?

I dont have much exp with async and await and how to manage this situation.
All i know is if i edit B then A and B get reloaded

I cant continue here, i would love to see someone else taking over this who has more knowledge about handling this

just ignore the positive 77... had the wrong base 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)

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.

IXLLEGACYIXL · 2024-01-14T18:20:11Z

ok ive tested it again,

the current state works how it is, even when multiple times editing the same project or other projects in addition
as long as the button aint pressed, the assembliesModified is getting filled up until the button is pressed, so that behaviour is fine

so only the overall rework should be adjusted

sources/editor/Stride.Core.Assets.Editor/View/DefaultPropertyTemplateProviders.xaml

sources/editor/Stride.Assets.Presentation/AssetEditors/ProjectWatcher.cs

Kryptos-FR · 2024-01-14T19:57:06Z

sources/editor/Stride.GameStudio/ViewModels/DebuggingViewModel.cs

+            var waitTask = Task.Run(async () =>
+            {
+                while (!condition()) await Task.Delay(frequency);
+            });
+            await waitTask;


Why running in another task here?

2ca2766
removed it, is this the way it should be?

What's the goal here? To await for an asynchronous event? If so, there are other patterns that can be used instead of an active pull (which this repeated loop is a kind of).

For instance, SemaphoreSlim or a TaskCompletionSource could be used instead, depending on the expected usage.

the goal here is to wait until a list is ready, the delay was introduced as i didnt think it would work without but it does
even without waiting it works as long as it fills up the dictionary and doesnt get blocked by the idispatcher

what i want to do is

on first change spawn notification ( stride is blocked )

until the button is pressed sum up the assemblies in the dictionary

on button click take the dictionary and do the stuff that is allready there

the projectwatcher never completes his task, neither does the debugger complete his task

they run in parallel, mybe semaphore would work but i have no idea how to implement it as i just read quickly over it

ok it was easier than i thought with asnyc enumerable
3436b3f

problem is now i dont know what to write in the dispose... there was before a .Wait()... how can i actually stop that thing? or should it be stopped?

got help from C# discord, i just have to run a break; to stop it

sources/editor/Stride.GameStudio/ViewModels/DebuggingViewModel.cs

IXLLEGACYIXL · 2024-01-14T20:47:32Z

im open for any changes to make it a mergeable PR , like kryptos helped

IXLLEGACYIXL · 2024-01-15T01:08:23Z

d13758e
with this i removed the broadcast from roslynpad

ive tried it and roslynpad "works" ... the exact same bug persists in roslynpad, no idea where that is comming from

there a reload doesnt trigger the dependent projects to reload
can someone confirm that i made roslynpad not worse than it was before? ive never started that thing

my main target was to fix it for the "normal way"

IXLLEGACYIXL · 2024-01-15T01:11:45Z

and i think i fixed by accident that stride didnt reload the types after a failed compilation

IXLLEGACYIXL · 2024-01-16T08:39:21Z

its finished so far and ready

Eideren

Minor nitpick but looks good otherwise. Sorry about the delay.

sources/editor/Stride.Assets.Presentation/AssetEditors/ProjectWatcher.cs

IXLLEGACYIXL · 2024-01-19T21:06:08Z

Minor nitpick but looks good otherwise. Sorry about the delay.

no problem, with the nitpick i found a nre , which may be the issue that projects dont reload on reference changes in the csproj

Eideren · 2024-01-20T16:28:04Z

Looks good, any last comment @Kryptos-FR ?

IXLLEGACYIXL · 2024-01-25T23:02:01Z

fixes #2054

it didnt launch alohatest which is an xunit project, it launched the windows one

IXLLEGACYIXL · 2024-01-25T23:08:17Z

fixes #2022 , couldnt reproduce it

removed half of the camera controller script , reloaded, added more syntax errors, reloaded again

reverted everything, camera controller is there to be added
added a syntactical wrong syncscript, reloaded, fixed syntax , reloaded, was there

IXLLEGACYIXL · 2024-01-25T23:19:46Z

there is a bug currently in my version and the current released one, when attaching a syncscript in a sub project then when opening stride it cant find the serializer for that one, dunno if i can track it down but its in both versions

probably better to make a separate issue and pr

IXLLEGACYIXL · 2024-01-26T02:27:35Z

ok so i investigated #2125
the error persists currently too, so even when this is merged then the "new bug" will come to the daylight as it wasnt possible for the bug to occurr since i fixed the project loading back then last year

when this is merged it would lead people into using that feature, then they will run into the next bug

i will add the fix for it, but im not too sure about it yet so i will add wip for now

IXLLEGACYIXL · 2024-01-26T04:02:39Z

startup is now also fixed of non root projects

so a new review would be needed

Kryptos-FR

I will attempt to refactor CodeViewModel. Its initialization looks a bit clumsy (previous code was even worse).

Kryptos-FR · 2024-01-28T19:17:17Z

sources/editor/Stride.Assets.Presentation/ViewModel/CodeViewModel.cs

@@ -139,6 +113,33 @@ void TrackedAssemblies_CollectionChanged(object sender, Core.Collections.Trackin
            //SymbolDisplayPartExtensions.StyleRunFromSymbolDisplayPartKind = StyleRunFromSymbolDisplayPartKind;
            //SymbolDisplayPartExtensions.StyleRunFromTextTag = StyleRunFromTextTag;
        }
+        private Task ProjectUpdate;


Where is it used?

Kryptos-FR · 2024-01-28T19:29:16Z

sources/editor/Stride.Assets.Presentation/ViewModel/CodeViewModel.cs

-                    }).Wait();
-                }), new DataflowLinkOptions());
-
+                ProjectUpdate = UpdateProjects(projectWatcher, workspace, strideAssetsViewModel);
                return workspace;


At this point, we are returned a workspace that might not be up to date yet (~~the async loop in UpdateProjects might not have completed yet~~). It feels very unstable (the previous code was like that as well).

What if instead, we make the whole continuation (from line 64) asynchronous as well so that we ensure that workspaceTask (and thus the Workspace property from line 152) only completes when everything is ready?

The code would look like this:

// line 64 workspaceTask = workspaceTask.ContinueWith(async previousTask => { var projectWatcher = await projectWatcherTask; var workspace = await previousTask; // code unchanged // line 103 _ = UpdateProjects(projectWatcher, workspace, strideAssetsViewModel); // fire & forget return workspace; }

edit: UpdateProjects is an infinite loop so my previous remark doesn't stand.

[Editor] Refactor initialization of CodeViewModel

IXLLEGACYIXL · 2024-01-30T18:29:21Z

is it now fine?

Eideren · 2024-01-31T23:35:22Z

I'm guessing those last changes come from @Kryptos-FR - no need to ask him to review them. If so, sure, I can merge. Just let me know.

IXLLEGACYIXL · 2024-01-31T23:42:13Z

I'm guessing those last changes come from @Kryptos-FR - no need to ask him to review them. If so, sure, I can merge. Just let me know.

i will test tomorrow again everything to be sure it works, and then notify you

IXLLEGACYIXL · 2024-02-01T17:47:45Z

its fine, checked again the reloading it can be merged

* Remove docs, has been moved to Stride docs repo * [OpenXR] Fixes build on graphics APIs other than D3D11 This build regression was introduced by previous commit d72aef5 * Update samples to Stride 4.2 (stride3d#2132) * [Samples] Update to 4.2 * [Samples] Remove Newtonsoft.Json dependency in CSharpIntermediate * [Tests] Fixes random test failures (stride3d#2133) * [Tests] Fixes random test failures Some of the tests in Stride.GameStudio.Tests must not run in parallel as they access a shared class which is not thread safe. * Sets wait time in TestFileVersionManager to 500ms (from 200) Should make it less likely that unit tests on teamcity fail. * Repair projectwatcher (stride3d#2106) * use positive 77 * repair projectwatcher * remove unused line * undo -77 change * undo -77 * order usings * fix formatting * remove unused solution * remove unused async * use previous cancelation method * remove extra task * add check to not throw assembly changes away * rework distribution of assemblychanges * remove unused using * add broadcast back in * remove assembly broadcast * add cancelation * replace cancelation location * improve if nesting * improve naming, fix reload on new references * fix loading chain of assets * refactor * [Editor] Refactor initialization of CodeViewModel --------- Co-authored-by: IXLLEGACYIXL <ixllegacy123@outlook.com> Co-authored-by: Nicolas Musset <musset.nicolas@gmail.com> * [OpenVR] Adds a minimal API to request and control Passthrough (supported by OpenXR) https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html#XR_FB_passthrough * [OpenXR] Makes device construction internal and improves exception messages of new StartPassthrough method * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] * [Assets] Prevent crash of the assets compiler when an assembly cannot be fully loaded. (stride3d#2144) Fixes stride3d#2140 * [Build] Remove stylecop (stride3d#2105) Co-authored-by: IXLLEGACYIXL <ixllegacy123@outlook.com> * [Presentation] Rework AssetViewModelAttribute * [Editor] Cleanup * [Assets] Mark Editor as obsolete on AssetViewModel * [Editor] Make Asset property public on all editors * [Editor] Make IAssetEditorsManager a service * [Editor] Keep track of asset/editor association in AssetEditorsManager * [Editor] Add a method to find an opened editor * [Editor] Change binding logic in editor views The default DataContext is now the editor instead of the asset * [Editor] Update sprite editor bindings * [Editor] Update UI editor bindings * [Editor] Update entity hierarchy editor bindings * [Editor] Update script editor bindings * [Editor] Update graphics compositor editor bindings * [Editor] Removed pointless constraint on column width Which is also spamming the console with binding errors. * [Editor] Remove editor-related properties from AssetViewModel * [Editor] Rework AssetEditorViewModelAttribute - add AssetEditorViewAttribute - rework view creation and initialization * [Editor] Make AssetEditorViewModel.Asset property virtual C# has covariant return types since version 9 * [Editor] Rework plugins initialization * [Editor] Rework AssetPreviewViewModelAttribute & AssetPreviewAttribute - add AssetPreviewViewAttribute to break the dependency between a preview and its UI-dependent view * [Editor] Fix initialization of SpriteSheetEditorViewModel returning false * [Editor] Make Stride.Core.Presentation.Quantum cross-platform * [Editor] Make Stride.Core.Assets.Quantum cross-platform * [Editor] Do not load StrideDefaultAssetsPlugin from module. It currently hardcodes loading the templates from a package which causes some tests to fail. Partially reverts c98c72e * feat: Release.yml added for PR categorisation (stride3d#2137) * feat: Release.yml added for PR categorisation * Categories updated * [Editor] Fix scene editor loading screen * [VR] feat: Add haptic support to OpenVR and Oculus runtimes (stride3d#2169) Co-authored-by: Eideren <contact@eideren.com> * [Audio] Audio emitter multiple references to same asset bugfix (stride3d#2176) Co-authored-by: Eideren <contact@eideren.com> * Use correct destination path in asset import overwrite dialog * Use GetFullPath to correct directory seperator for display * fix: File GraphicsResourceMap.cs without references removed (stride3d#2181) * feat: Update samples/template to top-level statements (stride3d#2187) * Update README.md (prerequisites) * Update README.md (VSIX prerequisites) * [VSPackage] Revert a few package upgrades so that VSIX builds properly * [Presentation] Fix issue with binding quantum nodes when associated name is not found (stride3d#2195) Note: the solution is rather hackish at the moment. To be revisited once we have an Avalonia version. Should we then introduce a service for setting/retrieving the Unset value * fix: [Asset] Unified 3D asset importer (on behalf of Noa7/Noah7071) (stride3d#2163) Co-authored-by: noa7 <noahwdv@gmail.com> Co-authored-by: noa7707 <157441788+noa7707@users.noreply.github.com> Co-authored-by: Noah7071 <157886157+Noah7071@users.noreply.github.com> --------- Co-authored-by: JornosDesktop <aggror.jorn@gmail.com> Co-authored-by: Elias Holzer <elias@vvvv.org> Co-authored-by: Nicolas Musset <nicolas@color-rise.xyz> Co-authored-by: IXLLEGACYIXL <107197024+IXLLEGACYIXL@users.noreply.github.com> Co-authored-by: IXLLEGACYIXL <ixllegacy123@outlook.com> Co-authored-by: Nicolas Musset <musset.nicolas@gmail.com> Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> Co-authored-by: Jorn Theunissen <aggror.jorn@stride3d.net> Co-authored-by: Marian Dziubiak <marian.dziubiak@gmail.com> Co-authored-by: Vaclav Elias <vaclavelias@gmail.com> Co-authored-by: Addison Schmidt <addison@assurancefamily.com> Co-authored-by: Eideren <contact@eideren.com> Co-authored-by: Tim Conner <tim@timconner.com> Co-authored-by: Jakub Ławreszuk <31008367+Jklawreszuk@users.noreply.github.com> Co-authored-by: xen2 <virgile@stride3d.net> Co-authored-by: noa7 <noahwdv@gmail.com> Co-authored-by: noa7707 <157441788+noa7707@users.noreply.github.com> Co-authored-by: Noah7071 <157886157+Noah7071@users.noreply.github.com>

IXLLEGACYIXL and others added 4 commits January 8, 2024 02:50

use positive 77

138bb78

Merge branch 'main' of https://github.com/NexStandard/stride

e0bfe69

Merge branch 'stride3d:master' into main

a6d2b1b

repair projectwatcher

adc2272

IXLLEGACYIXL marked this pull request as draft January 14, 2024 17:22

remove unused line

8ff67ad

Kryptos-FR reviewed Jan 14, 2024

View reviewed changes

IXLLEGACYIXL added 9 commits January 14, 2024 21:17

undo -77 change

7767fef

undo -77

5e6d06c

order usings

ca151ad

fix formatting

46882e4

remove unused solution

0f4d84f

remove unused async

5ad7e3d

use previous cancelation method

1620c63

remove extra task

2ca2766

add check to not throw assembly changes away

594e9a1

IXLLEGACYIXL added 4 commits January 15, 2024 01:28

rework distribution of assemblychanges

3436b3f

remove unused using

4cc9777

add broadcast back in

69fdcea

remove assembly broadcast

d13758e

IXLLEGACYIXL mentioned this pull request Jan 15, 2024

Using undo on an assembly reload causes various problems #2107

Open

IXLLEGACYIXL added 2 commits January 15, 2024 09:53

add cancelation

6c9a701

replace cancelation location

9e0b23d

IXLLEGACYIXL changed the title ~~POC Repair projectwatcher~~ Repair projectwatcher Jan 15, 2024

IXLLEGACYIXL marked this pull request as ready for review January 17, 2024 09:39

IXLLEGACYIXL force-pushed the repair_projectwatcher branch from 9e0b23d to a93b3b0 Compare January 18, 2024 14:09

merge

07d1be0

Eideren reopened this Jan 18, 2024

Eideren reviewed Jan 19, 2024

View reviewed changes

sources/editor/Stride.Assets.Presentation/AssetEditors/ProjectWatcher.cs Outdated Show resolved Hide resolved

IXLLEGACYIXL added 2 commits January 19, 2024 21:45

improve if nesting

dcfa458

improve naming, fix reload on new references

31ae806

IXLLEGACYIXL mentioned this pull request Jan 25, 2024

solution item is disappeared when save in Stride Editor #2123

Closed

IXLLEGACYIXL changed the title ~~Repair projectwatcher~~ WIP Repair projectwatcher Jan 26, 2024

IXLLEGACYIXL added 2 commits January 26, 2024 03:55

fix loading chain of assets

50645ac

refactor

3d0078d

IXLLEGACYIXL changed the title ~~WIP Repair projectwatcher~~ Repair projectwatcher Jan 26, 2024

IXLLEGACYIXL requested review from Kryptos-FR and Eideren January 26, 2024 04:02

Kryptos-FR reviewed Jan 28, 2024

View reviewed changes

Kryptos-FR and others added 2 commits January 28, 2024 21:02

[Editor] Refactor initialization of CodeViewModel

6e0d4f2

Merge pull request #2 from Kryptos-FR/feature/repair-projectWatcher

a5dfc6d

[Editor] Refactor initialization of CodeViewModel

Eideren merged commit 26c3696 into stride3d:master Feb 1, 2024
2 checks passed

VaclavElias added the area-Build label Mar 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Repair projectwatcher #2106

Repair projectwatcher #2106

IXLLEGACYIXL commented Jan 14, 2024 •

edited

Loading

IXLLEGACYIXL commented Jan 14, 2024 •

edited

Loading

Kryptos-FR Jan 14, 2024

IXLLEGACYIXL Jan 14, 2024

Kryptos-FR Jan 14, 2024

IXLLEGACYIXL Jan 15, 2024

IXLLEGACYIXL Jan 15, 2024

IXLLEGACYIXL Jan 15, 2024

IXLLEGACYIXL commented Jan 14, 2024

IXLLEGACYIXL commented Jan 15, 2024

IXLLEGACYIXL commented Jan 15, 2024

IXLLEGACYIXL commented Jan 16, 2024

Eideren left a comment

IXLLEGACYIXL commented Jan 19, 2024

Eideren commented Jan 20, 2024

IXLLEGACYIXL commented Jan 25, 2024

IXLLEGACYIXL commented Jan 25, 2024

IXLLEGACYIXL commented Jan 25, 2024 •

edited

Loading

IXLLEGACYIXL commented Jan 26, 2024

IXLLEGACYIXL commented Jan 26, 2024 •

edited

Loading

Kryptos-FR left a comment

Kryptos-FR Jan 28, 2024

Kryptos-FR Jan 28, 2024 •

edited

Loading

IXLLEGACYIXL commented Jan 30, 2024

Eideren commented Jan 31, 2024

IXLLEGACYIXL commented Jan 31, 2024

IXLLEGACYIXL commented Feb 1, 2024

Repair projectwatcher #2106

Repair projectwatcher #2106

Conversation

IXLLEGACYIXL commented Jan 14, 2024 • edited Loading

PR Details

Description

Related Issue

Now what

Types of changes

Checklist

IXLLEGACYIXL commented Jan 14, 2024 • edited Loading

Kryptos-FR Jan 14, 2024

Choose a reason for hiding this comment

IXLLEGACYIXL Jan 14, 2024

Choose a reason for hiding this comment

Kryptos-FR Jan 14, 2024

Choose a reason for hiding this comment

IXLLEGACYIXL Jan 15, 2024

Choose a reason for hiding this comment

IXLLEGACYIXL Jan 15, 2024

Choose a reason for hiding this comment

IXLLEGACYIXL Jan 15, 2024

Choose a reason for hiding this comment

IXLLEGACYIXL commented Jan 14, 2024

IXLLEGACYIXL commented Jan 15, 2024

IXLLEGACYIXL commented Jan 15, 2024

IXLLEGACYIXL commented Jan 16, 2024

Eideren left a comment

Choose a reason for hiding this comment

IXLLEGACYIXL commented Jan 19, 2024

Eideren commented Jan 20, 2024

IXLLEGACYIXL commented Jan 25, 2024

IXLLEGACYIXL commented Jan 25, 2024

IXLLEGACYIXL commented Jan 25, 2024 • edited Loading

IXLLEGACYIXL commented Jan 26, 2024

IXLLEGACYIXL commented Jan 26, 2024 • edited Loading

Kryptos-FR left a comment

Choose a reason for hiding this comment

Kryptos-FR Jan 28, 2024

Choose a reason for hiding this comment

Kryptos-FR Jan 28, 2024 • edited Loading

Choose a reason for hiding this comment

IXLLEGACYIXL commented Jan 30, 2024

Eideren commented Jan 31, 2024

IXLLEGACYIXL commented Jan 31, 2024

IXLLEGACYIXL commented Feb 1, 2024

IXLLEGACYIXL commented Jan 14, 2024 •

edited

Loading

IXLLEGACYIXL commented Jan 14, 2024 •

edited

Loading

IXLLEGACYIXL commented Jan 25, 2024 •

edited

Loading

IXLLEGACYIXL commented Jan 26, 2024 •

edited

Loading

Kryptos-FR Jan 28, 2024 •

edited

Loading