pop'n music 15 - 18 implemented - [merged] #219

Closed
opened 2022-05-30 04:46:56 +03:00 by icex2 · 48 comments
icex2 commented 2022-05-30 04:46:56 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on May 30, 2022, 03:46

Merges popn_support -> master

Summary

Adds support for pop'n music 15 to 18 in bt5.

Description

  • popnhook1 for pop'n 15 - 18 has been added
  • popnio has been added
  • inject.exe has new syntax for loading hook DLLs, real.dll=hook.dll. This will trigger an early IAT hook where it will load the EXE suspended without resolving imports, replace the reference to real.dll in the import table with hook.dll, and then resolve everything before launching. This allows for ezusb.dll to be hooked properly.
  • launcher.exe also has a new early IAT hook feature now. Use -I real.dll=hook.dll. The idea is the same as described above for inject.exe.
  • Updated ezusb constant namings based on what is visible in ezusb.dll's debug statements.

The launcher.exe implementation of early IAT hooking means that someone can implement popnhook2.dll for 19 and above. I have tried pop'n music Sunny Park using a modified version of popnhook1 and it seems to work to some degree: the I/O check and security check returns OK which means the ezusb hooking used in popnhook1 is also working for the later games using launcher.exe -I ezusb.dll=ezusb2-popn-shim.dll .... The process is rather invasive (manually resolving all imports means more chances to fail) so it has been implemented in such a way that the launcher will work the same as it has before as long as -I isn't specified.

One questionable thing I am not confident about is the texture_usage_fix hack flag I added in the conf. As the comment says, pop'n music 16 will work in Windows XP without the flag being set, but the game will immediately crash on later OSes without the flag being set in my experience. No other games had this issue in my experience. Enabling it in other games doesn't seem to have any negative effects.

Related Issue

https://dev.s-ul.net/djhackers/bemanitools/-/issues/28

How Has This Been Tested?

By launching and playing pop'n music 15, 16, 17, and 18.

Offline as well as network + card in has been tested for all games on Windows XP, Windows 10, and Windows 11.

Checklist

  • Implemented (unit) test(s) which prove that the introduced changes are working as expected.
  • Tested with the following games:
    • ... pop'n music 15
    • ... pop'n music 16
    • ... pop'n music 17
    • ... pop'n music 18
  • Followed the developer (style) guidelines.
  • Updated existing doc of or add new doc to README file(s).
  • Updated development documentation.
In GitLab by @33c17f40 on May 30, 2022, 03:46 _Merges popn_support -> master_ ## Summary <!--- Provide a general summary of your changes in the Title above --> Adds support for pop'n music 15 to 18 in bt5. ## Description <!--- Describe your changes in detail --> - popnhook1 for pop'n 15 - 18 has been added - popnio has been added - inject.exe has new syntax for loading hook DLLs, `real.dll=hook.dll`. This will trigger an early IAT hook where it will load the EXE suspended without resolving imports, replace the reference to real.dll in the import table with hook.dll, and then resolve everything before launching. This allows for ezusb.dll to be hooked properly. - launcher.exe also has a new early IAT hook feature now. Use `-I real.dll=hook.dll`. The idea is the same as described above for inject.exe. - Updated ezusb constant namings based on what is visible in ezusb.dll's debug statements. The launcher.exe implementation of early IAT hooking means that someone can implement popnhook2.dll for 19 and above. I have tried pop'n music Sunny Park using a modified version of popnhook1 and it seems to work to some degree: the I/O check and security check returns OK which means the ezusb hooking used in popnhook1 is also working for the later games using `launcher.exe -I ezusb.dll=ezusb2-popn-shim.dll ...`. The process is rather invasive (manually resolving all imports means more chances to fail) so it has been implemented in such a way that the launcher will work the same as it has before as long as `-I` isn't specified. One questionable thing I am not confident about is the `texture_usage_fix` hack flag I added in the conf. As the comment says, pop'n music 16 will work in Windows XP without the flag being set, but the game will immediately crash on later OSes without the flag being set in my experience. No other games had this issue in my experience. Enabling it in other games doesn't seem to have any negative effects. ## Related Issue <!--- This project only accepts pull requests related to open issues --> <!--- If suggesting a new feature or change, please discuss it in an issue first --> <!--- If fixing a bug, there should be an issue describing it with steps to reproduce --> <!--- Please link to the issue here: --> https://dev.s-ul.net/djhackers/bemanitools/-/issues/28 ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> By launching and playing pop'n music 15, 16, 17, and 18. Offline as well as network + card in has been tested for all games on Windows XP, Windows 10, and Windows 11. ## Checklist <!-- Make sure you covered all items, which apply, of the checklist below. --> <!-- Strikethrough items that do not apply and provide a brief description why. --> * [ ] Implemented (unit) test(s) which prove that the introduced changes are working as expected. * Tested with the following games: * [x] ... pop'n music 15 * [x] ... pop'n music 16 * [x] ... pop'n music 17 * [x] ... pop'n music 18 * [x] Followed the developer (style) guidelines. * [x] Updated existing doc of or add new doc to README file(s). * [x] Updated development documentation.
icex2 commented 2022-05-30 04:55:28 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on May 30, 2022, 03:55

