search

KSP-ModularManagement / KSPe

Extensions and utilities for Kerbal Space Program
http://ksp.lisias.net/add-ons/KSPAPIExtensions
Other
11 stars 5 forks source link

Help with code refactoring #24

Closed unquietwiki closed 2 years ago

unquietwiki commented 2 years ago

Het @Lisias : I saw on here that you were fighting with Visual Studio a bit. I did some refactoring on MechJeb, and wanted to try working on a few others, but my bandwidth is limited. I figure I'd have you take a look. One thing I did learn from those devs: Unity has some weird behavior that conflicts with use of ?. for stuff; otherwise, setting projects to .NET 4.8 & auditing the code worked for me on that & one other addon I was looking at. Hope you're keeping safe & well.

Edit: https://devblogs.microsoft.com/visualstudio/making-our-unity-analyzers-open-source/ this might help too.

Lisias commented 2 years ago

@unquietwiki

Hi! Thanks for the advices! I will give these links a look.

On a side note, I'm compiling using .NET 4.6, as this is the one used by KSP. I had heard of at least one situation where an Add'On compiled on 4.8 caused troubles that were fixed by recompiling the thing under 4.6. I suspect something subtle had changed inside the runtime's guts between 4.6 (used by KSP) and 4.8 and newer.

unquietwiki commented 2 years ago

Hey @Lisias , thanks for takinga look. From what I was able to determine, .NET 4.7.2 seems to be the baseline for the present Kerbal stuff; but the stuff I tweaked out for MechJeb is working on 4.8. I know there's a Mono runtime for at least the Linux/Mac setup; its immediately unclear to me if the Windows-native/.NET stuff is overriding that; but the newer code syntax, outside the Null stuff the other guys warned me about, is working. I've seen a few things still trying to use .NET 3.5 as a baseline; probably from what was available earlier in the last decade.

Most of the changes involving 4.6 -> 4.8 involve web frameworks & usability (better Unicode & DPI support). There was one thing that stuck out: a correction of 64-bit behavior from the original 4.6 release. KSP's 64-bit, so maybe we're seeing that difference?

https://docs.microsoft.com/en-us/dotnet/framework/migration-guide/mitigation-new-64-bit-jit-compiler

Lisias commented 2 years ago

Hey @Lisias , thanks for takinga look. From what I was able to determine, .NET 4.7.2 seems to be the baseline for the present Kerbal stuff;

Yes, but it's a mistake (I'm probably still using it too on some add'ons that didn't needed maintenance since I decided to double check this).

Inspecting the binaries for the runtime signature, you will find 4.6.57. This is a dump from the System.dll found on 1.12.3/KSP.app/Contents/Resources/Data/Managed/System.dll:

Screen Shot 2022-01-24 at 01 41 14

I know there's a Mono runtime for at least the Linux/Mac setup; its immediately unclear to me if the Windows-native/.NET stuff is overriding that; but the newer code syntax, outside the Null stuff the other guys warned me about, is working. I've seen a few things still trying to use .NET 3.5 as a baseline; probably from what was available earlier in the last decade.

Unity5 and Unity 2017 used .NET 3.5 . Since some people (like me) wants to keep binary compatibility to older KSPs (as, frankly, the older versions are way more solid and stable that the last ones), compiling against 3.5 makes sense.

I strongly advise against using anything newer than 4.6. Unity is using 4.6, KSP is using 4.6 - you are expanding your surface of attack for incompatibilities (even 4.7 may, theoretically, have a incompatibility lurking somewhere). As I said, I have at least one report of a bug that was fixed by recompiling the thing using 4.6 instead of 4.8.

You need to understand that what's really matter once the code is compiled is the CIL code generated and the library dynamically being linked at runtime. Code written with the newer syntax sugars will be compiled into CIL code anyway - so it's the target environment where the code will run that must be taken as guideline.

At least on the Unity 2017 era, you need to compile your code against the Unity DLLs from Windows if you want the code to run on all supported platforms - if you compile your code against the OSX against the native DLLs, your binaries will run only on MacOS! Took me a lot of time until I could figure this out!

Absence of Evidence is not Evidence of Absence. You are getting lucky until the moment, but sooner or later you will induce someone to bork - KSP is brittle, anything borking will induce 3rd parties to bork too, making things harsh to diagnose. 80% of my support tickers are exactly due that - 3rd parties borking on dependencies somehow, and then my ones borking as a consequence.

unquietwiki commented 2 years ago

