Jump to content

Scary Reflection stuff when wrapping an implicit operator.


Recommended Posts

I am pulling together a relfection wrapper for the CLS mod so that other mods can use it without having a hard dependency on it. I am "been inspired" by the work blitzy did for wrapping the toolbar mod, but have come upon a problem... (note that I have not executed any of this code yet, so it might all be borked!)

The CLS mod defines two implicit operators on the CLSPart class. The idea is that this allows you to cast a CLSPart into a part to get the Part object in question, or cast it to a ModuleConnectedLivingSpace to access the appropriate model. This all works nicely, and PapaJoe has told me that it makes his code nice, which is great.

Anyhow, I now need to wrap these operators using reflection. So following the toolbar example, I am proving a file of code that defines all the public classes in CLS, and for each of the public methods / properties providing some code that uses reflection to invoke the actual method or property. However I am struggling for the syntax to use for the implicit operator. Here is what I have got so far:


// Allow a CLSPart to be cast into a Part
public static implicit operator Part(CLSPart _p)
{
Type CLSPartType = AssemblyLoader.loadedAssemblies.SelectMany(a => a.assembly.GetExportedTypes()).SingleOrDefault(t => t.FullName == "ConnectedLivingSpace.CLSPart");
MethodInfo method = CLSPartType.GetMethod("op_Implicit", BindingFlags.Public | BindingFlags.Static);
Part retVal = (Part)method.Invoke(null,new object[] { _p });
return (retVal);
}

This all seems good. However, if I write the same code for the implicit operator that will cast into a ModuleConnectedLivingSpace I get:


// Allow a CLSPart to be cast into a ModuleConnectedLivingSpace.
public static implicit operator ModuleConnectedLivingSpace(CLSPart _p)
{
Type CLSPartType = AssemblyLoader.loadedAssemblies.SelectMany(a => a.assembly.GetExportedTypes()).SingleOrDefault(t => t.FullName == "ConnectedLivingSpace.CLSPart");
MethodInfo method = CLSPartType.GetMethod("op_Implicit", BindingFlags.Public | BindingFlags.Static);
ModuleConnectedLivingSpace retVal = (ModuleConnectedLivingSpace)method.Invoke(null, new object[] { _p });
return (retVal);
}

The problem is that this is the identical bit of code apart from the cast in the last line. It seems to me that this can not be correct. How does the code know to invoke the "implicit operator Part" rather than the "implicit operator ModuleConnectedLivingSpace"? it seems to me that calling GetMethod.("op_Implicit", ... ) is not going to specify which of these two casts I am interested in, so there must be a way of distinguishing between then. Does anyone know what it is?

Link to comment
Share on other sites

Perhaps also try "op_Explicit"? http://bytes.com/topic/c-sharp/answers/903775-getting-operators-using-reflection

Other than that, I'd try checking the return type of the method, and also if there are multiple methods with the same name.

That's just quick off the top of my head.

Edit: Also, are you sure this will work at all? You cannot cast into a type that is not known to the assembly where the source object is coming from. Conversely, you can also not cast into a type that is not known to the receiving assembly.

Link to comment
Share on other sites

Don't! I think you are probably choosing a great tool for the wrong task. Reflection-Wrappers are great, if someone using your mod have a soft dependency on your mod. Usually, the main task of such a wrapper is to provide a. version compatible and b. provide a default implementation in case your api isn't present on that installation. You code above does none of it. It also would horrible in terms of performance if used somewhat more frequently, and implicit operators trend to be used quite often.

What is your primary intention of providing a reflection wrapper?

Blizzy is right, you would have to provide wrappers for all classes of your assembly. Also only use reflection at initialization, not at call time (for perf). As already mentioned it would make sense to make your methods "transparent" in case your mod isn't present, so other mods don't frequently have to check whether YourWrapper.IsPresent. Thus a default implementations for GetConnectedLivingSpaces(part) might return an empty enumerator...

For larger mods it might makes sense to have an "exports.cs" file, that contains all static methods you use on reflection. This way you won't accidentally break reflection without getting a compiler warning. And so on... but again first make sure such a wrapper actually gives you a real benefit.

Link to comment
Share on other sites

Thanks for your reply. Let me take a step back and explain where I have got to, and then perhaps you can suggest a better approach.

So I have written a mod called Connected Living Space (CLS) that takes a look at your craft and some config related to the parts and figures out where a kerbals can move internally within the craft. It then generates a set of data that describes this with a view to other mods using it to do whatever they are looking to do. The idea is that there becomes one place to set config describing how kerbals can move within the parts of your new funky part pack, and there is one place you can go to to figure out the answer to questions such as "can a kerbal move from part a to part b without going EVA?" or "does Bill a have to share a living space with Jeb, and is it slowly driving him insane?" So far it is starting to work nicely, with players producing config for some of the major part packs and one particular mod (ShipManifest) using it to enhance its crew transfer feature, and interest from other modders as well for using it with their projects.

