Add support IIDX tricoro CN (狂热节拍 IIDX 2) - [merged] #218

Closed icex2 closed 1 year ago

In GitLab by @garbage on May 24, 2022, 21:10

Merges feature/support-iidx20-cn -> master

Summary

Support IIDX tricoro CN (狂热节拍 IIDX 2)

Description

Add support IIDX tricoro CN (狂热节拍 IIDX 2)

This is a strange game.

The name is tricoro but on a Lincle system, the ID is JDZ, and it is an exe.

This game does not have NETWORK function

Also, the CARD READER is not checked, so it is not included in the hooks.

How Has This Been Tested?

By launching and playing the game.

Checklist

[ ] Implemented (unit) test(s) which prove that the introduced changes are working as expected.
Tested with the following games:
- [ ] ...
- [ ] ...
[ ] Followed the developer (style) guidelines.
[ ] Updated existing doc of or add new doc to README file(s).
[ ] Updated development documentation.

In GitLab by @garbage on May 24, 2022, 21:27

added 1 commit

239fee87 - iidxhook5-cn: init

Compare with previous version

Hey, thank you for your contribution to the project. That's definintely an odd one but I am happy you took the time to implement support for it in BT5.

I see that the title of the MR says "Draft". I just wanted to check with you regarding reviewing this: Do you want to get feedback already in the draft stage or do you want us to wait until you don't consider it a draft anymore?

In GitLab by @garbage on May 26, 2022, 19:33

Commented on src/main/iidxhook5-cn/dllmain.c line 94

OpenProcess is basically used for hooks in exe games, but since it does not exist in this game, RegisterClassA is used instead.

In GitLab by @garbage on May 26, 2022, 19:45

marked this merge request as ready

In GitLab by @garbage on May 26, 2022, 19:47

I had it set to Draft because I was checking the operation, but I have now removed Draft. Please review.

Thanks for explaining. I think that's a very good to know detail that should be part of the code.

// OpenProcess is basically used for hooks in exe games, 
// but since it does not exist in this game, RegisterClassA is used instead.
static ATOM WINAPI my_RegisterClassA(const WNDCLASSA *lpWndClass)

I don't see this config parameter being used in dllmain.c of your hook. Did you forgot to use it or is this not relevant to the game? If the latter, I suggest to remove it from the reference config.

This looks odd: So the game is using a black round plug for game license management instead of a black usb dongle? I suppose the white dongle hooks were left out because the game does not make use of them and doesn't have any eamuse functionality? That might be worth pointing out in a comment here since this is a major difference from all the other games we are supporting so far (and consider the "defaults").

This game does not have NETWORK function
Also, the CARD READER is not checked, so it is not included in the hooks.

Just saw these in your MR description. I suggest adding something along the following lines. Feel free to re-phrase or add some more info that you consider relevant

+    ezusb_iidx_emu_node_security_plug_set_pcbid(&config_eamuse.pcbid);
+    // No white dongle hooks applies since the game does not have network functionality
+    // Also, card readers are not used/checked; no card reader hooks required

In GitLab by @garbage on May 28, 2022, 16:45

Commented on dist/iidx/iidxhook-20-cn.conf line 5

I certainly didn't use it.

Just need to have a little discussion about this parameter.

I checked and it seems that iidxhook1 and iidxhook2 use this parameter, but iidxhook3 and later do not.

However, the conf files iidxhook-09.conf through iidxhook-26.conf all seem to have this value.

If this is correct, I will need to review the other files as well.

In GitLab by @garbage on May 28, 2022, 16:47

added 1 commit

1f27f027 - iidxhook5-cn: add comments

Compare with previous version

In GitLab by @garbage on May 28, 2022, 16:51

Commented on src/main/iidxhook5-cn/dllmain.c line 142

changed this line in version 4 of the diff

In GitLab by @garbage on May 28, 2022, 16:51

added 1 commit

1ec161f1 - iidxhook5-cn: add comments

Compare with previous version

In GitLab by @garbage on May 28, 2022, 16:58

added 1 commit

45df77ab - iidxhook5-cn: edit comments

Compare with previous version

In GitLab by @garbage on May 28, 2022, 17:01

Commented on src/main/iidxhook5-cn/dllmain.c line 149

Please excuse my lack of explanation.

I have added some comments.

You are right about that. As it doesn't have any negative impact other than it might not work/be a useful feature, I suggest we keep it aligned to avoid inconsistencies that might raise the question "Why are we missing it here" later on. This can be cleaned up later as well. Let's consider it out of scope for this contribution.

resolved all threads

lgtm. As this is a slightly greater change and to be more certain I didn't miss anything major, I am awaiting another review by @xyen before merging this. Just FYI that it might take a few more days for him to take a look at this.

added 45 commits

45df77ab...da34e089 - 41 commits from branch djhackers:master
4064fcaf - iidxhook5-cn: init
276e107f - iidxhook5-cn: add comments
fa51f700 - iidxhook5-cn: add comments
0b99ef95 - iidxhook5-cn: edit comments

Compare with previous version

approved this merge request

Sorry for the late action. Forgot about this due to the other contributions and thought i was still awaiting further feedback on this one. Merging now. Thanks again for your effort.

djhackersdev / bemanitools