The ezusb changes could affect IIDX so if someone could please test IIDX games (any of the roundplug games) to make sure they still work properly, it would be appreciated. I gave some games a quick test while working on this and it still seemed to work but I couldn't test every roundplug IIDX game.

In GitLab by @33c17f40 on May 30, 2022, 03:55 The ezusb changes could affect IIDX so if someone could please test IIDX games (any of the roundplug games) to make sure they still work properly, it would be appreciated. I gave some games a quick test while working on this and it still seemed to work but I couldn't test every roundplug IIDX game.
icex2 commented 2022-05-30 04:59:19 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on May 30, 2022, 03:59

added 7 commits

  • abf40d02...ccfe6f79 - 6 commits from branch djhackers:master
  • d8f5828d - pop'n music 15 - 18 implemented

Compare with previous version

In GitLab by @33c17f40 on May 30, 2022, 03:59 added 7 commits <ul><li>abf40d02...ccfe6f79 - 6 commits from branch <code>djhackers:master</code></li><li>d8f5828d - pop&#39;n music 15 - 18 implemented</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2553&start_sha=abf40d02d78ba45036746188cb982be098a11ccc)
icex2 commented 2022-05-30 14:08:30 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on May 30, 2022, 13:08

added 1 commit

  • 0fa3bf10 - Remove old test code

Compare with previous version

In GitLab by @33c17f40 on May 30, 2022, 13:08 added 1 commit <ul><li>0fa3bf10 - Remove old test code</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2557&start_sha=d8f5828d6ec48b4cc4dc4f84d52635351a4adbce)
icex2 commented 2022-05-30 14:24:26 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on May 30, 2022, 13:24

added 1 commit

  • fe0beded - Small cleanup

Compare with previous version

In GitLab by @33c17f40 on May 30, 2022, 13:24 added 1 commit <ul><li>fe0beded - Small cleanup</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2559&start_sha=0fa3bf100574cb2dd7abc6a88e6e2bdd204da92a)
icex2 commented 2022-06-05 19:26:37 +03:00 (Migrated from github.com)

Wouldn't ezusb-popn-shim.dll be sufficient here? Is the 2 in the name denoting anything specific?

Wouldn't `ezusb-popn-shim.dll` be sufficient here? Is the `2` in the name denoting anything specific?
icex2 commented 2022-06-05 19:28:23 +03:00 (Migrated from github.com)

I am wondering if we need to expose this as a configurable setting. If you always apply the fix, does this break anything on the other games and under Windows XP? If not, I would prefer turning this into a hardcoded solution and have less configurable options exposed.

I am wondering if we need to expose this as a configurable setting. If you always apply the fix, does this break anything on the other games and under Windows XP? If not, I would prefer turning this into a hardcoded solution and have less configurable options exposed.
icex2 commented 2022-06-05 19:31:07 +03:00 (Migrated from github.com)

I would remove this line as you assume that one hook will likely cover all games. Not saying it cannot, but we have seen unpleasent surprises in the past and had to split into more hook DLLs than we wanted. Can be added once the hook is available.

I would remove this line as you assume that one hook will likely cover all games. Not saying it cannot, but we have seen unpleasent surprises in the past and had to split into more hook DLLs than we wanted. Can be added once the hook is available. ```suggestion:-0+0 ```
icex2 commented 2022-06-05 19:37:45 +03:00 (Migrated from github.com)

What about lights?

What about lights?
icex2 commented 2022-06-05 19:38:45 +03:00 (Migrated from github.com)

Dongle 1-5? Wondering how you figured that out.

Dongle 1-5? Wondering how you figured that out.
icex2 commented 2022-06-05 19:43:20 +03:00 (Migrated from github.com)

Hm, there are a lot of iidx references in the types still in the code. I get the idea that they apparently shared a lot of functionality and, with being the same, re-using code makes sense. However, it's somewhat odd to read now and it introduces a weird semantic dependency to the iidx stuff. Not sure what's the best approach here, but I would call "when in doubt, copy-paste first, consolidate leter when everything cleared up?".

What do you think?

Hm, there are a lot of `iidx` references in the types still in the code. I get the idea that they apparently shared a lot of functionality and, with being the same, re-using code makes sense. However, it's somewhat odd to read now and it introduces a weird semantic dependency to the iidx stuff. Not sure what's the best approach here, but I would call "when in doubt, copy-paste first, consolidate leter when everything cleared up?". What do you think?
icex2 commented 2022-06-05 19:45:05 +03:00 (Migrated from github.com)