However the author of Ship Manifest has received lots of hastle from players who are not into CLS saying "why have you broken SM" and "I do not want to use CLS" etc etc. At the moment CLS is a hard dependency for Ship Manifest, and whenever I make a new release of CLS and bump up the version number, SM needs to be rebuilt, which leads to more hassle, and incompatibilities etc. The author of Ship Manifest has been amazing patient and helpful, and has been very supportive of CLS, however it seems life would be easier if it was possible to Ship Mnaifest to work one way is CLS is not installed, and use CLS features if it is, without having to explain what CLS is and where the config options are to turn it off to players that simply do not care.

I thought that the best approach to this problem was to use reflection to write a wrapper around CLS. That way Ship manifest will still work if CLS is not installed. It would also encourage other modders to provide support for CLS if a toolbox style wrapper was available.

The issue with implicit operator casts is a consequence of writing a wrapper for all the publicly exposed CLS functionality. It is almost complete and it is just these implicit operator methods left to sort out. I might have written CLS not to provide such implicit type conversions, but it seemed like a nice way of doing things at the time.

So what do you think my best way forward is? I feel guilty every time I make a release and PapaJoe catches a load of stick from his user-base over all the dependency issues.

Link to comment
Share on other sites

Thanks for your reply. Let me take a step back and explain where I have got to, and then perhaps you can suggest a better approach.

So I have written a mod called Connected Living Space (CLS) that takes a look at your craft and some config related to the parts and figures out where a kerbals can move internally within the craft. It then generates a set of data that describes this with a view to other mods using it to do whatever they are looking to do. The idea is that there becomes one place to set config describing how kerbals can move within the parts of your new funky part pack, and there is one place you can go to to figure out the answer to questions such as "can a kerbal move from part a to part b without going EVA?" or "does Bill a have to share a living space with Jeb, and is it slowly driving him insane?" So far it is starting to work nicely, with players producing config for some of the major part packs and one particular mod (ShipManifest) using it to enhance its crew transfer feature, and interest from other modders as well for using it with their projects.

However the author of Ship Manifest has received lots of hastle from players who are not into CLS saying "why have you broken SM" and "I do not want to use CLS" etc etc. At the moment CLS is a hard dependency for Ship Manifest, and whenever I make a new release of CLS and bump up the version number, SM needs to be rebuilt, which leads to more hassle, and incompatibilities etc. The author of Ship Manifest has been amazing patient and helpful, and has been very supportive of CLS, however it seems life would be easier if it was possible to Ship Mnaifest to work one way is CLS is not installed, and use CLS features if it is, without having to explain what CLS is and where the config options are to turn it off to players that simply do not care.

I thought that the best approach to this problem was to use reflection to write a wrapper around CLS. That way Ship manifest will still work if CLS is not installed. It would also encourage other modders to provide support for CLS if a toolbox style wrapper was available.

The issue with implicit operator casts is a consequence of writing a wrapper for all the publicly exposed CLS functionality. It is almost complete and it is just these implicit operator methods left to sort out. I might have written CLS not to provide such implicit type conversions, but it seemed like a nice way of doing things at the time.

So what do you think my best way forward is? I feel guilty every time I make a release and PapaJoe catches a load of stick from his user-base over all the dependency issues.

@codepoet,

First, it was my decision to incorporate CLS, so the burden is mine not yours. As with all "new" things, there will be "resistance". I can live with that, and since the plugin does allow players to "turn it off", they can get the experience they want, even if it means adding another plugin. So please don't feel guilty. We are in this together, and the end results will justify the "initial pain" on introducing something new. :)

Now, I must admit, I've not done much with reflection, so I'm in unfamiliar territory. I'm going to do some research and see what I come up with.

Link to comment
Share on other sites

I just want to say I like the concept of CLS. The reason why I don't want to use them is simple: I don't need it, because my own game rule is to not to transfer resources or crew, if there is no "imagine" connection. It's not because I don't like the idea. The idea is great, but I need not a script what I have already working in my brain. I have a lot of mods and plugins installed and really need to keep all resources for performance I can :)

Edited by acc
Link to comment
Share on other sites

Don't! I think you are probably choosing a great tool for the wrong task. Reflection-Wrappers are great, if someone using your mod have a soft dependency on your mod. Usually, the main task of such a wrapper is to provide a. version compatible and b. provide a default implementation in case your api isn't present on that installation. You code above does none of it. It also would horrible in terms of performance if used somewhat more frequently, and implicit operators trend to be used quite often.

