WIP: Instance drawing #14
Conversation
…arted with a more advanced way of mimicking instance creation and handling in the test harness.
Add a test case to the GraphicsTestPlugin
|
@jaurenq can you have a look at the XPNet API for instancedrawing. CreateInstance is now a method of the XPSceneryObject, but maybe you would prefer a solution that is nearer to the XP SDK here, that is, something like |
|
I will also remove the WIP, as this is in a quite mature state already |
|
I'll check it out. My preference on merging back to master would be to only merge things that are totally ready to go - meaning I'd create a new nuget package for end-users to use with a usable version of whatever new new APIs are included. Is that where this is? (Maybe it is, I haven't looked at it yet -- just you said "quite advanced", and "quite advanced" and "finished feature" aren't quite the same thing :-) |
| XPLM_API XPLMInstanceRef XPLMCreateInstance(XPLMObjectRef obj, const char ** datarefs) | ||
| { | ||
| instanceCounter++; | ||
| std::cout << "XPLMTestHarness: Creating instance for object " << obj << std::endl; |
There was a problem hiding this comment.
Would prefer to have the test harness just store and provide data, without logging to the console. The data can then be verified/asserted/logged either over in the test plugin (C#) or in XPNetPluginTestHost.cpp. If you didn't notice, there were no log messages here in the harness prior to your push last month. One of the directions I want to move with XPNet is to turn the test host into an actual automated unit test suite, which would mean that the places where it currently says, "look for log message XYZ to see if the test succeeded" would turn into tests that are checked for pass/fail. In preparation for that, all of the test output was isolated to XPNetPluginTestHost.cpp, and the harness here just tried/tries to be a simple stand-in for X-Plane. It's OK to do testing by checking logging output currently (that's how the other tests work now as well), but the logging and checking should be done in the test plugin or the test host, not the test harness, to reduce the amount of rework we'll have to do to make those tests automated. (If you're not sure how to do what I'm talking about, or are unfamiliar with automated unit testing, let me know and we'll discuss).
XPNet.CLR/Scenery/XPlaneScenery.cs
Outdated
| @@ -71,6 +73,14 @@ public void Dispose() | |||
| { | |||
| PluginBridge.ApiFunctions.XPLMUnloadObject(m_objectRef); | |||
| } | |||
| public IXPInstance CreateInstance(IEnumerable<string> inDataRefs) | |||
There was a problem hiding this comment.
Looking at some examples of how XPLMCreateInstance is called, it may be useful for our callers for this to either look like Create(params string[] data) or to at least have an override that looks like that. Then you can write something like this:
var inst = m_api.Instance.Create(
"sim/graphics/animation/ground_traffic/tire_steer_deg_a",
"sim/graphics/animation/ground_traffic/tire_steer_deg_b",
"sim/graphics/animation/ground_traffic/tire_steer_deg_c"
);
I.e., then the caller doesn't even have to deal with making an array/list for simple cases. The compiler will treat this as an array allocation so there'd be no performance penalty to doing that.
|
@mbrachner have you tested Draw in X-Plane yet, or with more than one location (more than one XPLMDrawInfo_t) passed to it yet? The X-Plane API definition of XPLMDrawObjects accepts a pointer to an XPLMDrawInfo_t (which is actually the pointer to the first element in an array of them), but the mapping you added indicates that a single instance of XPLMDrawInfo_t will be passed (by value). If that's working, I'm not sure why, looks like it'd crash to me :-) |
|
It works in X-Plane, however just with one instance. Sorry, that was one mistake in coding and one in testing it, I completely disregarded the case of several instances. Will take care of it. |
Rename former XPProbeResult to XPProbeInfo.
| // XPLMProbeResult enum from the X-Plane API | ||
| ProbeHitTerrain = 0, | ||
| ProbeError = 1, | ||
| ProbeMissed = 2 |
There was a problem hiding this comment.
Same note here as elsewhere: I think these can just be HitTerrain, Error and Missed, b/c in .NET you already have to include the enum name to reference an enumeration, so you've already typed Probe once anytime you use these. So like XPProbeResult.HitTerrain instead of XPProbeResult.ProbeHitTerrain. (I could see this going either way, and reasonable people could disagree, this is more an argument for keeping the API consistent -- doing it the same way everywhere).
There was a problem hiding this comment.
In fact, I had your previous comment in mind here, and I was hesitating. The thing here is that the class is a ProbeResult, and not a probe. So if we had an enumerable "ProbeResultOK", "ProbeResultError" or so, I would totally agree, and I would name it "OK" or "Error". But here I understand it in the way of "The probe result is, that the probe hit terrain", as it is also explicated in the SDK documentation. For the drawing phases the intention was clearer, because they separated the word "Phase" from the actual phase, that is, for example, xplm_Phase_FirstScene. In a way, the naming of the enumerables is not 100% consistent in the SDK, because I would have felt a more consistent way of enumerating this to be "xplm_Result_ProbeHitTerrain", "xplm_Result_ProbeMissed", and "xplm_Result_ProbeError". In the end I am totally okay with whatever you feel is best. Would you still prefer if I remove "Probe" here?
- Add creation routines for XPlane Instances
| { | ||
| public IXPInstance Create(IXPSceneryObject inSceneryObject, IEnumerable<string> inDataRefs) | ||
| { | ||
| return inSceneryObject.CreateInstance(inDataRefs); |
There was a problem hiding this comment.
@jaurenq what do you think about this solution? Now we we can "redirect" users, who want to create the instance via the instance API, so they can use it in a similar way as they would have done it in the SDK, they can also instantiate directly from the XPSceneryObject via CreateInstance, and we do not need to expose the object reference, keeping it private in the XPSceneryObject.
Add instance drawing API mappings to XPNet