Nvm, I think it makes sense to keep this as it denotes BT5's semantics of ezusb(1) vs. ezusb2. Forgot popn uses the ezusb (FX) 2 board.

Nvm, I think it makes sense to keep this as it denotes BT5's semantics of ezusb(1) vs. ezusb2. Forgot popn uses the ezusb (FX) 2 board.
icex2 commented 2022-06-05 19:46:25 +03:00 (Migrated from github.com)

@tau might be something to upstream into mainline capnhook?

@tau might be something to upstream into mainline capnhook?
icex2 commented 2022-06-05 19:50:40 +03:00 (Migrated from github.com)

Another indicator that iidx stuff leaks into popn and creates some unpleasent coupling. To keep things simple, I would favor copy-paste the stuff you need for popn for now into its own module/namespace. Consolidation, if necessary and a good idea, can follow later.

Another indicator that iidx stuff leaks into popn and creates some unpleasent coupling. To keep things simple, I would favor copy-paste the stuff you need for popn for now into its own module/namespace. Consolidation, if necessary and a good idea, can follow later.
icex2 commented 2022-06-05 19:54:20 +03:00 (Migrated from github.com)

We have these documented in Modules.mk at the top. Might want to move that to a proper readme file in docs overall and add the ones you have here.

We have these documented in `Modules.mk` at the top. Might want to move that to a proper readme file in `docs` overall and add the ones you have here.
icex2 commented 2022-06-05 19:55:13 +03:00 (Migrated from github.com)
* [pop'n music](doc/popnhook/README.md)
```suggestion:-0+0 * [pop'n music](doc/popnhook/README.md) ```
icex2 commented 2022-06-06 02:04:53 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:04

added 1 commit

  • 0afcd743 - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

In GitLab by @33c17f40 on Jun 6, 2022, 01:04 added 1 commit <ul><li>0afcd743 - Apply 1 suggestion(s) to 1 file(s)</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2572&start_sha=fe0beded1767aa932edbc992cc5f04849953d277)
icex2 commented 2022-06-06 02:04:53 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:04

Commented on README.md line 70

changed this line in version 5 of the diff

