Slion / VirtualDesktop

C# wrapper for the Virtual Desktop API on Windows 11.
MIT License
21 stars 9 forks source link

Refixed the ordering of the interface guids. #5

Closed mzomparelli closed 1 year ago

mzomparelli commented 1 year ago

This works, test it. The GUIDs should be ordered.

Slion commented 1 year ago

I don't really want to merge that as it is as I would like to keep app.config readable by keeping serializeAs set to Xml.

mzomparelli commented 1 year ago

Show me? I did not change that part. App.config is created upon every build

mzomparelli commented 1 year ago

Show me? I did not change that part. App.config is created upon every build

I only changed Settings.settings and that chnaged app.config when I built to test

mzomparelli commented 1 year ago

It still has that image

mzomparelli commented 1 year ago

oh you want xml

Slion commented 1 year ago

I only changed Settings.settings and that chnaged app.config when I built to test

That's why I'm changing the app.config directly which admittedly is a bit of a mess too.

mzomparelli commented 1 year ago

Well string is fine. It's readable for me and it's Settings.settings that is creating app.config. Never ever modify app.config manually. Always modify in Settings.settings

mzomparelli commented 1 year ago

Well string is fine. It's readable for me and it's Settings.settings that is creating app.config. Never ever modify app.config manually. Always modify in Settings.settings

Why maintain the guids in two places when you only have to do it in one place

mzomparelli commented 1 year ago

I only changed Settings.settings and that chnaged app.config when I built to test

That's why I'm changing the app.config directly which admittedly is a bit of a mess too.

Why do you need it Serialize as xml?

mzomparelli commented 1 year ago

Everytime you build you will lose the serialize xml you put in the app.config. Give me a good reason that you are modifying app.config manually.

mzomparelli commented 1 year ago

You didn't need to add the minor version to the builds. That is overkill. 22000 would have handled prior to 22621 I'm pretty sure and if it won't then we need to find those chnages.

mzomparelli commented 1 year ago

I'm going to undo that minor build stuff

mzomparelli commented 1 year ago

Do you know what build number came right before 22621?

mzomparelli commented 1 year ago

See I'm sure it'll work without the minor versions image

mzomparelli commented 1 year ago

We should just take the int of the Build double

Slion commented 1 year ago

Everytime you build you will lose the serialize xml you put in the app.config. Give me a good reason that you are modifying app.config manually.

It keeps app.config readable so that we can easily read the diffs as those UIDs are so important to keeping the whole thing working.

mzomparelli commented 1 year ago

I propose doing this image

Slion commented 1 year ago

You didn't need to add the minor version to the builds. That is overkill. 22000 would have handled prior to 22621 I'm pretty sure and if it won't then we need to find those chnages.

Because the UIDs changed between 22621.2134 and 22621.2215

mzomparelli commented 1 year ago

You didn't even add the minor to the namespace image

mzomparelli commented 1 year ago

We are getting rid of the minor and going back to only the major

mzomparelli commented 1 year ago

2134 is not even a valid build. Was that insider? We don't support old insiders forever.

Slion commented 1 year ago

2134 is not even a valid build. Was that insider? We don't support old insiders forever.

No it was the official build before 2215 came along a few days ago.

mzomparelli commented 1 year ago

2134 is not even a valid build. Was that insider? We don't support old insiders forever.

No it was the official build before 2215 came along a few days ago.

But 22000 works for that build right?

The >= logic would have caught it for 22000

image

Slion commented 1 year ago

You didn't even add the minor to the namespace image

Oups, looks like it is not strictly needed. Feel free to fix it though.

Slion commented 1 year ago

But 22000 works for that build right?

The provider yes. But not the UIDs.

Slion commented 1 year ago

So 22621.2134 was using the 22000 provider with UIDs specific to 22621. Then 2215 came along. I too like to keep things simple but if Microsoft changes those UIDs between minor version then we need to take them into account too.

mzomparelli commented 1 year ago

rovider yes. But not the UIDs.

2134 is identical to 22000

image

Slion commented 1 year ago

I've fixed the ordering on main while keeping app.config clean. Hope you don't mind.

mzomparelli commented 1 year ago

I do actually. We don't modify app.config. Tell me why you need app.config a certain way? Also I'm undoing the minor stuff you added as I just confirmed 22000 is the same as 22621.2134

mzomparelli commented 1 year ago

can you undo to my commit from last night

mzomparelli commented 1 year ago

I will fix the Settings.settings. guids after.

Slion commented 1 year ago

Apologize I probably got confused with the UID being different for 22000 and 22621 before 2215. But the fact remain that since the UIDs are changing between minor versions we need to take it into account.

mzomparelli commented 1 year ago

No the minor version don't matter. I just confirmed that 22621.2134 matches 22000. I've been maintaining my own for a while and I only use the major build

mzomparelli commented 1 year ago

You're killing me. I'm starting ,my own fork and I won't do pull requests

Slion commented 1 year ago

Fair enough and too bad… 😭 Hope for you they don't change those UIDs between minor versions again. Trust me I did not implement that because I fancy doing it.

mzomparelli commented 1 year ago

Fair enough and too bad… 😭 Hope for you they don't change those UIDs between minor versions again. Trust me I did not implement that because I fancy doing it.

Yes you did. It's unnecessary and creates more code than necessary. Also more to maintain.

mzomparelli commented 1 year ago

hat because I fancy doing it.

You still didnt give me a valid reason for manually modifying app.config and why you need it serialize xml

mzomparelli commented 1 year ago

You would think you would trust the guy that fixed this for you

Slion commented 1 year ago

Yes you did. It's unnecessary and creates more code than necessary. Also more to maintain.

Sad you won't trust me. That's the reason why #1 was created and why people started turning up asking for a fix. The UIDs were changed between 22621.2134 and 22621.2215 which was released a few days ago.

mzomparelli commented 1 year ago

No they were not. Tell me this, was this library working for 22621.2134? If so then it was using 22000. I confirmed they are the same.

Please trust me and take my next pull

Slion commented 1 year ago

You would think you would trust the guy that fixed this for you

Your are awesome man. I'm most impressed with your work on reverse engineering and I really wish we could find common ground and work together on that one.

mzomparelli commented 1 year ago

Then trust my changes. Actuially prove to me it does not work on 22621.2134. I know my changes will

Slion commented 1 year ago

Tell me this, was this library working for 22621.2134?

Yes it was. That's actually the reason I started that fork.

mzomparelli commented 1 year ago

ok then there was no need to add minor build

Slion commented 1 year ago

I meant it was working on 2134 before your changes that fixed it for 2215.

mzomparelli commented 1 year ago

You're wrong. I will test it myself. I'm reverting your changes so I can test it and show you the proper way

Slion commented 1 year ago

I would gladly be proven wrong as it means the code would remain less complex albeit marginally.

mzomparelli commented 1 year ago

Also you should understand 22621.2134 not a full build. It's the same as 22000 for Virtual Desktops. 2134 is not a build that Microsoft will give you anymore. 22H2 is 2215, 22H1 is 22000

Slion commented 1 year ago

Also you should understand 22621.2134 not a full build. It's the same as 22000 for Virtual Desktops. 2134 is not a build that Microsoft will give you anymore. 22H2 is 2215, 22H1 is 22000

I know but there is potentially still people out there who are still running 2134 until they upgrade to 2215 like I did a couple of days ago.

mzomparelli commented 1 year ago

ok but 2134 will use 22000. I will prove it just give me a bit. Do you have 2134 installed and ready?