-
Notifications
You must be signed in to change notification settings - Fork 15
WIP: Instance drawing #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 6 commits
fefe3b0
eb051e1
0eaf212
2c8f513
829c8aa
aeb99ca
6f905fb
b64b2d7
30df4cb
9cf6c1d
c2fe9b2
6a80edd
410fcee
becea96
dead143
e03c9bb
007afe0
ec81927
4801011
784a86e
dd1de73
ee20683
8e3107c
fd7f955
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,4 +13,5 @@ PublishProfiles | |
plugin | ||
package | ||
.DS_Store | ||
*.user | ||
*.user | ||
/XPNetPluginTestHost/xpnetcfg.json |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
|
||
namespace XPNet | ||
{ | ||
public interface IXPlaneInstance | ||
mbrachner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
void CreateInstance(); | ||
void DestroyInstance(); | ||
} | ||
|
||
internal class XPlaneInstance : IXPlaneInstance | ||
{ | ||
public void CreateInstance() | ||
{ | ||
throw new NotImplementedException(); | ||
} | ||
|
||
public void DestroyInstance() | ||
{ | ||
throw new NotImplementedException(); | ||
} | ||
} | ||
|
||
public interface IXPInstance : IDisposable | ||
{ | ||
void SetPosition(XPLMDrawInfo_t xPLMDrawInfo_t, IEnumerable<float> v); | ||
} | ||
|
||
internal unsafe class XPInstance : IXPInstance | ||
{ | ||
readonly void* m_instanceRef; | ||
mbrachner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
public XPInstance(void* instanceRef) | ||
{ | ||
m_instanceRef = instanceRef; | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
PluginBridge.ApiFunctions.XPLMDestroyInstance(m_instanceRef); | ||
} | ||
|
||
public void SetPosition(XPLMDrawInfo_t xPLMDrawInfo_t, IEnumerable<float> v) | ||
mbrachner marked this conversation as resolved.
Show resolved
Hide resolved
jaurenq marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
var floatArray = v.ToArray(); | ||
fixed (float* p = &floatArray[0]) | ||
{ | ||
PluginBridge.ApiFunctions.XPLMInstanceSetPosition(m_instanceRef, xPLMDrawInfo_t, p); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
|
||
namespace XPNet | ||
{ | ||
|
@@ -56,6 +57,7 @@ unsafe void Enumerator(string inFilePath, void* inRef) | |
public interface IXPSceneryObject : IDisposable | ||
{ | ||
void Draw(int lighting, int earthRelativ, XPLMDrawInfo_t[] drawInfos); | ||
IXPInstance CreateInstance(IEnumerable<string> inDataRefs); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you mentioned in your comments on the pull request, my general intent is to map each X-Plane top-level functional area to a top-level class in XPNet, to make it easy to use the X-Plane SDK docs as a general guide to how to use XPNet. Not that we map everything perfectly item-for-item, but rather that, if the X-Plane SDK says that there's something called the Instance API that you can call CreateInstance on, that you could make a pretty good guess that there'll be something in XPNet called Instance that you can call Create() or CreateInstance() on. Putting it on Scenery means we'd have to somehow lead people there, and it may make it harder for us to adjust if X-Plane adds major new features to their Instance API in the future (it's pretty tiny right now and kinda seems to go with Scenery, but who's to say that it will be forever - I'm sure they separated it for a reason). So where in X-Plane you have XPLMInstance and XPLMCreateInstance, in XPNet you'd have m_api.Instance.Create(). (Not CreateInstance b/c it's an Object-Oriented API, and you already called the function on an object named Instance, so you don't need to repeat the word Instance twice). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I can do that. Just one thought, if we create the XPInstance via the scenery object, we would have the chance to keep track of the created instances and automatically dispose them with the object. That would speak for the current approach. Would that change your opinion? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is another problem here, where the XPSceneryObject.CreateInstance would be a much more elegant solution: The XPSceneryObject holds a private reference to the SceneryObject (m_objectRef). So, if we have a Instance.Create(xpSceneryObject, dataRefs) in the API, how do we get that reference to call XPLMCreateInstance(m_objectRef, ...)? Making the field that holds this reference internal instead does not work, because we expect the interface IXPSceneryObject in Instance.Create, I guess we still need to use the interface, and we probably do not want to make it public. If we would stay with XPSceneryObject.CreateInstance, we have this reference easily available. Any ideas? |
||
} | ||
|
||
internal unsafe class XPSceneryObject : IXPSceneryObject | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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( 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. |
||
{ | ||
List<string> dataRefList = inDataRefs.ToList(); | ||
dataRefList.Add(null); | ||
string[] dataRefArray = dataRefList.ToArray(); | ||
var instanceRef = PluginBridge.ApiFunctions.XPLMCreateInstance(m_objectRef, dataRefArray); | ||
mbrachner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return new XPInstance(instanceRef); | ||
} | ||
|
||
public void Draw(int lighting, int earthRelative, XPLMDrawInfo_t[] drawInfos) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).