This should allow for better re-use / easier updates to be made to it in the future.
In GitLab by @xyen on Nov 24, 2019, 06:51
_Merges camhook -> master_
This should allow for better re-use / easier updates to be made to it in the future.
Can you make these keys and the following default values also macros? The mixture of the macros, which are also used in other config modules already, and constants implied a different kind of usage to me.
Can you make these keys and the following default values also macros? The mixture of the macros, which are also used in other config modules already, and constants implied a different kind of usage to me.
DANCERUSH and SDVX5 would be two that come to mind.
In GitLab by @praxis on Nov 24, 2019, 19:25
Commented on [Module.mk line 83](https://github.com/djhackersdev/bemanitools/compare/fd9b84392ba671b125e9669f945a237dc09f762d..e45b0927757b62ec0ce107517033b451e13406b0#diff-6690c45935f5f2c281abf648b5072040R83)
DANCERUSH and SDVX5 would be two that come to mind.
Nvm that comment (leaving it for clarity). Looking further down, I saw that you made this configurable. Then, I suggest adding a small comment pointing that out that the cam count can be changed.
Nvm that comment (leaving it for clarity). Looking further down, I saw that you made this configurable. Then, I suggest adding a small comment pointing that out that the cam count can be changed.
Question: Related to one of my previous comments, do you already know if these GUIDs are identical on other games? It's ok to keep it here for now. Introduce another refactoring step later to further refine this and make it re-usable with any other game once that's a use-case you/someone else is working on.
Question: Related to one of my previous comments, do you already know if these GUIDs are identical on other games? It's ok to keep it here for now. Introduce another refactoring step later to further refine this and make it re-usable with any other game once that's a use-case you/someone else is working on.
It'll be fine as long as games use the same API to enumerate/access the camera.
In GitLab by @praxis on Nov 24, 2019, 19:51
Commented on [src/main/iidxhook8/cam.c line 30](https://github.com/djhackersdev/bemanitools/compare/fd9b84392ba671b125e9669f945a237dc09f762d..e45b0927757b62ec0ce107517033b451e13406b0#diff-a948f88c931cbeeef23ad15099a22581L30)
This is an actual official/upstream GUID that MinGW mis-defined for the longest time: https://sourceforge.net/p/mingw-w64/bugs/770/
It'll be fine as long as games use the same API to enumerate/access the camera.
In order to improve the overall code base and avoiding silent regression, could you add unit-tests for the changes?
For the config module, you can copy-paste code from the already existing config tests, so this should be a low effort one.
For the camhook, can you write tests to verify the swap logic? Let me know if you have any questions on how to integrate this into the existing structure.
In order to improve the overall code base and avoiding silent regression, could you add unit-tests for the changes?
For the config module, you can copy-paste code from the already existing config tests, so this should be a low effort one.
For the camhook, can you write tests to verify the swap logic? Let me know if you have any questions on how to integrate this into the existing structure.
yep, in fact, that bug report in mingw is mine lol.
In GitLab by @xyen on Nov 24, 2019, 22:44
Commented on [src/main/iidxhook8/cam.c line 30](https://github.com/djhackersdev/bemanitools/compare/fd9b84392ba671b125e9669f945a237dc09f762d..e45b0927757b62ec0ce107517033b451e13406b0#diff-a948f88c931cbeeef23ad15099a22581L30)
yep, in fact, that bug report in mingw is mine lol.
the only game that uses 2 cameras atm. is IIDX, and it addresses the 2nd camera as 7. Both IIDX and SDVX (and I think DRS) address the first (and only camera in some cases) as 1.
In GitLab by @xyen on Nov 24, 2019, 22:46
Commented on [src/main/camhook/cam.c line 90](https://github.com/djhackersdev/bemanitools/compare/fd9b84392ba671b125e9669f945a237dc09f762d..e45b0927757b62ec0ce107517033b451e13406b0#diff-cc4d7cae74a7918058230af46b19ad1eR90)
the only game that uses 2 cameras atm. is IIDX, and it addresses the 2nd camera as 7. Both IIDX and SDVX (and I think DRS) address the first (and only camera in some cases) as 1.
yeah, the point here is that if a game requires more cameras, we can just edit the max number, add the keys in, and be good to go, will add some comments.
In GitLab by @xyen on Nov 24, 2019, 22:46
Commented on [src/main/camhook/config-cam.c line 12](https://github.com/djhackersdev/bemanitools/compare/fd9b84392ba671b125e9669f945a237dc09f762d..e45b0927757b62ec0ce107517033b451e13406b0#diff-bfed1f54ff3f065d496f9b82120d14e8R12)
yeah, the point here is that if a game requires more cameras, we can just edit the max number, add the keys in, and be good to go, will add some comments.
I already wrote a preliminary sdvxhook2, and it originally used a copy-paste of the iidxhook camera module, which is why I wanted to swap it out. DRS also uses the same camera mechanism for the RGB camera, although I haven't personally written a hook for it yet.
In GitLab by @xyen on Nov 24, 2019, 22:48
Commented on [Module.mk line 83](https://github.com/djhackersdev/bemanitools/compare/fd9b84392ba671b125e9669f945a237dc09f762d..e45b0927757b62ec0ce107517033b451e13406b0#diff-6690c45935f5f2c281abf648b5072040R83)
I already wrote a preliminary sdvxhook2, and it originally used a copy-paste of the iidxhook camera module, which is why I wanted to swap it out. DRS also uses the same camera mechanism for the RGB camera, although I haven't personally written a hook for it yet.
In GitLab by @tudor on Nov 24, 2019, 23:36
Commented on [Module.mk line 83](https://github.com/djhackersdev/bemanitools/compare/fd9b84392ba671b125e9669f945a237dc09f762d..e45b0927757b62ec0ce107517033b451e13406b0#diff-6690c45935f5f2c281abf648b5072040R83)
Which branch is sdvxhook2 on?
In GitLab by @xyen on Nov 24, 2019, 23:44
Commented on [Module.mk line 83](https://github.com/djhackersdev/bemanitools/compare/fd9b84392ba671b125e9669f945a237dc09f762d..e45b0927757b62ec0ce107517033b451e13406b0#diff-6690c45935f5f2c281abf648b5072040R83)
None, this needs to go upstream first.
In GitLab by @xyen on Nov 26, 2019, 06:32
added 1 commit
<ul><li>e45b0927 - camhook: fixup some minor formatting and enable iidxhook8-config-cam-test</li></ul>
[Compare with previous version](/djhackers/bemanitools/merge_requests/11/diffs?diff_id=1082&start_sha=671099fd7f99a874639eb0335ffeecabc4337239)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
In GitLab by @xyen on Nov 24, 2019, 06:51
Merges camhook -> master
This should allow for better re-use / easier updates to be made to it in the future.
A general qustion: What are the reasons to have this as a separate module? Are there more games that can re-use this?
Can you make these keys and the following default values also macros? The mixture of the macros, which are also used in other config modules already, and constants implied a different kind of usage to me.
In GitLab by @praxis on Nov 24, 2019, 19:25
Commented on Module.mk line 83
DANCERUSH and SDVX5 would be two that come to mind.
Do you know already that the software side is identical or that it is just using the same hardware?
Nvm that comment (leaving it for clarity). Looking further down, I saw that you made this configurable. Then, I suggest adding a small comment pointing that out that the cam count can be changed.
With the number of cams "configurable" and supporting different games, are these addresses always the same?
Nit: Code style, new line to separate the for loop block.
Question: Related to one of my previous comments, do you already know if these GUIDs are identical on other games? It's ok to keep it here for now. Introduce another refactoring step later to further refine this and make it re-usable with any other game once that's a use-case you/someone else is working on.
In GitLab by @praxis on Nov 24, 2019, 19:51
Commented on src/main/iidxhook8/cam.c line 30
This is an actual official/upstream GUID that MinGW mis-defined for the longest time: https://sourceforge.net/p/mingw-w64/bugs/770/
It'll be fine as long as games use the same API to enumerate/access the camera.
In order to improve the overall code base and avoiding silent regression, could you add unit-tests for the changes?
For the config module, you can copy-paste code from the already existing config tests, so this should be a low effort one.
For the camhook, can you write tests to verify the swap logic? Let me know if you have any questions on how to integrate this into the existing structure.
Ah, good to know. Might be worth a comment + the link for clarification, imo.
In GitLab by @xyen on Nov 24, 2019, 22:44
Commented on src/main/iidxhook8/cam.c line 30
yep, in fact, that bug report in mingw is mine lol.
In GitLab by @xyen on Nov 24, 2019, 22:45
@icex2 yeah, I was gonna replace the iidxhook8 camera test with one for camhook in general.
In GitLab by @xyen on Nov 24, 2019, 22:46
Commented on src/main/camhook/cam.c line 90
the only game that uses 2 cameras atm. is IIDX, and it addresses the 2nd camera as 7. Both IIDX and SDVX (and I think DRS) address the first (and only camera in some cases) as 1.
In GitLab by @xyen on Nov 24, 2019, 22:46
Commented on src/main/camhook/config-cam.c line 12
yeah, the point here is that if a game requires more cameras, we can just edit the max number, add the keys in, and be good to go, will add some comments.
In GitLab by @xyen on Nov 24, 2019, 22:48
Commented on Module.mk line 83
I already wrote a preliminary sdvxhook2, and it originally used a copy-paste of the iidxhook camera module, which is why I wanted to swap it out. DRS also uses the same camera mechanism for the RGB camera, although I haven't personally written a hook for it yet.
In GitLab by @tudor on Nov 24, 2019, 23:36
Commented on Module.mk line 83
Which branch is sdvxhook2 on?
In GitLab by @xyen on Nov 24, 2019, 23:44
Commented on Module.mk line 83
None, this needs to go upstream first.
In GitLab by @xyen on Nov 26, 2019, 06:32
added 1 commit
e45b0927- camhook: fixup some minor formatting and enable iidxhook8-config-cam-testCompare with previous version
In GitLab by @xyen on Nov 26, 2019, 06:33
resolved all threads
In GitLab by @xyen on Nov 26, 2019, 21:56
merged
In GitLab by @xyen on Nov 26, 2019, 21:57
ok given by icex2, merged.