Adds monitor rotation and adapter select override.
Adapter select can be back-ported to other games if required, but SDVX appears to be the primary game where local matching (and hence adapter override) is useful.
In GitLab by @xyen on Jan 28, 2020, 10:18
_Merges sdvx_adapter_rotate -> master_
Adds monitor rotation and adapter select override.
Adapter select can be back-ported to other games if required, but SDVX appears to be the primary game where local matching (and hence adapter override) is useful.
In GitLab by @xyen on Jan 28, 2020, 10:23
added 1 commit
<ul><li>78374492 - d3d9exhook/sdvxhook2: add option to force monitor orientation</li></ul>
[Compare with previous version](/djhackers/bemanitools/merge_requests/24/diffs?diff_id=1144&start_sha=49b720f5e2da2f48142edf7aa88ec7f5a8103680)
In GitLab by @xyen on Jan 28, 2020, 10:25
added 1 commit
<ul><li>bcbd5178 - formatting update</li></ul>
[Compare with previous version](/djhackers/bemanitools/merge_requests/24/diffs?diff_id=1145&start_sha=78374492a11488ab8d3104b59054e02f356524d5)
I feel like this bool variable to indicate this feature being active adds more surface for error and some minor complexity than just using the adapter_address == NULL for checks. I would consider the adapter_adress == NULL the simpler and prefered solution.
I feel like this bool variable to indicate this feature being active adds more surface for error and some minor complexity than just using the adapter_address == NULL for checks. I would consider the adapter_adress == NULL the simpler and prefered solution.
Make this a log_assert at the beginning of the function instead of a silent fail and return. Since this is an internal API, setting the parameter to NULL is rather a programming error that should surface and be fixed.
Make this a log_assert at the beginning of the function instead of a silent fail and return. Since this is an internal API, setting the parameter to NULL is rather a programming error that should surface and be fixed.
Nit: Slightly unspecific to what this actually tries to achieve. You match the network adapter and what's it going to do with that? Another sentence or two to clarify this might be good.
Nit: Slightly unspecific to what this actually tries to achieve. You match the network adapter and what's it going to do with that? Another sentence or two to clarify this might be good.
I would add some backup log_asserts and check the values (and ranges) of the parameters to have safe pre-conditions for using these in this module. Furthermore, this makes sure to reveal such programming errors before code is working with invalid values or requires a lot of if-checks to avoid that.
I would add some backup log_asserts and check the values (and ranges) of the parameters to have safe pre-conditions for using these in this module. Furthermore, this makes sure to reveal such programming errors before code is working with invalid values or requires a lot of if-checks to avoid that.
Style: This formatting is not good. Breaking the statement in the if to two lines like that makes this difficult to read.
The following int32_t delta = part needs a preceding line break to set it apart from the previous if-header.
Style: This formatting is not good. Breaking the statement in the if to two lines like that makes this difficult to read.
The following `int32_t delta =` part needs a preceding line break to set it apart from the previous if-header.
Another rather minor but probably relevant thing if it comes to history and keeping track of things: It would be great to know what games this got tested with. I don't ask you to run a full integration test on all games. But it would be good to know if any issues with any game come up that we know immediately if this was even tested or not.
Another rather minor but probably relevant thing if it comes to history and keeping track of things: It would be great to know what games this got tested with. I don't ask you to run a full integration test on all games. But it would be good to know if any issues with any game come up that we know immediately if this was even tested or not.
NULL can be passed here and is expected in cases where the parameter is missing, or a deref-ed NULL is expected when it's an empty string ("").
Returning when it's empty is expected, as this just means one hasn't been passed in
In GitLab by @xyen on Jan 29, 2020, 05:58
Commented on [src/main/hooklib/adapter.c line 127](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..991f776c2a381c5d06352cf09fb4594828439c63#diff-880217526f58a81b305254d60bd009f0R127)
NULL can be passed here and is expected in cases where the parameter is missing, or a deref-ed NULL is expected when it's an empty string ("").
Returning when it's empty is expected, as this just means one hasn't been passed in
use_address_override is set to false by default, and this ensures that the override part of the hook is only used by game hooks that enable it (only sdvxhook2 at present). The adapter hook is used by several other game hooks at present, and I wanted to avoid any potential breakage.
In GitLab by @xyen on Jan 29, 2020, 06:00
Commented on [src/main/hooklib/adapter.c line 125](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..991f776c2a381c5d06352cf09fb4594828439c63#diff-880217526f58a81b305254d60bd009f0R125)
`use_address_override` is set to false by default, and this ensures that the override part of the hook is only used by game hooks that enable it (only sdvxhook2 at present). The adapter hook is used by several other game hooks at present, and I wanted to avoid any potential breakage.
In GitLab by @xyen on Jan 29, 2020, 06:00
Commented on [src/main/hooklib/adapter.h line 7](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..bcbd5178d1bc67f1b54981e39cbea515a03843c8#diff-46a0b7fcd6785de0c58f037aeacd59feR7)
Agreed, will update.
degrees of rotation, yeah, should be implicitly clockwise, technically the spec doesn't specify either.
In GitLab by @xyen on Jan 29, 2020, 06:00
Commented on [dist/sdvx5/sdvxhook2.conf line 28](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..991f776c2a381c5d06352cf09fb4594828439c63#diff-508abdbaee1a5f897c3fe1d4a9cf1543R28)
degrees of rotation, yeah, should be implicitly clockwise, technically the spec doesn't specify either.
In GitLab by @xyen on Jan 29, 2020, 06:01
Commented on [src/main/d3d9exhook/config-gfx.c line 150](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..991f776c2a381c5d06352cf09fb4594828439c63#diff-37c79e11d6d0e932cce872441ec02504R150)
see below.
I'd prefer to check the values right before usage, that way no possible invalid values can ever get applied to the actual usage.
In GitLab by @xyen on Jan 29, 2020, 06:02
Commented on [src/main/d3d9exhook/d3d9ex.c line 328](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..991f776c2a381c5d06352cf09fb4594828439c63#diff-88be4261cef25befc4653e41ac224407R328)
I'd prefer to check the values right before usage, that way no possible invalid values can ever get applied to the actual usage.
that's been there for a while, forgot to remove I guess.
In GitLab by @xyen on Jan 29, 2020, 06:07
Commented on [src/main/d3d9exhook/d3d9ex.c line 223](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..bcbd5178d1bc67f1b54981e39cbea515a03843c8#diff-88be4261cef25befc4653e41ac224407L223)
that's been there for a while, forgot to remove I guess.
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on [src/main/hooklib/adapter.h line 7](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..bcbd5178d1bc67f1b54981e39cbea515a03843c8#diff-46a0b7fcd6785de0c58f037aeacd59feR7)
changed this line in [version 4 of the diff](/djhackers/bemanitools/merge_requests/24/diffs?diff_id=1147&start_sha=bcbd5178d1bc67f1b54981e39cbea515a03843c8#b1512fad44200cc622c6bb2917f9aba0a068f38f_7_11)
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on [src/main/d3d9exhook/d3d9ex.c line 244](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..bcbd5178d1bc67f1b54981e39cbea515a03843c8#diff-88be4261cef25befc4653e41ac224407R244)
changed this line in [version 4 of the diff](/djhackers/bemanitools/merge_requests/24/diffs?diff_id=1147&start_sha=bcbd5178d1bc67f1b54981e39cbea515a03843c8#635ef12b21ec4a077c76251b15c5580dd5d42f19_244_246)
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on [src/main/d3d9exhook/d3d9ex.c line 223](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..bcbd5178d1bc67f1b54981e39cbea515a03843c8#diff-88be4261cef25befc4653e41ac224407L223)
changed this line in [version 4 of the diff](/djhackers/bemanitools/merge_requests/24/diffs?diff_id=1147&start_sha=bcbd5178d1bc67f1b54981e39cbea515a03843c8#635ef12b21ec4a077c76251b15c5580dd5d42f19_259_271)
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on [src/main/d3d9exhook/d3d9ex.c line 231](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..bcbd5178d1bc67f1b54981e39cbea515a03843c8#diff-88be4261cef25befc4653e41ac224407R231)
changed this line in [version 4 of the diff](/djhackers/bemanitools/merge_requests/24/diffs?diff_id=1147&start_sha=bcbd5178d1bc67f1b54981e39cbea515a03843c8#635ef12b21ec4a077c76251b15c5580dd5d42f19_231_231)
In GitLab by @xyen on Jan 29, 2020, 06:28
added 2 commits
<ul><li>742a87e5 - address MR comments.</li><li>e305ee13 - d3d9exhook: actually rotate the right monitor</li></ul>
[Compare with previous version](/djhackers/bemanitools/merge_requests/24/diffs?diff_id=1147&start_sha=bcbd5178d1bc67f1b54981e39cbea515a03843c8)
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on [src/main/d3d9exhook/d3d9ex.c line 230](https://github.com/djhackersdev/bemanitools/compare/c2e202f8b0f38f99d43825391a520a6b666446de..78374492a11488ab8d3104b59054e02f356524d5#diff-88be4261cef25befc4653e41ac224407R230)
updated
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on [src/main/d3d9exhook/d3d9ex.c line 244](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..bcbd5178d1bc67f1b54981e39cbea515a03843c8#diff-88be4261cef25befc4653e41ac224407R244)
updated
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on [src/main/d3d9exhook/d3d9ex.c line 231](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..bcbd5178d1bc67f1b54981e39cbea515a03843c8#diff-88be4261cef25befc4653e41ac224407R231)
spaced out
In GitLab by @xyen on Jan 29, 2020, 06:29
added 1 commit
<ul><li>991f776c - d3d9exhook: actually rotate the right monitor</li></ul>
[Compare with previous version](/djhackers/bemanitools/merge_requests/24/diffs?diff_id=1148&start_sha=e305ee135ea1990724d536baf4ec866fe5bdba96)
In GitLab by @xyen on Jan 29, 2020, 06:29
Commented on [src/main/hooklib/adapter.c line 130](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..991f776c2a381c5d06352cf09fb4594828439c63#diff-880217526f58a81b305254d60bd009f0R130)
updated
In GitLab by @xyen on Jan 29, 2020, 06:30
Commented on [src/main/d3d9exhook/d3d9ex.c line 237](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..991f776c2a381c5d06352cf09fb4594828439c63#diff-88be4261cef25befc4653e41ac224407R237)
updated
Since ‘adapter_address’ set to NULL doesn’t have a use and you even check and filter that, I don’t see the benefit for having another boolean flag to use as a feature switch. I think you can use the NULL state of the ‘adapter_address’ variable as the feature state which blends in nicely when disabled via the configuration file.
Since ‘adapter_address’ set to NULL doesn’t have a use and you even check and filter that, I don’t see the benefit for having another boolean flag to use as a feature switch. I think you can use the NULL state of the ‘adapter_address’ variable as the feature state which blends in nicely when disabled via the configuration file.
adapter_address is passed in as an input, it isn't the feature flag.
override_address (where the value gets copied) isn't a pointer either, it's is an array of 16 chars that ALWAYS exists.
I purposely do NOT assign the pointer as input, as that would require taking ownership of the memory, instead I copy it after verifying the size etc.
The reason we have the flag, is because calling adapter_hook_override() is optional, only sdvxhook2 does it right now.
In GitLab by @xyen on Jan 29, 2020, 07:36
Commented on [src/main/hooklib/adapter.c line 125](https://github.com/djhackersdev/bemanitools/compare/a5fdecef6156f9ff1863029de73ffa444d9c64f2..991f776c2a381c5d06352cf09fb4594828439c63#diff-880217526f58a81b305254d60bd009f0R125)
`adapter_address` is passed in as an input, it isn't the feature flag.
override_address (where the value gets copied) isn't a pointer either, it's is an array of 16 chars that ALWAYS exists.
I purposely do NOT assign the pointer as input, as that would require taking ownership of the memory, instead I copy it after verifying the size etc.
The reason we have the flag, is because calling `adapter_hook_override()` is optional, only sdvxhook2 does it right now.
Thanks for the explanation. Now I realized that IP_ADDRESS_STRING must be fixed size array. I mistook that for a dynamic thing. Then everything's fine like that.
Thanks for the explanation. Now I realized that `IP_ADDRESS_STRING` must be fixed size array. I mistook that for a dynamic thing. Then everything's fine like that.
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 Jan 28, 2020, 10:18
Merges sdvx_adapter_rotate -> master
Adds monitor rotation and adapter select override.
Adapter select can be back-ported to other games if required, but SDVX appears to be the primary game where local matching (and hence adapter override) is useful.
In GitLab by @xyen on Jan 28, 2020, 10:23
added 1 commit
78374492- d3d9exhook/sdvxhook2: add option to force monitor orientationCompare with previous version
In GitLab by @xyen on Jan 28, 2020, 10:25
added 1 commit
bcbd5178- formatting updateCompare with previous version
I feel like this bool variable to indicate this feature being active adds more surface for error and some minor complexity than just using the adapter_address == NULL for checks. I would consider the adapter_adress == NULL the simpler and prefered solution.
Make this a log_assert at the beginning of the function instead of a silent fail and return. Since this is an internal API, setting the parameter to NULL is rather a programming error that should surface and be fixed.
In contrast, that's a good check to make it fail, log a warning/error and return because an invalid formated string can be set from a configuration.
Nit: Slightly unspecific to what this actually tries to achieve. You match the network adapter and what's it going to do with that? Another sentence or two to clarify this might be good.
Nit: I assume the values 0, 90, 180 and 270 refer to turning your monitor clockwise?
You might want to add another range check for -1 and [0-3] for the value here to catch the error early on and fall back to a default value.
I would add some backup log_asserts and check the values (and ranges) of the parameters to have safe pre-conditions for using these in this module. Furthermore, this makes sure to reveal such programming errors before code is working with invalid values or requires a lot of if-checks to avoid that.
Style nit: Doesn't match the style of the overall code in this module (copy-paste?).
EnumDisplaySettings(NULL, ENUM_CURRENT_SETTINGS, &dm) != 0Style nit: Replace with snake_case and no type pre-fixes -> ret
Did you forget to remove this or this something missing here?
Style: This formatting is not good. Breaking the statement in the if to two lines like that makes this difficult to read.
The following
int32_t delta =part needs a preceding line break to set it apart from the previous if-header.Nit style: Please have line breaks before and after control blocks.
Nit style: Please have line breaks before and after control blocks.
Another rather minor but probably relevant thing if it comes to history and keeping track of things: It would be great to know what games this got tested with. I don't ask you to run a full integration test on all games. But it would be good to know if any issues with any game come up that we know immediately if this was even tested or not.
In GitLab by @xyen on Jan 29, 2020, 05:58
Commented on src/main/hooklib/adapter.c line 127
NULL can be passed here and is expected in cases where the parameter is missing, or a deref-ed NULL is expected when it's an empty string ("").
Returning when it's empty is expected, as this just means one hasn't been passed in
In GitLab by @xyen on Jan 29, 2020, 06:00
Commented on src/main/hooklib/adapter.c line 125
use_address_overrideis set to false by default, and this ensures that the override part of the hook is only used by game hooks that enable it (only sdvxhook2 at present). The adapter hook is used by several other game hooks at present, and I wanted to avoid any potential breakage.In GitLab by @xyen on Jan 29, 2020, 06:00
Commented on src/main/hooklib/adapter.h line 7
Agreed, will update.
In GitLab by @xyen on Jan 29, 2020, 06:00
Commented on dist/sdvx5/sdvxhook2.conf line 28
degrees of rotation, yeah, should be implicitly clockwise, technically the spec doesn't specify either.
In GitLab by @xyen on Jan 29, 2020, 06:01
Commented on src/main/d3d9exhook/config-gfx.c line 150
see below.
In GitLab by @xyen on Jan 29, 2020, 06:02
Commented on src/main/d3d9exhook/d3d9ex.c line 328
I'd prefer to check the values right before usage, that way no possible invalid values can ever get applied to the actual usage.
In GitLab by @xyen on Jan 29, 2020, 06:07
Commented on src/main/d3d9exhook/d3d9ex.c line 223
that's been there for a while, forgot to remove I guess.
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on src/main/hooklib/adapter.h line 7
changed this line in version 4 of the diff
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on src/main/d3d9exhook/d3d9ex.c line 244
changed this line in version 4 of the diff
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on src/main/d3d9exhook/d3d9ex.c line 223
changed this line in version 4 of the diff
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on src/main/d3d9exhook/d3d9ex.c line 231
changed this line in version 4 of the diff
In GitLab by @xyen on Jan 29, 2020, 06:28
added 2 commits
742a87e5- address MR comments.Compare with previous version
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on src/main/d3d9exhook/d3d9ex.c line 230
updated
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on src/main/d3d9exhook/d3d9ex.c line 244
updated
In GitLab by @xyen on Jan 29, 2020, 06:28
Commented on src/main/d3d9exhook/d3d9ex.c line 231
spaced out
In GitLab by @xyen on Jan 29, 2020, 06:29
added 1 commit
991f776c- d3d9exhook: actually rotate the right monitorCompare with previous version
In GitLab by @xyen on Jan 29, 2020, 06:29
Commented on src/main/hooklib/adapter.c line 130
updated
In GitLab by @xyen on Jan 29, 2020, 06:30
Commented on src/main/d3d9exhook/d3d9ex.c line 237
updated
In GitLab by @xyen on Jan 29, 2020, 06:30
Tested with SDVX5, as that's the only game that this override functionality is enabled on.
Seeing my previous comment about the book flag, I just realized that log_assert doesn’t make sense here. Nvm, resolved.
Since ‘adapter_address’ set to NULL doesn’t have a use and you even check and filter that, I don’t see the benefit for having another boolean flag to use as a feature switch. I think you can use the NULL state of the ‘adapter_address’ variable as the feature state which blends in nicely when disabled via the configuration file.
In GitLab by @xyen on Jan 29, 2020, 07:36
Commented on src/main/hooklib/adapter.c line 125
adapter_addressis passed in as an input, it isn't the feature flag.override_address (where the value gets copied) isn't a pointer either, it's is an array of 16 chars that ALWAYS exists.
I purposely do NOT assign the pointer as input, as that would require taking ownership of the memory, instead I copy it after verifying the size etc.
The reason we have the flag, is because calling
adapter_hook_override()is optional, only sdvxhook2 does it right now.Thanks for the explanation. Now I realized that
IP_ADDRESS_STRINGmust be fixed size array. I mistook that for a dynamic thing. Then everything's fine like that.resolved all threads
In GitLab by @xyen on Jan 30, 2020, 10:20
merged