What is your primary intention of providing a reflection wrapper?

Blizzy is right, you would have to provide wrappers for all classes of your assembly. Also only use reflection at initialization, not at call time (for perf). As already mentioned it would make sense to make your methods "transparent" in case your mod isn't present, so other mods don't frequently have to check whether YourWrapper.IsPresent. Thus a default implementations for GetConnectedLivingSpaces(part) might return an empty enumerator...

For larger mods it might makes sense to have an "exports.cs" file, that contains all static methods you use on reflection. This way you won't accidentally break reflection without getting a compiler warning. And so on... but again first make sure such a wrapper actually gives you a real benefit.

The concept of checking only at initialization would work for me, as I would check for the existence of the dll, and flip a switch to bypass any CLS calls. I theory that would work, as I would make no calls except for the first to initialize my environment.

Link to comment
Share on other sites

The problem is that this is the identical bit of code apart from the cast in the last line. It seems to me that this can not be correct. How does the code know to invoke the "implicit operator Part" rather than the "implicit operator ModuleConnectedLivingSpace"? it seems to me that calling GetMethod.("op_Implicit", ... ) is not going to specify which of these two casts I am interested in, so there must be a way of distinguishing between then. Does anyone know what it is?

I found this on the web. not sure if it is helpful or not (you may be well past this), but you can decide.

http://chrisxwallis.wordpress.com/2010/10/01/late-binding-to-a-class-librarydll-in-c/

Upon consideration, I got to thinking.

If the interface is instantiated at run time, and we do the dll existence check there, If the dll is not htere I simpley end up with a null for the object. If there, and an object is in existence, I would think that the need for the function level reflection becomes moot.. Do I understand correctly? At that point I can make the calls to the functions via the Instantiated dll object's interface methods...

Too simple? am I missing something? both objects are .NET, so the need for native binding does not exist....

Edited by Papa_Joe
Link to comment
Share on other sites

Yes, too simple I am afraid. The problem is that if you code makes calls to the CLS assembly then the assembly loader will want it to be present, and barf if it is not. The theory behind the reflection wrapper is that the wrapper (a file full of code that you include in your project) provides implementations of all the classes etc that you are expecting to use, but in your namespace, rather than CLS's. Instead of referencing the CLS assembly, you build against that file, which will compile as the wrapper file is providing all the stuff that the CLS assembly would have. The wrapper then uses reflection to load the CLS assembly (if possible) and if it is there, use reflection to call its methods etc. If we did not use reflection to do this a dependency to CLS assembly would be created.

I do not think that there is a problem with the principle (obviosuly I have a few technical things to shake out) but I am alarmed to read Faark saying "don't bother" as I understood that this is a good example of when a reflection wrapper is the tool for the job. If it is not the best tool for the job, then what is, because simply creating a dependency can cause users frustration, and that is then a barrier to other modders using CLS, which I do not want.

Link to comment
Share on other sites

For clarity in the discussion, here is the wrapper file I have been writing. None of this code has been run yet, but it is "code complete", however I am pretty un hopeful that the operator implicits will work.


using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Text;
using UnityEngine;