@Lisias Awesome information, thank you for providing that. I'm used to working on C# apps that are bound to the system runtime; which has been 4.8 for a couple of years now. That's a weird version number in the Mono runtime.

https://www.mono-project.com/docs/about-mono/compatibility/ & https://docs.microsoft.com/en-us/dotnet/standard/net-standard , which buttress what you're saying. I also found https://devblogs.microsoft.com/dotnet/announcing-net-standard-2-1/ , which might explain why stuff does seem to work. The common denominator is .NET Standard 2.0, which is available on net461 & later. I think a lot of what you're talking about could be avoided by standardizing on that, instead of .NET Framework 4.x; especially if Mono's handling UI stuff. I even caught a point on the 2nd article specifically calling out using Std 2.0 with Mono & Unity.

Updating the csproj files & Nuget references can be a bit of a minefield; I just tried doing that with code on my end, and borked that; there's some tooling I found that might work better, but that'll have to wait a bit. You should have a look at it: https://github.com/hvanbakel/CsprojToVs2017

Lisias commented 2 years ago

https://www.mono-project.com/docs/about-mono/compatibility/ & https://docs.microsoft.com/en-us/dotnet/standard/net-standard , which buttress what you're saying. I also found https://devblogs.microsoft.com/dotnet/announcing-net-standard-2-1/ , which might explain why stuff does seem to work. The common denominator is .NET Standard 2.0, which is available on net461 & later. I think a lot of what you're talking about could be avoided by standardizing on that, instead of .NET Framework 4.x; especially if Mono's handling UI stuff. I even caught a point on the 2nd article specifically calling out using Std 2.0 with Mono & Unity.

The best way to avoid problems is to cope with whatever Unity and KSP is using, anything else is a leap of faith.

Unity has customised some things on the Mono Runtime. It's unwise to second guess them! We may not agree with the changes, but we will need to cope with them the same.

unquietwiki commented 2 years ago

I wonder if this is why the Unity nuget guy gave up recently. Saw that while researching this stuff.

Michael Adams, unquietwiki.com

On Mon, Jan 24, 2022, 9:13 AM Lisias @.***> wrote:

https://www.mono-project.com/docs/about-mono/compatibility/ & https://docs.microsoft.com/en-us/dotnet/standard/net-standard , which buttress what you're saying. I also found https://devblogs.microsoft.com/dotnet/announcing-net-standard-2-1/ , which might explain why stuff does seem to work. The common denominator is .NET Standard 2.0, which is available on net461 & later. I think a lot of what you're talking about could be avoided by standardizing on that, instead of .NET Framework 4.x; especially if Mono's handling UI stuff. I even caught a point on the 2nd article specifically calling out using Std 2.0 with Mono & Unity.

The best way to avoid problems is to cope with whatever Unity and KSP is using, anything else is a leap of faith.

Unity has customised some things on the Mono Runtime. It's unwise to second guess them! We may not agree with the changes, but we will need to cope with them the same.

— Reply to this email directly, view it on GitHub https://github.com/net-lisias-ksp/KSPAPIExtensions/issues/24#issuecomment-1020339348, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHV7PY6BOU6CWVGJG75IV3UXWCE7ANCNFSM5MRKTYSA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

unquietwiki commented 2 years ago

@Lisias On your safety point... the damage point that the MechJeb guys flagged, that Unity dislikes, and that isn't in .NET Standard 2.0, are the newer null-based operators. I was also reading up more on CIL, and I'm not sure if there has been an actual change in instructions at that level; since it's basically abstracted assembly. .NET Framework has a lot of bits for building apps; so does Mono & Unity; and for being cross-platform, the latter two cannot simply use the complete feature set of .NET Framework; so there has to be a lower-common-denominator. I think that's why MS was citing Standard 2.0 for ideal targeting. It doesn't help in our discussion here that Mono has a weird version numbering system: in Kerbal, you see a "MonoRuntime 2.0" thing, but I'm pretty sure it's more like the 4-6 range.

As time permits this week & next, I plan to do some more experimenting with this, based on your concerns. I'll keep you apprised!

Lisias commented 2 years ago

From all this time on KSP modding, I came to conclusion that less is more.

I had settled up with .NET 3.5 because I just don't have any problems with it no matter the KSP version I'm running.

I do interim .NET 4.6 compilations now and then to see if I get any perceptible improvements, but until this moment, I had none.

