Is System.Reflection.Assembly.GetTypes() failing for other modders in KSP 1.7?

[Dunbaratu]

Here's the long github issue I wrote about this: https://github.com/KSP-KOS/KOS/issues/2491

What I'm basically asking is this - do  any other plugin writers out there use the System.Reflection.Assembly.GetTypes() call, and are you having it barf on you with an exception just like I am, starting in KSP 1.7 when it didn't happen in KSP 1.6?

The issue linked to above gives the long winded details of the issue.  I just wanted to see if others have the same problem, because I am noticing other mods in the output_log also throwing exceptions trying to call GetTypes().  It's as if when the problem happens, then it fails for everyone else too.  (Or at least a lot of other people not just me).

[xEvilReeperx]

This kind of error can be a real PITA to debug, but in this case it's correct. The specific problem (at least, in my test bench of just downloading KAS 1.2 + current version of KOS) is that SaveUpgradePipeline.KAS.PatchFilesProcessor fails to load. This happens because it doesn't implement an abstract method. Between 1.6 and 1.7, UpgradeScript.OnUpgrade's signature changed.

I didn't check that other mod you mentioned but I imagine something similar is going on. I don't think there's a 1.7 version of KAS yet or else this would be caught already

[Dunbaratu]

4 hours ago, xEvilReeperx said:

This kind of error can be a real PITA to debug, but in this case it's correct. The specific problem (at least, in my test bench of just downloading KAS 1.2 + current version of KOS) is that SaveUpgradePipeline.KAS.PatchFilesProcessor fails to load. This happens because it doesn't implement an abstract method. Between 1.6 and 1.7, UpgradeScript.OnUpgrade's signature changed.

I didn't check that other mod you mentioned but I imagine something similar is going on. I don't think there's a 1.7 version of KAS yet or else this would be caught already

 

It's as if the system is contradicting itself, by saying, "that class is doesn't exist, so I can't include it in GetTypes()" and "that class exists, so I will include it in GetTypes()" at the same time.  If it doesn't exist, then why is GetTypes() including it and therefore bombing out?

Is there any set of flags, or another call I can use, that tells the system, "When I say GetTypes(), what I meant was the ones that are really there.  Get me the Types that you actually loaded.  If 1000 classes were attempted to be loaded, but 1 of them failed to load, then fine, at least let me see the other 999 of them."

I feel like I'm still going to want to protect myself from this in the future, even if KAS and Wind Tunnel get a 1.7 upgrade to fix this one instance of the problem.  I wasted a lot of my own time debugging a problem that wasn't "mine" to fix, it turns out.  If I cannot find a better version of what GetTypes() does that actually executes it safely, then I'll just have to trap the exception and issue a message to the user along the lines of "Some Mod, which might be kOS or might be some other mod, I can't tell which, has tried to load a class and it failed.  This is going to break kOS, but kOS can't fix it - check your error log for the first exception that looks like this: ....... and remove the mod it's talking about."

The bettter solution (but much uglier) would be to have a safe way to query all the type names that GetTypes() *wants* to return, in a fashion that won't break, and then try accessing them one at a time with a try/catch wrapper around each attempt, so that when the attempt fails I can at least report which class it is that's causing it, rather than just having GetTypes() in general fail overall.

[dkavolis]

https://docs.microsoft.com/en-us/dotnet/api/system.reflection.assembly.gettypes?view=netframework-3.5

You could just catch ReflectionTypeLoadException  and loop trough its Types property ignoring null values to get all loaded Types.

Edited April 20, 2019 by dkavolis

[Dunbaratu]

2 minutes ago, dkavolis said:

https://docs.microsoft.com/en-us/dotnet/api/system.reflection.assembly.gettypes?view=netframework-3.5

You could just catch ReflectionTypeLoadException  and loop trough its Types property ignoring null values to get all loaded assemblies.

Okay I've *never* seen an exception work that way before, ever.  So basically, its saying the exception didn't interrupt the program flow at all, and GetTypes() continued doing the rest of its work on the other classes before it finished.  It still built the rest of the list.  You just have to look inside the exception to find it, since it didn't return in the normal fashion.  That's utterly bizarre way to use an exception but at least it's a workaround.  (It's weird to me because it's basically using an exception for something that's a mere warning, rather than using it for an error that actually broke program flow.)

[dkavolis]

It's probably done to get the developers attention in case there were any errors during loading, otherwise you might not even notice and assume everything works as it should. And later you get an error in another place due to some types not loaded and wonder what went wrong. I'm sure Microsoft had good reasons to do it this way.

[Dunbaratu]

2 minutes ago, dkavolis said:

It's probably done to get the developers attention in case there were any errors during loading, otherwise you might not even notice and assume everything works as it should. And later you get an error in another place due to some types not loaded and wonder what went wrong. I'm sure Microsoft had good reasons to do it this way.

Microsoft has bizarre alien ways of thinking.  I find their techniques a weird random hodgepodge.  It would seem more intuitive that if the actual error occurs at the time the DLL is loaded, then that's when to have an exception to get a developer's attention, and if that exception is caught and ignored by the dev, then from then on make GetTypes() just return the classes that really exist, or have it be considered normal for it to be populated with some incomplete Types that have some kind of "loaded = false" flag on them that you have to protect around (or a bitflag set of options to GetTypes, as is common in other reflection calls, one of which explicitly says "only get the ones that are loaded, please".)   There's other ways to do it that wouldn't have had this weird "If there's a warning it still works anyway but you have to retrieve the return value from  *over there* instead of where it belongs.".

Anyway, thanks for the information.  It makes the problem fixable.