namespace KerbalHotSeat
{
public class CLSAddon
{
private static bool? _CLSAvailable = null;
private static CLSAddon _instance;
private CLSTypes types = new CLSTypes();
private PropertyInfo vesselProperty;
private MethodInfo vesselPropertyGet;
private object realCLSAddon;


/// <summary>
/// Whether the Toolbar Plugin is available.
/// </summary>
public static bool CLSAvailable
{
get
{
if (_CLSAvailable == null)
{
_CLSAvailable = Instance != null;
}
return (bool)_CLSAvailable;
}
}

/// <summary>
/// The global tool bar manager instance.
/// </summary>
public static CLSAddon Instance
{
get
{
if ((CLSAvailable != false) && (_instance == null))
{
Type type = CLSTypes.getType("ConnectedLivingSpace.CLSAddon");
if (type != null)
{
object realCLSAddon = CLSTypes.getStaticProperty(type, "Instance").GetValue(null, null);
_instance = new CLSAddon(realCLSAddon);
}
}
return _instance;
}
}

public CLSVessel Vessel
{
get
{
CLSVessel returnVal = (CLSVessel)vesselPropertyGet.Invoke(realCLSAddon, null);
return returnVal;
}
}

// Constructor - the wrapper gets a refference to the real object passed in, stores it and can use it for future calls.
private CLSAddon(object realCLSAddon)
{
this.realCLSAddon = realCLSAddon;
this.vesselProperty = CLSTypes.getProperty(types.CLSVesselType, "Vessel");
this.vesselPropertyGet = vesselProperty.GetGetMethod();
}


}


public class CLSVessel
{
private object realCLSVessel;
private PropertyInfo SpacesProperty;
private PropertyInfo PartsProperty;
private MethodInfo ClearMethod;
private MethodInfo HighlightMethod;
private MethodInfo SpacesPropertyGet;
private MethodInfo PartsPropertyGet;

public CLSVessel()
{
Type CLSVesselType = CLSTypes.getType("ConnectedLivingSpace.CLSVessel");
realCLSVessel = Activator.CreateInstance(CLSVesselType, null);
ClearMethod = CLSTypes.getMethod(CLSVesselType, "Clear");
HighlightMethod = CLSTypes.getMethod(CLSVesselType, "Highlight");
SpacesProperty = CLSTypes.getProperty(CLSVesselType,"Spaces");
SpacesPropertyGet = SpacesProperty.GetGetMethod();
PartsProperty = CLSTypes.getProperty(CLSVesselType, "Parts");
PartsPropertyGet = PartsProperty.GetGetMethod();
}

public List<CLSSpace> Spaces
{
get
{
List<CLSSpace> returnVal = (List<CLSSpace>)SpacesPropertyGet.Invoke(realCLSVessel,null);
return returnVal ;
}
}

public List<CLSPart> Parts
{
get
{
List<CLSPart> returnVal = (List<CLSPart>)PartsPropertyGet.Invoke(realCLSVessel, null);
return returnVal;
}
}

public void Clear()
{
ClearMethod.Invoke(realCLSVessel, null);
}

public void Highlight(bool arg)
{
HighlightMethod.Invoke(realCLSVessel, new object[] {arg});
}
}


// Warpper class for the CLSSpace object
public class CLSSpace
{
private object realCLSSpace;
private PropertyInfo CrewProperty;
private PropertyInfo MaxCrewProperty;
private PropertyInfo PartsProperty;
private PropertyInfo NameProperty;
private PropertyInfo VesselProperty;
private MethodInfo HighlightMethod;
private MethodInfo CrewPropertyGet;
private MethodInfo MaxCrewPropertyGet;
private MethodInfo PartsPropertyGet;
private MethodInfo NamePropertyGet;
private MethodInfo NamePropertySet;
private MethodInfo VesselPropertyGet;

public List<CLSPart> Parts
{
get
{
List<CLSPart> returnVal = (List<CLSPart>)PartsPropertyGet.Invoke(realCLSSpace, null);
return returnVal;
}
}

public int MaxCrew
{
get
{
int returnVal = (int)MaxCrewPropertyGet.Invoke(realCLSSpace, null);
return returnVal;
}
}

public String Name
{
get
{
String returnVal = (String)NamePropertyGet.Invoke(realCLSSpace, null);
return returnVal;
}
set
{
NamePropertySet.Invoke(realCLSSpace, new object[] { value });
}
}

public CLSVessel Vessel
{
get
{
CLSVessel returnVal = (CLSVessel)VesselPropertyGet.Invoke(realCLSSpace, null);
return returnVal;
}
}

public List<CLSKerbal> Crew
{
get
{
List<CLSKerbal> returnVal = (List<CLSKerbal>)CrewPropertyGet.Invoke(realCLSSpace, null);
return returnVal;
}
}

public CLSSpace(CLSVessel v)
{
Type CLSSpaceType = CLSTypes.getType("ConnectedLivingSpace.CLSSpace");
realCLSSpace = Activator.CreateInstance(CLSSpaceType, new object[] { v });

HighlightMethod = CLSTypes.getMethod(CLSSpaceType, "Highlight");
CrewProperty = CLSTypes.getProperty(CLSSpaceType, "Crew");
MaxCrewProperty = CLSTypes.getProperty(CLSSpaceType, "MaxCrew");
PartsProperty = CLSTypes.getProperty(CLSSpaceType, "Parts");
NameProperty = CLSTypes.getProperty(CLSSpaceType, "Name");
VesselProperty = CLSTypes.getProperty(CLSSpaceType, "Vessel");
CrewPropertyGet = CrewProperty.GetGetMethod();
MaxCrewPropertyGet = MaxCrewProperty.GetGetMethod();
PartsPropertyGet = PartsProperty.GetGetMethod();
NamePropertyGet = NameProperty.GetGetMethod();
NamePropertySet = NameProperty.GetSetMethod();
VesselPropertyGet = VesselProperty.GetGetMethod();
}

public void Highlight(bool val)
{
HighlightMethod.Invoke(realCLSSpace, new object[] { val });
}
}

public class CLSPart
{
private object realCLSPart;
private PropertyInfo SpaceProperty;
private PropertyInfo HatchStatusProperty;
private PropertyInfo DockedProperty;
private PropertyInfo CrewProperty;
private PropertyInfo HabitableProperty;
private PropertyInfo NavigableProperty;

private MethodInfo HighlightMethod;

private MethodInfo ImplicitPartCastMethod;

private MethodInfo CrewPropertyGet;
private MethodInfo SpacePropertyGet;
private MethodInfo DockedPropertyGet;
private MethodInfo HabitablePropertyGet;
private MethodInfo NavigablePropertyGet;

public CLSPart(Part p)
{
Type CLSPartType = CLSTypes.getType("ConnectedLivingSpace.CLSPart");
realCLSPart = Activator.CreateInstance(CLSPartType, new object[] { p });

HighlightMethod = CLSTypes.getMethod(CLSPartType, "Highlight");
ImplicitPartCastMethod = CLSTypes.getMethod(CLSPartType, "op_Implicit");
CrewProperty = CLSTypes.getProperty(CLSPartType, "Crew");
CrewPropertyGet = CrewProperty.GetGetMethod();
DockedProperty = CLSTypes.getProperty(CLSPartType, "Docked");
DockedPropertyGet = DockedProperty.GetGetMethod();
HabitableProperty = CLSTypes.getProperty(CLSPartType, "Habitable");
HabitablePropertyGet = HabitableProperty.GetGetMethod();
NavigableProperty = CLSTypes.getProperty(CLSPartType, "Navigable");
NavigablePropertyGet = NavigableProperty.GetGetMethod();
}

public CLSSpace Space
{
get
{
CLSSpace returnVal = (CLSSpace)SpacePropertyGet.Invoke(realCLSPart, null);
return returnVal;
}
}

public bool Docked
{
get
{
bool returnVal = (bool)DockedPropertyGet.Invoke(realCLSPart,null);
return returnVal;
}
}

public List<CLSKerbal> Crew
{
get
{
List<CLSKerbal> returnVal = (List<CLSKerbal>)CrewPropertyGet.Invoke(realCLSPart, null);
return returnVal;

}
}

// Allow a CLSPart to be cast into a Part
public static implicit operator Part(CLSPart _p)
{
Type CLSPartType = AssemblyLoader.loadedAssemblies.SelectMany(a => a.assembly.GetExportedTypes()).SingleOrDefault(t => t.FullName == "ConnectedLivingSpace.CLSPart");
MethodInfo method = CLSPartType.GetMethod("op_Implicit", BindingFlags.Public | BindingFlags.Static);
Part retVal = (Part)method.Invoke(null,new object[] { _p });
return (retVal);
}

// Allow a CLSPart to be cast into a ModuleConnectedLivingSpace. Note that this might fail, if the part in question does not have the CLS module configured.
public static implicit operator ModuleConnectedLivingSpace(CLSPart _p)
{
Type CLSPartType = AssemblyLoader.loadedAssemblies.SelectMany(a => a.assembly.GetExportedTypes()).SingleOrDefault(t => t.FullName == "ConnectedLivingSpace.CLSPart");
MethodInfo method = CLSPartType.GetMethod("op_Implicit", BindingFlags.Public | BindingFlags.Static);

object realModCLS = method.Invoke(null, new object[] { _p });

ModuleConnectedLivingSpace retVal = new ModuleConnectedLivingSpace(realModCLS);

return (retVal);
}

public void Highlight(bool val)
{
HighlightMethod.Invoke(realCLSPart, new object[] { val });
}

public bool Habitable
{
get
{
bool returnVal = (bool)HabitablePropertyGet.Invoke(realCLSPart, null);
return returnVal;
}
}

public bool Navigable
{
get
{
bool returnVal = (bool)NavigablePropertyGet.Invoke(realCLSPart, null);
return returnVal;
}
}
}

public class CLSKerbal
{
private object realCLSKerbal;
private PropertyInfo PartProperty;
private MethodInfo PartPropertyGet;

public CLSKerbal(ProtoCrewMember k, CLSPart p)
{
Type CLSKerbalType = CLSTypes.getType("ConnectedLivingSpace.CLSKerbal");
realCLSKerbal = Activator.CreateInstance(CLSKerbalType, new object[] { k,p });

PartProperty = CLSTypes.getProperty(CLSKerbalType, "Part");
PartPropertyGet = PartProperty.GetGetMethod();
}

public static implicit operator ProtoCrewMember(CLSKerbal _k)
{
ProtoCrewMember retVal = (ProtoCrewMember)_k.realCLSKerbal;
return retVal;
}

public CLSPart Part
{
get
{
CLSPart returnVal = (CLSPart)PartPropertyGet.Invoke(realCLSKerbal, null);
return returnVal;
}
}
}

public class ModuleConnectedLivingSpace : PartModule
{
private object realModCLS; // TODO how does this get set?

// Constructor for the wrapper class. The actual class does not have a constructor
public ModuleConnectedLivingSpace(object real)
{
this.realModCLS = real;
}

// Allow a Part to be cast into a ModuleConnectedLivingSpace.
public static implicit operator ModuleConnectedLivingSpace(Part _p)
{
Type CLSPartType = AssemblyLoader.loadedAssemblies.SelectMany(a => a.assembly.GetExportedTypes()).SingleOrDefault(t => t.FullName == "ConnectedLivingSpace.CLSPart");
MethodInfo method = CLSPartType.GetMethod("op_Implicit", BindingFlags.Public | BindingFlags.Static);

object realModCLS = method.Invoke(null, new object[] { _p });

ModuleConnectedLivingSpace retVal = new ModuleConnectedLivingSpace(realModCLS);

return (retVal);

}

public override string GetInfo()
{
Type ModuleConnectedLivingSpaceType = CLSTypes.getType("ConnectedLivingSpace.ModuleConnectedLivingSpace");

MethodInfo methodInfo = CLSTypes.getMethod(ModuleConnectedLivingSpaceType,"GetInfo");

string retVal = (string)methodInfo.Invoke(this.realModCLS, null);

return retVal;
}
}


// Class to help with some of the reflection stuff.
internal class CLSTypes
{
internal readonly Type CLSAddonType;
internal readonly Type CLSVesselType;
internal readonly Type CLSSpaceType;
internal readonly Type CLSPartType;
internal readonly Type CLSKerbalType;

internal CLSTypes()
{
CLSAddonType = getType("ConnectedLivingSpace.CLSAddon");
CLSVesselType = getType("ConnectedLivingSpace.CLSVessel");
CLSSpaceType = getType("ConnectedLivingSpace.CLSSpace");
CLSPartType = getType("ConnectedLivingSpace.CLSPart");
CLSKerbalType = getType("ConnectedLivingSpace.CLSKerbal");
}

internal static Type getType(string name)
{
return AssemblyLoader.loadedAssemblies.
SelectMany(a => a.assembly.GetExportedTypes()).
SingleOrDefault(t => t.FullName == name);
}

internal static PropertyInfo getProperty(Type type, string name)
{
return type.GetProperty(name, BindingFlags.Public | BindingFlags.Instance);
}

internal static PropertyInfo getStaticProperty(Type type, string name)
{
return type.GetProperty(name, BindingFlags.Public | BindingFlags.Static);
}

internal static EventInfo getEvent(Type type, string name)
{
return type.GetEvent(name, BindingFlags.Public | BindingFlags.Instance);
}

internal static MethodInfo getMethod(Type type, string name)
{
return type.GetMethod(name, BindingFlags.Public | BindingFlags.Instance);
}

internal static FieldInfo getField(Type type, string name)
{
return type.GetField(name);
}

}



}

