Expands XLaunchXBE to allow caller to set LaunchData. #501
Conversation
abaire
force-pushed
the
adds_launch_data_handling
branch
from
cea2312
to
b7ec3f4
Compare
|
Do you have a specific use case for this code? |
|
Initially I had two use cases: The other would be using a similar approach to MAMEoX in having a menu launcher and a series of worker XBEs in some of the tests I've been writing to aid xemu contributions (e.g., https://github.com/abaire/nxdk_pgraph_tests). There are a few tests that are reliant on having a clean boot state to be valid; though this may just be because I haven't figured out the right set of explicit commands to reset to initial state (e.g., the behavior of the fixed function pipeline when lighting is enabled in a BEGIN_END but no normals are explicitly provided w/ the vertex data). That said, at the moment I'm already maintaining some patches on top of the nxdk to expose the internals of pbkit and I could always write info to a cache file so I'm not blocked by not having this functionality. |
| } | ||
| if (launchData) { | ||
| memcpy(launchDataPage->LaunchData, launchData, sizeof(launchDataPage->LaunchData)); | ||
| } |
There was a problem hiding this comment.
If launchData is NULL the LaunchDataPage will still be allocated and point at zero-bytes; is this intended?
There was a problem hiding this comment.
It is intentional that LaunchDataPage's LaunchData will be zero bytes (preserving the existing behavior).
lib/hal/xbox.c
Outdated
| if (LaunchDataPage == NULL) | ||
| void XLaunchXBEEx(const char *xbePath, const void *launchData) | ||
| { | ||
| if (LaunchDataPage == NULL) { | ||
| LaunchDataPage = MmAllocateContiguousMemory(0x1000); |
There was a problem hiding this comment.
This should probably be using LaunchDataPageSize now?
There was a problem hiding this comment.
Excellent point, done.
| if (!lastSlash) { | ||
| // if we couldn't find a trailing slash, the conversion to | ||
| // the xbox path mustn't have worked, so we will return | ||
| return; |
There was a problem hiding this comment.
nit: What should happen to the LaunchDataPage here? Should the allocation be undone?
There was a problem hiding this comment.
I suspect it should, but I was conservative about preserving the existing behavior which does leave the LDP allocated and persisted.
abaire
force-pushed
the
adds_launch_data_handling
branch
from
b7ec3f4
to
046601c
Compare
|
I gave this another look (sorry for taking so long), and I think it's fine to merge this as is. There are a couple of design issues, but those stem from the original code, not your changes. |
|
Commit prefix is incorrect; should be |
thrimbor
force-pushed
the
adds_launch_data_handling
branch
from
046601c
to
88c6f04
Compare
|
I took the liberty of fixing that myself. Merging now. |
XLaunchXBEthat allows theLaunchDatamember to be set on the persisted launch data struct.Basic test at abaire/nxdk_launch_test
Note that there seems to be a bug in the handling of "D:" prefixed paths (see #500).