You just need to be careful to avoid mixing .NET 35 and .NET 46 mutexes, threads and similars - i.e., do not try to lock something on .NET 35 from a .NET46 code and vice versa, build facades to be used on each side and everything will be fine.

unquietwiki commented 2 years ago

@Lisias At least we're not messing with Java. I cry for the Minecraft modders..

Lisias commented 2 years ago

@Lisias At least we're not messing with Java. I cry for the Minecraft modders..

I will tell you something - I have numerous systems under Java. Not a single one of them gives me any troubles. I have some systems running 24/7 for 300 days without a single issue, we are not even wasting money monitoring them anymore, we monitor the caller systems and if they report a faulty operation we go there and check the logs see to who else else is borking.

On the other hand, all my partners are using .NET - and all of them are having trouble after trouble. Rare is the day in which I don't end pinpointing a .NET subsystem as the source of the incident - including some government systems that used to be Java and were ported to .NET .

The magic is using Java, not that shitty half baked libraries that are still being in use nowadays I just doesn't understand how.

In the same way you are what you eat, your code is what you link to.

unquietwiki commented 2 years ago

Hey @Lisias I wanted to report back on my investigation into this stuff, as I was able to make time for (wife's been ill with suspected 'rona, and I've been a tad ill too).

So, I found a bunch of stuff that should inform our thinking on this stuff....

TLDR: 4.8 target should be fine if you don't accidentally use C#8 features; you can do C#7 & lower without major issues; use the Unity plugin to flag the null conflicts; folks are annoyed with Unity over their bastardized Mono falling out of sync with the .NET ecosystem.

Lisias commented 2 years ago

Hey @Lisias I wanted to report back on my investigation into this stuff, as I was able to make time for (wife's been ill with suspected 'rona, and I've been a tad ill too).

Wow. I hope she recovers well and fast.

TLDR: 4.8 target should be fine if you don't accidentally use C#8 features; you can do C#7 & lower without major issues; use the Unity plugin to flag the null conflicts; folks are annoyed with Unity over their bastardized Mono falling out of sync with the .NET ecosystem.

Unity did a lot of changes on Mono in the past, some of them on the garbage collector (or at least it was what I was told), and I think they painted themselves to a corner.

Yes, KSP is using the 4.x profile for sure. It's a game made initially for Unity 4, 10 years ago, and then updated to Unity 5, then 2017 and now 2019. This can render migrating to .NET 2.0 too much hassle for little to no gain.

Since I'm tied to C#7 features, it's really necessary to compile against 4.8? I think it's safer to stick with whatever KSP is using. Bastardised or not, is where my code is going to run and I have the personal guideline that it's more efficient to cope with the things we can't change that to fight eternally against what cannot be changed. KSP releases already on the wild are not going to change, so...

And since my goal is to support everything from KSP 1.3.1 to nowadays, my target ends up being 3.5 anyway - unless on things that are specific to Unity 2017 or 2019 (when so I try to target exactly what it's being used by the target environment).

unquietwiki commented 2 years ago

@Lisias thanks for your well-wishes. I guess ultimately, it doesn't matter that much: I know as an IT pro, 4.8's going to be on any Windows going back a couple of years, and you have to worry about downloading some framework pack & keeping that consistent among contributors. What is manageable, and consistent, is the use of the analyzers & editorconfig to enforce code styles & eliminate Unity gotchas.

On your own desire to maintain support for older stuff... https://marketplace.visualstudio.com/items?itemName=ironcev.sharpen this is something I messed with a while back, that I wish was on VS 2022; but works on VS 2017 & 2019. It lets you apply C# version-specific fixes and patterns to your code. If you're still using that environment, this might tickle your fancy.

Lisias commented 2 years ago

@unquietwiki KSP runs on Linux and MacOS, and these are important eco-systems on the Scene. I use MacOS myself.

So I'm not really interested on anything that will be on any Windows, I need to keep things tight on 3 different platforms the best I can - and so I need to settle up with whatever is available consistently on these 3 platforms.

If Visual Studio drops support for the Mono versions I need to cope with, I will drop Visual Studio and go back to Eclipse - it's cumbersome, but Eclipse can be targeted to anything and the kitchen's sink.

I didn't knew Sharpen - it can be useful. Thanks for the tip, let's see if it works on MacOS and Linux!

unquietwiki commented 2 years ago

Hey @Lisias , hope things are well. Regarding this code adventure of the past month or so, I put together notes on findings from that, including stuff we discussed here. Please feel free to suggest any corrections, or additional notes. Thanks!