Link to comment
Share on other sites

Thats actually an interesting concept. You would end up with a bunch of duplicate interface files, since they cannot be changed once released anymore. Also our APIs would have to implement a lot of them. Not very practical and i don't really think it will be adopted for KSP, but it should work.

I'm currently creating such a wrapper for LoD as well. Not yet done, but that is what i have so far: API side, External wrapper.

It certainly isn't perfect, but there are a bunch of design decisions you might want to adopt or consider.

- Do not use Reflection when executing your API methods. Do that when you initialize your wrapper. The code shown initially loops though all assemblies and all types on every single implicit cast (what should be an ultra lightweight call), what ofc doesn't make much sense. Toolbar and your remaining code at least stores MethodInfo's. Thats kinda better, but still has to do a bunch of type checking at execution. Delegates are probably the best way to go, see methodInfo.CreateDelegate.

- You will change your code. Because of that i prefer to not let the wrapper "search" for methods by name, argument list, type or visibility. I have sth like "Delegate[] GiveMeFunctionsForWrapperVersion(int version)". In future version i could change my API however i want but the old wrapper might still function, as long as i still provide those single method and it still returns compatible delegates. It also allows me to not use any reflection except for this initial call.

- Type safety. Another reason to use delegates. I am forwarding a list of delegates, so i kinda break type safety for a seconds. But it would crash on initialization and more importantly i will get compiler errors whenever i change sth unintentionally. Except ofc its a change in GiveMeFunctionsForWrapperVersion(..), but thats a much simpler rule to follow (don't change it unless you change and consider wrappers as well).

- What I'm not very good in yet is making this wrapper in a way that the user don't has to bother whether LOD is available. I intend that all Wrapper calls are at least either ignored or return default values that you can use as "usual". In your case it seems (after a very brief look) like SM only uses Vessel.GetSpaces and sth like CanTransfer(fromPart, toPart). So you could provide a simple default implementation for both of it and save whoever uses your API some work. But yes, in that case SM seems to not need this anymore and thus it isn't really necessary. Maybe some day some other developer tries to find what parts kerbals can live in instead of figuring it out by himself he can just use your API and thus also profit in case your mod is installed.

Regarding your code in particular:

- ModuleConnectedLivingSpace is wrong. It must not derive from PartModule, since that would mean you have 2 of them instantiated (one in your DLL, one in the DLL that included your wrapper)

- Just because i'm curious: Why did you choose implicit conversion over a readonly property?

public Part Part { get; private set; }

- Any of your constructors seem to create an instance in your mod.dll. Doesn't those already exist? If they never do, why can't someone just include this particular code/class into its own dll?

Link to comment
Share on other sites

Thanks, the code helps me a lot with understanding the problem.

@Faark, Your approach is similar to what I was thinking (tho my thinking was very rudimentary). Do the lifting in one place, and simply ignore the object if not properly instantiated.

Link to comment
Share on other sites

Faark, thanks for that - there is a lot to think about in there. As I have not done this stuff before it is all learning for me. I am now starting to wonder is implicit casts are a good idea. Life would be simpler without them. I am also wondering if it would be good to do a wrapper that has everything SM needs, and the add the rest later.

Link to comment
Share on other sites

Faark, thanks for that - there is a lot to think about in there. As I have not done this stuff before it is all learning for me. I am now starting to wonder is implicit casts are a good idea. Life would be simpler without them. I am also wondering if it would be good to do a wrapper that has everything SM needs, and the add the rest later.

Personally, I think that what is good for the community comes first. I can rework what I need to for the good of the group. The way I see it, if we have a chance to "do it right", let's take it.

Link to comment
Share on other sites

Personally, I think that what is good for the community comes first. I can rework what I need to for the good of the group. The way I see it, if we have a chance to "do it right", let's take it.

I agree with you on doing it right - one reason I swapped religion as a job and software as a hobby from the other way round is I hated not having the commercial space to get the software right.

Anyhow, think I will:

1) add a none opimplicit way of doing the things that those type conversions do.

