-
Notifications
You must be signed in to change notification settings - Fork 256
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
Add controller port configuration GUI and support virtual SB controller with feedback status #1196
Add controller port configuration GUI and support virtual SB controller with feedback status #1196
Conversation
…event mis-use host equivalent. Add newly findings for device table related info.
now the XINPUT_STATE is correctly refering to host type, no need to specify the name space.
Prepare for configurabale host input, and virtual xbox controller.
always set state to enable rumble.
using console as controller output. using XInput and direct input combined keys
now registry works.
SBC controller feedback add possibility of using same dialog for virtual controller input.
sort of working. revert the dlg message handling from wnd.cpp
modeless dialog got issues. modal dialog will occupy focus. only way is to use a DLL, load the DLL when cxbx-r emulation child process started, and create a dummy window from DLL then create a modal dialog. lots of code change and clean up.
Please add a marker (like OLD_INPUT or something like that) to all code-blocks that you've disabled, and create an issue to remove all thusly marked code (otherwise, a lot of crap keeps lingering on). |
Nearly all files in src/CxbxVSBC are marked binary - that can't be right! Intentation and comment-style aren't consistent, please try to match existing code style.
EDIT by @RadWolfie: |
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.
See my comments for change request.
I don't see why submodules need to be updated in this pull request. If we need to update the submodules, it must be on its own pull request.
As for files on github show "Binary file not shown.", are you saving them with windows' newline? If so, I'm not sure why it's showing the message.
build/win32/Cxbx.vcxproj
Outdated
@@ -223,7 +223,7 @@ | |||
</HeaderFileName> | |||
</Midl> | |||
<ClCompile> | |||
<Optimization>Full</Optimization> | |||
<Optimization>Disabled</Optimization> |
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.
Cannot be unoptimized in master branch.
build/win32/Cxbx.sln
Outdated
@@ -128,13 +128,33 @@ Global | |||
{B8D9AFC2-B38F-4714-846D-8A2754F076C9}.Debug_Direct3D9|Any CPU.Build.0 = Release|Win32 | |||
{B8D9AFC2-B38F-4714-846D-8A2754F076C9}.Debug_Direct3D9|Win32.ActiveCfg = Debug|Win32 | |||
{B8D9AFC2-B38F-4714-846D-8A2754F076C9}.Debug_Direct3D9|Win32.Build.0 = Debug|Win32 | |||
{B8D9AFC2-B38F-4714-846D-8A2754F076C9}.Debug_Direct3D9|x64.ActiveCfg = Debug|x64 |
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.
Cannot have x64 build as Cxbx is only design to compile in x86 executable.
See below as well.
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.
removed with added commit
build/win32/Cxbx.vcxproj
Outdated
</ItemGroup> | ||
<ItemGroup> | ||
<None Include="..\..\CONTRIBUTORS" /> | ||
<None Include="..\..\COPYING" /> | ||
<None Include="..\..\README.md" /> | ||
<None Include="..\..\resource\.editorconfig" /> | ||
<None Include="..\..\resource\Logo.bmp" /> | ||
<None Include="..\..\resource\ribbon1.mfcribbon-ms" /> |
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.
I do not see ribbon1.mfcribbon-ms file in this pull request.
Also, I don't think it should be in here since I never heard of mfcribbon-ms file extension.
src/CxbxKrnl/EmuXapi.cpp
Outdated
|
||
|
||
|
||
void XTL::Load(const char *szRegistryKey) |
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.
Wrong place to put it. It should remain inside XBController.cpp file or different file inside /src/Common/Win32 directory. (part 1/4)
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.
XBcontroller is for directinput, that's what I have in my mind.
are you sure the xbcontroller is generalized with xinput?
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.
It may not contain any xinput. If you want to keep it separate, you can make a new file in src/Common/Win32
folder. I thought you had a new file in src/Common/Win32
directory for Xinput? All registry settings are in there to separate from main source code of Cxbx and CxbxKrnl's folders.
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.
ok, let me check what I can do
src/CxbxKrnl/EmuXapi.cpp
Outdated
// ****************************************************************** | ||
// * func: XTL::Save | ||
// ****************************************************************** | ||
void XTL::Save(const char *szRegistryKey) |
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.
Wrong place to put it. It should remain inside XBController.cpp file or different file inside /src/Common/Win32 directory. (part 2/4)
src/CxbxKrnl/EmuXapi.cpp
Outdated
} | ||
|
||
//Set HostType and HostPort setting from global array per xbox port. The setted value will take effect from next time xbe loading. | ||
void XTL::SetXboxPortToHostPort(DWORD dwXboxPort, DWORD dwHostType, DWORD dwHostPort) |
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.
Wrong place to put it. It should remain inside XBController.cpp file or different file inside /src/Common/Win32 directory. (part 3/4)
src/CxbxKrnl/EmuXapi.cpp
Outdated
g_XboxPortMapHostPort[dwXboxPort] = dwHostPort; | ||
} | ||
//retrieve HostType and HostPort setting from global array per xbox port. | ||
void XTL::GetXboxPortToHostPort(DWORD dwXboxPort, DWORD &dwHostType, DWORD &dwHostPort) |
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.
Wrong place to put it. It should remain inside XBController.cpp file or different file inside /src/Common/Win32 directory. (part 4/4)
build/win32/Cxbx.vcxproj.filters
Outdated
@@ -633,6 +640,7 @@ | |||
<None Include="..\..\resource\.editorconfig"> | |||
<Filter>Code Format\resource</Filter> | |||
</None> | |||
<None Include="..\..\resource\ribbon1.mfcribbon-ms" /> |
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.
Same here
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.
the ribbon1 is added by VS, I don't know why it's there. I will remove it.
src/CxbxVSBC/CxbxVSBC.vcxproj
Outdated
<ConfigurationType>DynamicLibrary</ConfigurationType> | ||
<UseDebugLibraries>true</UseDebugLibraries> | ||
<PlatformToolset>v141</PlatformToolset> | ||
<CharacterSet>Unicode</CharacterSet> |
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.
I believe this is the fault. Also PlatformToolset is recommended to use $(DefaultPlatformToolset)
above.
build/win32/Cxbx.vcxproj
Outdated
@@ -162,7 +162,7 @@ | |||
</HeaderFileName> | |||
</Midl> | |||
<ClCompile> | |||
<Optimization>Disabled</Optimization> | |||
<Optimization>Full</Optimization> |
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.
Um... Debug should never have optimization? I think you choose "all configurations" to set full optimization by mistake.
build/win32/Cxbx.vcxproj
Outdated
@@ -101,7 +101,7 @@ | |||
</HeaderFileName> | |||
</Midl> | |||
<ClCompile> | |||
<Optimization>Disabled</Optimization> | |||
<Optimization>Full</Optimization> |
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.
Same here, see comment below.
src/CxbxKrnl/EmuXapi.h
Outdated
// ****************************************************************** | ||
// accroding to XDK document, all game controller use 0x01 GAMEPAD device type. then specify the subtype in returned Capabilities when XInputGetCapabilities called. | ||
#define X_XINPUT_DEVTYPE_GAMEPAD 0x01 | ||
// SteelBatalion controller is the only one with special device type other than 1. |
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.
I repeat my previous comment: I suggest to drop references to the XDK in the code/comments, since it's technically illegal to have. Just say what the variable(s) is/are for, and we are good. Same for other instances in this PR
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.
What we can't do is copy and paste exact from source code. Reading/reviewing the help documentation is okay to mention if there are more complicated process need to explain (just can't have the word xdk in it).
For this type of note, I don't think it's worth mention the document here.
EDIT: Fix my wording.
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.
I agree. Direct mentioning to official resource will be removed.
@jackchentwkh @RadWolfie have either of you decided on what to do on the above mentioned review comments? It'd be nice to get this merged in soon! (Once deemed stable, that is) |
There are variety needs to be fix, haven't seen new commits to resolve them from his end. Right now, there's a merge conflict for |
Sorry I was busy this week. Shall find some time this weekend. |
Co-Authored-By: anita999 <anita999@ms12.hinet.net>
Thank you very much, RadWolfie! you're always helping and helpful!
After all the reviews been fixed. Verifying on my end to ensure configuration is working before approve to merge. |
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.
lgtm!
EDIT: My recommendation is to use squash and merge option.
Plus I do not own both Steel Battalion and hardware device. So I can't verify this part.
Lgtm - great job all around! |
This should be the last fixup for pull request.
I took a peek in AppVeyor to find out both release and debug zipped folder doesn't contain CxbxVSBC.dll file. New commit should resolve it. |
the newly merged LLE/HLE status altered the resource file which introduced conflicts. |
Such wonderful GUI windows as seen in the original post. But I have found six typos that could be fixed before testing begins for alpha 0.1. Typos: Maybe this one too? |
@ObiKKa thanks a lot for the corrections for typo. |
I got trouble rebasing this PR to master. |
I'm working on it. Then we can resolve the merge conflict to get this in master soon as possible. EDIT: Waiting for pull request jackchentwkh#4 to be merge. Then merge conflict should be resolve. |
Jc controller merge resolve
This reverts commit 034039d.
@jackchentwkh please document any important information about the Steel Battalion on the Xbox Dev Wiki . There's not much info there, and you've obviously done a great deal of research getting it into Cxbx-R |
Using dialog to let user assign which host controller he want to assign to each xbox port.
add preliminary support for virtual SteelBatallion controller. input mapped to 1st Xinput and directinput,
key mapping will be provided later.
also add a new dialog via CxbxVSBC.dll for virtual SteelBatallion controller feedback status.
later I will add GUI input support via the same dialog.
besides the virtual SteelBatallion controller, the user configuration for host controller assignment is a major change.
I also add some new structures together with my new findings for the DeviceTable and DeviceInfo of xbox devices.
EDIT by RadWolfie:
Here's the new change to GUI. (Used to have option toggle XInput option.)
Only with Steel Batallion title, controller feedback status will open.