sdvxhook2: Add sdvxhook2 - [merged] #113

Closed icex2 closed 1 year ago

In GitLab by @xyen on Nov 26, 2019, 22:02

Merges sdvxhook2 -> master

Pretty self-explanatory.

In GitLab by @tudor on Nov 26, 2019, 22:03

Commented on dist/sdvx5/gamestart.bat line 16

sdvxhook2? or sdvxhook5?

In GitLab by @xyen on Nov 26, 2019, 22:04

Commented on dist/sdvx5/gamestart.bat line 16

changed this line in version 2 of the diff

In GitLab by @xyen on Nov 26, 2019, 22:04

added 1 commit

e42de4fa - sdvxhoook2: typo in gamestart

Compare with previous version

In GitLab by @xyen on Nov 26, 2019, 22:04

resolved all threads

Nit: Copy/paste leftover: IIDX ACIO bus

Include order: System includes with <...> before project/local includes with "..."

Nit: Empty line before control block

Nit: leading underscore to function name?

Style: Make variable names of struct lower-case

Is this really a thing we need for sdvxhook2 as well? Hard to believe considering this was required with IIDX 16 initially.

Nit: Missing newline after control block

My previous d3d9 hook module refactoring for IIDX also changed the main interfaces to init and configure the module which made the whole module more streamlined imo. Suggestions:

Introduce a configuration structure that takes parameters to configure the module instead of having multiple function calls (see iidxhook-util/d3d9.h)
Have one function to init a configuration with default values and one to apply one to the module.

"Disable BIO2 emulation and enable usage of real BIO2 hardware on COM4"

Some general notes:

Proper namespacing of functions in modules is sometimes missing, e.g. acio_XXX -> sdvxhook2_acio_XXX. I assume because these were copy-pasted from existing infrastructure and adapted to work with sdvxhook2 which is fine. For new code, we should stick to some guidelines to establish a consistent style over time.
Add a sdvxhook2.md readme file similar to iidxhookX.md readme files
Can you add tests for the various modules? Especially tests to verify the bio2 and acio modules are processing messages correctly would be good to ensure any changes/refactorings can be verified without having to boot the games.

In GitLab by @xyen on Nov 30, 2019, 15:09

Commented on src/main/sdvxhook2/config-io.c line 29

This actually isn't the case, it enables usage of real BIO2 hardware on whatever the port that the BIO2 declares it self as. I just happened to pick COM4 as the default.