2) provide a reflection wrapper for those read only properties but not the opimplicit ones.

3) acknowledge that the mod using CLS is at liberty to make a better reflection wrapper if they are concerned about performance.

4) synchronise a release with PapaJoe so CLS and SM that is does not have a hard dependency are released at the same time.

I hope that provides a way forwards that does not cut off the optiond for improvements in the future, but also gets us to the place we are heading towards.

Link to comment
Share on other sites

Personally, I think that what is good for the community comes first. I can rework what I need to for the good of the group. The way I see it, if we have a chance to "do it right", let's take it.

I agree with you on doing it right - one reason I swapped religion as a job and software as a hobby from the other way round is I hated not having the commercial space to get the software right.

Anyhow, think I will:

1) add a none opimplicit way of doing the things that those type conversions do.

2) provide a reflection wrapper for those read only properties but not the opimplicit ones.

3) acknowledge that the mod using CLS is at liberty to make a better reflection wrapper if they are concerned about performance.

4) synchronise a release with PapaJoe so CLS and SM that is does not have a hard dependency are released at the same time.

I hope that provides a way forwards that does not cut off the optiond for improvements in the future, but also gets us to the place we are heading towards.

Link to comment
Share on other sites

OK, I am stuck again :(

I have been getting my wrapper up together - I am sure there are things about it that could be better, but I just want to achieve functional to start with. However I have got stuck on my methods that return a List of objects as a return value.

In general whenever I need to return an object of a type that is being wrapped, I create a wrapper version of it and hide the reference inside. It looks like this:


public CLSSpace Space
{
get
{
CLSSpace returnVal = new CLSSpace(SpacePropertyGet.Invoke(realCLSPart, null));
return returnVal;
}
}

as the wrapper class has a constuctor that takes the object returned by the Invoke call.

All good, that works. However if the wrapped call returns a List<CLSPart> for example, I need to create a List of wrapped CLSPart objects. That means I need to take the object that comes back from the call to Invoke and iterate through it. However this does not seem to be possible without being able to deal with the wrapper CLSPart type. Here is some code to illustrate the problem:


public List<CLSSpace> Spaces
{
get
{
List<CLSSpace> returnValue = new List<CLSSpace>();

object list = SpacesPropertyGet.Invoke(realCLSVessel, null);

Debug.Log(list.ToString());

IEnumerable<object> listEnum = (IEnumerable<object>)list; // This call fails with a cast exception

if(null != list)
{
foreach (object o in listEnum)
{
returnValue.Add(new CLSSpace(o));
}
}

return returnValue;
}
}

I want to iterate through the list variable, but I can't without being able to handle the unwrapped CLSSpace type. Can anyone suggest a solution?

Link to comment
Share on other sites

First of all, I'd like to mention that I think you're on the right track, what with all the wrapping and such.


IEnumerable<object> listEnum = (IEnumerable<object>)list; // This call fails with a cast exception

Is that even possible in C#? It is possible in Java because of erasure, which means that at runtime, a List<String> is the same as List<Integer> or even List (not at compile-time.) If it's not possible in C#, you could try casting to just IEnumerable. If that also fails, I only see getting the elements via reflection instead of regular means.

Link to comment
Share on other sites

I do all my best thinking in the bath - here is what happened this morning...

I am thinking "why is this so hard - it was never like this back in the COM days - we had IDispatch and everything - it was no problem."

The i realised that of course the thing that makes it easy with COM to do what I am seeking to do (i.e loose binding) is the ability to publish your interface separately from your implementation. That is what I am missing - defining interfaces!

So here is the plan:

1) I define a whole load of ICLSPart, ICLSSpace etc interfaces in a single CLSInterfaces.cs file