In GitLab by @33c17f40 on Jun 6, 2022, 01:04 Commented on [README.md line 70](https://github.com/djhackersdev/bemanitools/compare/ccfe6f7923bd94552d23a1384ec194c3aa444d0e..fe0beded1767aa932edbc992cc5f04849953d277#diff-04c6e90faac2675aa89e2176d2eec7d8R70) changed this line in [version 5 of the diff](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2572&start_sha=fe0beded1767aa932edbc992cc5f04849953d277#8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_70_70)
icex2 commented 2022-06-06 02:12:26 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:12

Commented on dist/popn/gamestart-15.bat line 15

Yes, that's exactly the reasoning I chose for the name ezusb2 instead of ezusb.

In GitLab by @33c17f40 on Jun 6, 2022, 01:12 Commented on [dist/popn/gamestart-15.bat line 15](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..d08f341fd85f2a701080692f36802a05ed99e32b#diff-bae5352a731c5eb1fbc736395d7962a0R15) Yes, that's exactly the reasoning I chose for the name `ezusb2` instead of `ezusb`.
icex2 commented 2022-06-06 02:15:20 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:15

Commented on dist/popn/popnhook-16.conf line 23

None of the other games need it so I'm ok with making it a hardcoded solution instead, but I do not want it enabled for all games unless people find that the other games don't work without it (in my testing on WinXP, Win10, and Win11 that wasn't the case). At best it does nothing and at worst it could have unintended side effects. I think it should be easy to conditionally enable it based on the mcode though so that's not a big deal.

In GitLab by @33c17f40 on Jun 6, 2022, 01:15 Commented on [dist/popn/popnhook-16.conf line 23](https://github.com/djhackersdev/bemanitools/compare/ccfe6f7923bd94552d23a1384ec194c3aa444d0e..e77fe10836aea7fbced77852a46baffee0b65029#diff-b192e535a3af75594bf062fb441fd510R23) None of the other games need it so I'm ok with making it a hardcoded solution instead, but I do not want it enabled for all games unless people find that the other games don't work without it (in my testing on WinXP, Win10, and Win11 that wasn't the case). At best it does nothing and at worst it could have unintended side effects. I think it should be easy to conditionally enable it based on the mcode though so that's not a big deal.
icex2 commented 2022-06-06 02:17:04 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:17

Commented on doc/popnhook/README.md line 19

changed this line in version 6 of the diff

In GitLab by @33c17f40 on Jun 6, 2022, 01:17 Commented on [doc/popnhook/README.md line 19](https://github.com/djhackersdev/bemanitools/compare/ccfe6f7923bd94552d23a1384ec194c3aa444d0e..0afcd743ed837617d6319c44803bafde523e005c#diff-57d3a808202529b3aea14c4d426b0a92R19) changed this line in [version 6 of the diff](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2574&start_sha=0afcd743ed837617d6319c44803bafde523e005c#1fbdce90255640a79ab854abedec36d9c6a3439d_19_19)
icex2 commented 2022-06-06 02:17:05 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:17

added 1 commit

  • aaee2f16 - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

In GitLab by @33c17f40 on Jun 6, 2022, 01:17 added 1 commit <ul><li>aaee2f16 - Apply 1 suggestion(s) to 1 file(s)</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2574&start_sha=0afcd743ed837617d6319c44803bafde523e005c)
icex2 commented 2022-06-06 02:37:04 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:37

Commented on src/main/ezusb-iidx/secplug-cmd.h line 36

That's just how the code works out in ezusb.dll.

Using pop'n 16's ezusb.dll for a concrete example. There is a large message handler at 0x1000C0E0. At the top of the functions it calls you'll see stuff like (using IDA's decompilation as an example, because it's faster)

  *(*(v4 + 28) + 2) = 1; // node ID, 1 = EZUSB_IIDX_MSG_NODE_SECURITY_PLUG
  *(*(v4 + 28) + 3) = 3; // command ID, 3 = EZUSB_IIDX_SECPLUG_CMD_V2_WRITE_DATA

That's an easy way to know exactly what node and command you're looking at out of the message handlers. From there, you can usually find debug messages/errors. For example, node ID 1 command ID 3's error message says "[Security] ライトエラー" ("[Security] Write error"). So you know it's the write command. Similarly, node ID 1 command ID 2's error message says "[Security] リードエラー" ("[Security] Read error").

This part of the message handler corresponds to the 5 dongle messages.

  if ( this[6] > 11 )
  {
    *(this[7] + 88) = result;
    *(this[7] + 92) = sub_10007C50;
  }
  if ( this[6] > 12 )
  {
    *(this[7] + 96) = result;
    *(this[7] + 100) = sub_10007C70;
  }
  if ( this[6] > 13 )
  {
    *(this[7] + 104) = result;
    *(this[7] + 108) = sub_10007C90;
  }
  if ( this[6] > 14 )
  {
    *(this[7] + 112) = result;
    *(this[7] + 116) = sub_10007CB0;
  }
  if ( this[6] > 15 )
  {
    *(this[7] + 120) = result;
    *(this[7] + 124) = sub_10007CD0;
  }

If you go into each function they look like this:

int __cdecl sub_10007C50(int a1)
{
  return sub_10007A00(a1, *(dword_1003000C + 12));
}

int __cdecl sub_10007C70(int a1)
{
  return sub_10007A00(1, a1, *(dword_1003000C + 12));
}

int __cdecl sub_10007C90(int a1)
{
  return sub_10007A00(2, a1, *(dword_1003000C + 12));
}

int __cdecl sub_10007CB0(int a1)
{
  return sub_10007A00(3, a1, *(dword_1003000C + 12));
}

int __cdecl sub_10007CD0(int a1)
{
  return sub_10007A00(4, a1, *(dword_1003000C + 12));
}

They all are calling the same sub_10007A00 with the values 0-4. The 0-4 value is used to set the node ID/command ID as such:

  switch ( a1 )
  {
    case 0:
      v6 = *(a2 + 12);
      *(*(v6 + 28) + 2) = 1;
      *(*(v6 + 28) + 3) = 7;
      goto LABEL_9;
    case 1:
      v7 = *(a2 + 12);
      *(*(v7 + 28) + 2) = 1;
      *(*(v7 + 28) + 3) = 8;
      goto LABEL_9;
    case 2:
      v8 = *(a2 + 12);
      *(*(v8 + 28) + 2) = 1;
      *(*(v8 + 28) + 3) = 9;
      goto LABEL_9;
    case 3:
      v9 = *(a2 + 12);
      *(*(v9 + 28) + 2) = 1;
      *(*(v9 + 28) + 3) = 10;
      goto LABEL_9;
    case 4:
      v10 = *(a2 + 12);
      *(*(v10 + 28) + 2) = 1;
      *(*(v10 + 28) + 3) = 11;
...

The shared function sub_10007A00 has a debug error message of "[Security] セレクトエラー" ("[Security] Select error") so it's the dongle/whatever selection function.

So it's safe to say that there are 5 dongle commands in total. How they differ exactly is harder to understand because the code is the exact same between all 5 of the commands, except for the command ID being assigned. And the game itself expects a white dongle or black dongle to be returned depending on what slot it reads. IIDX is lenient here but pop'n is picky and will only work with the one specific configuration.

In GitLab by @33c17f40 on Jun 6, 2022, 01:37 Commented on [src/main/ezusb-iidx/secplug-cmd.h line 36](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..d08f341fd85f2a701080692f36802a05ed99e32b#diff-3139137678ffe5004bc6b7d009242d87R36) That's just how the code works out in `ezusb.dll`. Using pop'n 16's ezusb.dll for a concrete example. There is a large message handler at 0x1000C0E0. At the top of the functions it calls you'll see stuff like (using IDA's decompilation as an example, because it's faster) ``` *(*(v4 + 28) + 2) = 1; // node ID, 1 = EZUSB_IIDX_MSG_NODE_SECURITY_PLUG *(*(v4 + 28) + 3) = 3; // command ID, 3 = EZUSB_IIDX_SECPLUG_CMD_V2_WRITE_DATA ``` That's an easy way to know exactly what node and command you're looking at out of the message handlers. From there, you can usually find debug messages/errors. For example, node ID 1 command ID 3's error message says "[Security] ライトエラー" ("[Security] Write error"). So you know it's the write command. Similarly, node ID 1 command ID 2's error message says "[Security] リードエラー" ("[Security] Read error"). This part of the message handler corresponds to the 5 dongle messages. ``` if ( this[6] > 11 ) { *(this[7] + 88) = result; *(this[7] + 92) = sub_10007C50; } if ( this[6] > 12 ) { *(this[7] + 96) = result; *(this[7] + 100) = sub_10007C70; } if ( this[6] > 13 ) { *(this[7] + 104) = result; *(this[7] + 108) = sub_10007C90; } if ( this[6] > 14 ) { *(this[7] + 112) = result; *(this[7] + 116) = sub_10007CB0; } if ( this[6] > 15 ) { *(this[7] + 120) = result; *(this[7] + 124) = sub_10007CD0; } ``` If you go into each function they look like this: ``` int __cdecl sub_10007C50(int a1) { return sub_10007A00(a1, *(dword_1003000C + 12)); } int __cdecl sub_10007C70(int a1) { return sub_10007A00(1, a1, *(dword_1003000C + 12)); } int __cdecl sub_10007C90(int a1) { return sub_10007A00(2, a1, *(dword_1003000C + 12)); } int __cdecl sub_10007CB0(int a1) { return sub_10007A00(3, a1, *(dword_1003000C + 12)); } int __cdecl sub_10007CD0(int a1) { return sub_10007A00(4, a1, *(dword_1003000C + 12)); } ``` They all are calling the same `sub_10007A00` with the values 0-4. The 0-4 value is used to set the node ID/command ID as such: ``` switch ( a1 ) { case 0: v6 = *(a2 + 12); *(*(v6 + 28) + 2) = 1; *(*(v6 + 28) + 3) = 7; goto LABEL_9; case 1: v7 = *(a2 + 12); *(*(v7 + 28) + 2) = 1; *(*(v7 + 28) + 3) = 8; goto LABEL_9; case 2: v8 = *(a2 + 12); *(*(v8 + 28) + 2) = 1; *(*(v8 + 28) + 3) = 9; goto LABEL_9; case 3: v9 = *(a2 + 12); *(*(v9 + 28) + 2) = 1; *(*(v9 + 28) + 3) = 10; goto LABEL_9; case 4: v10 = *(a2 + 12); *(*(v10 + 28) + 2) = 1; *(*(v10 + 28) + 3) = 11; ... ``` The shared function `sub_10007A00` has a debug error message of "[Security] セレクトエラー" ("[Security] Select error") so it's the dongle/whatever selection function. So it's safe to say that there are 5 dongle commands in total. How they differ exactly is harder to understand because the code is the exact same between all 5 of the commands, except for the command ID being assigned. And the game itself expects a white dongle or black dongle to be returned depending on what slot it reads. IIDX is lenient here but pop'n is picky and will only work with the one specific configuration.
icex2 commented 2022-06-06 02:45:29 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:45

Commented on src/main/bemanitools/popnio.h line 86

That's true I guess. I never do light emulation because I have no way to test or use it. I'll see about at least exposing it.

In GitLab by @33c17f40 on Jun 6, 2022, 01:45 Commented on [src/main/bemanitools/popnio.h line 86](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..d08f341fd85f2a701080692f36802a05ed99e32b#diff-1b6f2c3b17b5a2862a24b38ee8a42a9bR86) That's true I guess. I never do light emulation because I have no way to test or use it. I'll see about at least exposing it.
icex2 commented 2022-06-06 02:49:23 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:49

Commented on src/main/ezusb2-popn-emu/msg.c line 42

I already discussed this on Discord but there's way too much code here for me to comfortably just copypaste it all. I personally question if any of the nodes really even need to be game-specific.

In GitLab by @33c17f40 on Jun 6, 2022, 01:49 Commented on [src/main/ezusb2-popn-emu/msg.c line 42](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..d08f341fd85f2a701080692f36802a05ed99e32b#diff-55c13f5c6658b8e8df51ef3191b11858R42) I already discussed this on Discord but there's way too much code here for me to comfortably just copypaste it all. I personally question if any of the nodes really even need to be game-specific.
icex2 commented 2022-06-06 02:52:15 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:52

Commented on src/main/popnhook1/dllmain.c line 6

changed this line in version 7 of the diff

In GitLab by @33c17f40 on Jun 6, 2022, 01:52 Commented on [src/main/popnhook1/dllmain.c line 6](https://github.com/djhackersdev/bemanitools/compare/ccfe6f7923bd94552d23a1384ec194c3aa444d0e..aaee2f164f9eefffa1b75985d11b4a78f96f48a6#diff-82c6033f7cfd94bfe637cd6bba6f9f74R6) changed this line in [version 7 of the diff](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2576&start_sha=aaee2f164f9eefffa1b75985d11b4a78f96f48a6#21b183f946d822cf9083f6d6ddf92e272ee09ca3_6_1)
icex2 commented 2022-06-06 02:52:15 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:52

added 1 commit

  • e77fe108 - Move avs versions to Module.mk

Compare with previous version

In GitLab by @33c17f40 on Jun 6, 2022, 01:52 added 1 commit <ul><li>e77fe108 - Move avs versions to Module.mk</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2576&start_sha=aaee2f164f9eefffa1b75985d11b4a78f96f48a6)
icex2 commented 2022-06-06 02:59:24 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:59

Commented on dist/popn/popnhook-16.conf line 23

changed this line in version 8 of the diff

In GitLab by @33c17f40 on Jun 6, 2022, 01:59 Commented on [dist/popn/popnhook-16.conf line 23](https://github.com/djhackersdev/bemanitools/compare/ccfe6f7923bd94552d23a1384ec194c3aa444d0e..e77fe10836aea7fbced77852a46baffee0b65029#diff-b192e535a3af75594bf062fb441fd510R23) changed this line in [version 8 of the diff](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2578&start_sha=e77fe10836aea7fbced77852a46baffee0b65029#ec2d3a5a6531f94f4b09d2b1c62bcc47f51ea19b_23_22)
icex2 commented 2022-06-06 02:59:25 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 01:59

added 1 commit

  • e37ee88f - Hide texture_usage_fix setting

Compare with previous version

In GitLab by @33c17f40 on Jun 6, 2022, 01:59 added 1 commit <ul><li>e37ee88f - Hide texture_usage_fix setting</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2578&start_sha=e77fe10836aea7fbced77852a46baffee0b65029)
icex2 commented 2022-06-06 04:39:02 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 6, 2022, 03:39

added 1 commit

  • ee72ec1f - Use constant to make code clearer

Compare with previous version

In GitLab by @33c17f40 on Jun 6, 2022, 03:39 added 1 commit <ul><li>ee72ec1f - Use constant to make code clearer</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2580&start_sha=e37ee88f2d961f433bb19eb31705a266007eb3e8)
icex2 commented 2022-06-06 19:11:04 +03:00 (Migrated from github.com)

I was actually wondering if you need a feature toggle for that to begin with. I guess that works, but if it doesn't hurt to have that texture fix on for all versions supported by popnhook1, that would streamline the code further by removing that one condition.
If that isn't an option, this is fine as is.

I was actually wondering if you need a feature toggle for that to begin with. I guess that works, but if it doesn't hurt to have that texture fix on for all versions supported by popnhook1, that would streamline the code further by removing that one condition. If that isn't an option, this is fine as is.
icex2 commented 2022-06-06 19:13:14 +03:00 (Migrated from github.com)

I personally question if any of the nodes really even need to be game-specific.

That's a fair point. The changes right now don't break anything, but create some sort of tech-debt that needs to be addressed rather sooner than later.
Considering the high value of this MR, I would not make this a blocker for merging as it can be followed up in a separate refactoring MR.

Getting another opinion since he also worked a lot on iidx stuff: What do you think @xyen?

> I personally question if any of the nodes really even need to be game-specific. That's a fair point. The changes right now don't break anything, but create some sort of tech-debt that needs to be addressed rather sooner than later. Considering the high value of this MR, I would not make this a blocker for merging as it can be followed up in a separate refactoring MR. Getting another opinion since he also worked a lot on iidx stuff: What do you think @xyen?
icex2 commented 2022-06-06 19:18:17 +03:00 (Migrated from github.com)

I suggest that we should have at least the API "completed" as we do not have means for versioning on an API level right now (which sucks). I see that once we create an actual release package, people will very quickly create their own implementations of popnio.

Can you come up with a meaningful function signature that you just stub for now in the default popnio implementation?

I suggest that we should have at least the API "completed" as we do not have means for versioning on an API level right now (which sucks). I see that once we create an actual release package, people will very quickly create their own implementations of popnio. Can you come up with a meaningful function signature that you just stub for now in the default popnio implementation?
icex2 commented 2022-06-08 00:38:25 +03:00 (Migrated from github.com)

In GitLab by @xyen on Jun 7, 2022, 23:38

Commented on src/main/ezusb2-popn-emu/msg.c line 42

I'm fine with leaving it as-is, and merging / renaming the node later

In GitLab by @xyen on Jun 7, 2022, 23:38 Commented on [src/main/ezusb2-popn-emu/msg.c line 42](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..d08f341fd85f2a701080692f36802a05ed99e32b#diff-55c13f5c6658b8e8df51ef3191b11858R42) I'm fine with leaving it as-is, and merging / renaming the node later
icex2 commented 2022-06-08 01:33:34 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 8, 2022, 24:33

added 33 commits

  • ee72ec1f...4c0d53b0 - 32 commits from branch djhackers:master
  • deb7f958 - Merge branch 'master' into popn_support

Compare with previous version

In GitLab by @33c17f40 on Jun 8, 2022, 24:33 added 33 commits <ul><li>ee72ec1f...4c0d53b0 - 32 commits from branch <code>djhackers:master</code></li><li>deb7f958 - Merge branch &#39;master&#39; into popn_support</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2599&start_sha=ee72ec1fb9f4581c99aef9e43ba58a701b584537)
icex2 commented 2022-06-08 01:39:04 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 8, 2022, 24:39

added 7 commits

  • c10d0e41 - pop'n music 15 - 18 implemented
  • d1ab369e - Remove old test code
  • f851fa89 - Small cleanup
  • 047f4eed - Apply 1 suggestion(s) to 1 file(s)
  • febfea95 - Move avs versions to Module.mk
  • 4611cc97 - Hide texture_usage_fix setting
  • 556a08d7 - Use constant to make code clearer

Compare with previous version

In GitLab by @33c17f40 on Jun 8, 2022, 24:39 added 7 commits <ul><li>c10d0e41 - pop&#39;n music 15 - 18 implemented</li><li>d1ab369e - Remove old test code</li><li>f851fa89 - Small cleanup</li><li>047f4eed - Apply 1 suggestion(s) to 1 file(s)</li><li>febfea95 - Move avs versions to Module.mk</li><li>4611cc97 - Hide texture_usage_fix setting</li><li>556a08d7 - Use constant to make code clearer</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2601&start_sha=deb7f9580eff54e80282a0a9c8e9ced96bddf4a3)
icex2 commented 2022-06-08 01:49:28 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 8, 2022, 24:49

Commented on src/main/popnhook1/dllmain.c line 124

I can't stop you guys from changing it after it's committed in the future but I do not want to make this an always on patch if I can help it. It's a hack that I don't even really want in the first place and only fixes the one specific game. Applying it to all games could have unintended side effects.

In GitLab by @33c17f40 on Jun 8, 2022, 24:49 Commented on [src/main/popnhook1/dllmain.c line 124](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..556a08d71761ded8a72c24b93c0f943aff27a72b#diff-82c6033f7cfd94bfe637cd6bba6f9f74R124) I can't stop you guys from changing it after it's committed in the future but I do not want to make this an always on patch if I can help it. It's a hack that I don't even really want in the first place and only fixes the one specific game. Applying it to all games could have unintended side effects.
icex2 commented 2022-06-08 02:41:15 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 8, 2022, 01:41

Commented on src/main/popnhook1/dllmain.c line 124

changed this line in version 12 of the diff

In GitLab by @33c17f40 on Jun 8, 2022, 01:41 Commented on [src/main/popnhook1/dllmain.c line 124](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..556a08d71761ded8a72c24b93c0f943aff27a72b#diff-82c6033f7cfd94bfe637cd6bba6f9f74R124) changed this line in [version 12 of the diff](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2603&start_sha=556a08d71761ded8a72c24b93c0f943aff27a72b#21b183f946d822cf9083f6d6ddf92e272ee09ca3_124_124)
icex2 commented 2022-06-08 02:41:16 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 8, 2022, 01:41

added 1 commit

  • e0256754 - Add support for pop'n lights

Compare with previous version

In GitLab by @33c17f40 on Jun 8, 2022, 01:41 added 1 commit <ul><li>e0256754 - Add support for pop&#39;n lights</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2603&start_sha=556a08d71761ded8a72c24b93c0f943aff27a72b)
icex2 commented 2022-06-08 02:43:00 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 8, 2022, 01:43

Commented on src/main/bemanitools/popnio.h line 86

I implemented lights to the best of my ability. I can't test if it works properly when configured but the bits and such I documented in the operator menu also match what was documented in https://dev.s-ul.net/djhackers/bemanitools/-/issues/28#note_7140. Coin blocker and counter aren't configurable I think so I stubbed those.

In GitLab by @33c17f40 on Jun 8, 2022, 01:43 Commented on [src/main/bemanitools/popnio.h line 86](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..d08f341fd85f2a701080692f36802a05ed99e32b#diff-1b6f2c3b17b5a2862a24b38ee8a42a9bR86) I implemented lights to the best of my ability. I can't test if it works properly when configured but the bits and such I documented in the operator menu also match what was documented in https://dev.s-ul.net/djhackers/bemanitools/-/issues/28#note_7140. Coin blocker and counter aren't configurable I think so I stubbed those.
icex2 commented 2022-06-08 02:45:49 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 8, 2022, 01:45

added 1 commit

  • 2f804ec5 - Use memcmp instead of strcmp

Compare with previous version

In GitLab by @33c17f40 on Jun 8, 2022, 01:45 added 1 commit <ul><li>2f804ec5 - Use memcmp instead of strcmp</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2605&start_sha=e025675463353418b6b179b67aa5e7f47e575b51)
icex2 commented 2022-06-11 00:56:46 +03:00 (Migrated from github.com)

Ok, np. I definintely trust your judgement on whether this might become an issue or not. Let's keep your solution.

Ok, np. I definintely trust your judgement on whether this might become an issue or not. Let's keep your solution.
icex2 commented 2022-06-11 00:59:33 +03:00 (Migrated from github.com)

Copy-paste mistake? lights as a parameter doesn't look meaningful. Same goes for the function name popn_io_set_coin_blocker_lights. What about void popn_io_set_coin_blocker(boolean enable); as this addresses only the coin blocker to begin with?
Same for coin counter function below

Copy-paste mistake? `lights` as a parameter doesn't look meaningful. Same goes for the function name `popn_io_set_coin_blocker_lights`. What about `void popn_io_set_coin_blocker(boolean enable);` as this addresses only the coin blocker to begin with? Same for coin counter function below
icex2 commented 2022-06-11 01:33:37 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 11, 2022, 24:33

Commented on src/main/bemanitools/popnio.h line 110

changed this line in version 14 of the diff

In GitLab by @33c17f40 on Jun 11, 2022, 24:33 Commented on [src/main/bemanitools/popnio.h line 110](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..2f804ec5d55c2323d5b0f23fa673eebf5cab021d#diff-1b6f2c3b17b5a2862a24b38ee8a42a9bR110) changed this line in [version 14 of the diff](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2607&start_sha=2f804ec5d55c2323d5b0f23fa673eebf5cab021d#1d36f75e28d74bb54c94a08d5e968401745f3f51_110_102)
icex2 commented 2022-06-11 01:33:37 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 11, 2022, 24:33

added 1 commit

  • d08f341f - Refactor coin lights

Compare with previous version

In GitLab by @33c17f40 on Jun 11, 2022, 24:33 added 1 commit <ul><li>d08f341f - Refactor coin lights</li></ul> [Compare with previous version](/djhackers/bemanitools/-/merge_requests/118/diffs?diff_id=2607&start_sha=2f804ec5d55c2323d5b0f23fa673eebf5cab021d)
icex2 commented 2022-06-11 01:35:22 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 11, 2022, 24:35

Commented on src/main/bemanitools/popnio.h line 110

It wasn't a copypaste mistake. The values are 4 bits wide and I didn't know (still don't really) if something can change depending on what bits are set. I just did a quick check of the code and it always seems to write 0xf to those fields, but without someone doing hardware testing I can't say if it could be something besides 0xf.

It's dealing with lights so I'd prefer not to remove the word "light" from the function names so it's clear. The values are being written to usbLamp after all.

In GitLab by @33c17f40 on Jun 11, 2022, 24:35 Commented on [src/main/bemanitools/popnio.h line 110](https://github.com/djhackersdev/bemanitools/compare/4c0d53b04de4e1638aaf7f3f925eb11e61c5e389..2f804ec5d55c2323d5b0f23fa673eebf5cab021d#diff-1b6f2c3b17b5a2862a24b38ee8a42a9bR110) It wasn't a copypaste mistake. The values are 4 bits wide and I didn't know (still don't really) if something can change depending on what bits are set. I just did a quick check of the code and it always seems to write 0xf to those fields, but without someone doing hardware testing I can't say if it could be something besides 0xf. It's dealing with lights so I'd prefer not to remove the word "light" from the function names so it's clear. The values are being written to `usbLamp` after all.
icex2 commented 2022-06-11 01:36:35 +03:00 (Migrated from github.com)

Fair points, thanks for explaining and lgtm now.

Fair points, thanks for explaining and lgtm now.
icex2 commented 2022-06-11 01:42:08 +03:00 (Migrated from github.com)

In GitLab by @33c17f40 on Jun 11, 2022, 24:42

resolved all threads

In GitLab by @33c17f40 on Jun 11, 2022, 24:42 resolved all threads
icex2 commented 2022-06-11 01:45:42 +03:00 (Migrated from github.com)

approved this merge request

approved this merge request
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Max/djhackersdev_bemanitools#219