2) I get my various objects to implement those interfaces and ship that as an assembly

3) I provide a CLSWrapper.cs that also implements those interfaces, but under the covers does all the reflection stuff required. However the good news is that I no longer need to worry about the reflection code not knowing about my types, because it does not need to know about my types - it just needs to know about my interfaces, which it already does.

any problems?

Link to comment
Share on other sites

3) I provide a CLSWrapper.cs that also implements those interfaces

Not possible. You cannot use any type (interface or not) without creating a hard dependency.

This is why I ended up adding all public types in the Toolbar Plugin API to the wrapper as well, to make it absolutely transparent to the user of the wrapper whether they're using the wrapper or the real thing.

Link to comment
Share on other sites

Is that even possible in C#? It is possible in Java because of erasure, which means that at runtime, a List<String> is the same as List<Integer> or even List (not at compile-time.)

They introduced co & contra variance in some later version of .NET, what should make it possible. At least in the same assembly and at compile time. Ofc not in KSP/Unity, anyway.

Yes, a non-generic enumerable as suggested by blizzy should work. If you are fine with enumerables it could look like this:

IEnumerable<CLSSpace> Spaces
{
get
{
var list = (System.Collections.IEnumerable)SpacesPropertyGet.Invoke(realCLSVessel, null)
foreach (var obj in list)
{
yield return new CLSSpace(obj);
}
}
}

Ofc you could always a simple method in your API assembly that you use just for reflection and has simple objects that use only types from .net, unity and KSP instead of your own. No one is forcing you to use exactly the same "kind" of method in both assemblies. If you want to have implicit cast operators, the wrapper's operator could invoke some static method in the API that does the conversion (and maybe uses its "local" implicit cast operator, but you wrapper doesn't have to care how its done).

@Interfaces:

You have to creates the interfaces in a different project to make it work. Those your_interfaces.dll have to be referenced by both dlls (your API and the one using your stuff) and both of them would have to ship with this interfaces.dll next to their dll that implements them. You still need some reflection to get some objects with interfaces to the dll using your API in the first place (likely your casting or some kind of "manager"). You can never again change your interfaces.dll without breaking compatibility. If you want to add a feature, create a new interfacesV2.dll and make your types in the api implement both versions.

Edited by Faark
Link to comment
Share on other sites

The caveats you have pointed out with the interfaces route is what I would have expected - the interfaces.dll assembly plays the role of the typelibrary in the COM world, and allows clients to know what they have got to build against before they ever see the implementation. From where I am coming from interfaces are supposed to be well defined and unchanging, so I am cool with that - like you say it is always possible to add a new interface.

So would it be such a bad thing if we ended up with:

CLS.dll which provides the CLS implementation

CLSInterfaces.dll that defines the interfaces

SM.dll which has a reference to CLSInterfaces.dll that is shipped alongside SM.dll

Would it matter that there are two CLSInterface.dll files hanging around? Would they both get loaded, or would there be separate dependency paths?

Link to comment
Share on other sites

Would it matter that there are two CLSInterface.dll files hanging around? Would they both get loaded, or would there be separate dependency paths?

There better be 2 CLSInterface.dll's, to make sure KSP can load them properly. Yes, KSP should only load the first one it finds and reuse that, as long as they have the same assembly name, version, etc. Otherwise it would be two different types internally and thus wouldn't work.

Btw, with interfaces you definitively can't use operators :P

Instead you will have a single [singelton?] class / .cs file SM has to include that takes care of stuff like that and "first contact" in general (e.g. getting a reference to some singelton object that implements a general "manager" interface).

Link to comment
Share on other sites

This thread is quite old. Please consider starting a new thread rather than reviving this one.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...