DDR X support + nearly full USB memory implementation - [merged] #214

Closed icex2 closed 1 year ago

In GitLab by @33c17f40 on May 14, 2022, 13:41

_Merges add_ddr_xsupport -> master

Summary

Added support for DDR X.
Added nearly full support for the USB memory device. This code works in X up to latest DDR (only tested A for later games).

Description

A large majority of the DDR 12-16 code is reused so I moved that code to ddrhook-util. The major changes to the shared code are the addition of a new initializer for the p3io code that is used to enable the plug emulation and the USB memory implementation.

I also added a new parameter for ddrhook, -m which can be used to specify the path for the USB memory data. By default for ddrhook and ddrhookx, the default path is usbmem in the same folder as the launcher. The subfolders usbmem\p1 and usbmem\p2 will be automatically created if they don't exist. The data for each individual drive goes inside the p1 and p2 folders. DDR X2 through DDR 2014 are untested as I do not have the data for it but I presume it works since DDR A reads and accepts the edit data files just fine.

Edit data can be easily found online for testing: http://dgddru.blog.fc2.com/blog-category-2.html

For the USB memory emulation, currently writing files is unsupported. I haven't seen it used in-game (but I'm not knowledgeable about modern DDR tbh) but the functionality exists.

Please note that DDR X game has an issue with arrows that apparently also affects DDR X2: the arrows are mostly gray. I have absolutely no idea how to fix that so it can just be hex patched on the user's end similar to what people do for X2 apparently. It won't be fixed by this PR.

USB memory support as tested in DDR X:

Related Issue

How Has This Been Tested?

By launching and playing the game. Also tested to make sure DDR A still boots as expected.

Tested relative and absolute paths for the USB memory paths in both DDR X's conf and through DDR 12-16's -m parameter.

Checklist

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

In GitLab by @33c17f40 on May 14, 2022, 13:47

@icex2 In case you see this PR before the PM, check your PMs on sows in regards to this PR.

In GitLab by @33c17f40 on May 14, 2022, 18:00

added 1 commit

369c628a - Fix DDR X2 and X3 not booting

Compare with previous version

In GitLab by @33c17f40 on May 14, 2022, 18:01

Fix for https://dev.s-ul.net/djhackers/bemanitools/-/issues/83 has been pushed. https://dev.s-ul.net/djhackers/bemanitools/-/merge_requests/113/diffs?commit_id=369c628a27e85965bcf2b8b5d3773786d111136c

In GitLab by @33c17f40 on May 14, 2022, 18:28

added 1 commit

02976621 - Use process_hijack_startup instead of hooking an early function

Compare with previous version

In GitLab by @33c17f40 on May 17, 2022, 04:50

added 1 commit

1095ac3e - Update README

Compare with previous version

In GitLab by @33c17f40 on May 17, 2022, 04:51

marked the checklist item Updated existing doc of or add new doc to README file(s). as completed

Naming-wise, I was expecting ddrhook1 instead of ddrhookx or simply ddrhook if there is only a single hook module we need at this point. Why ddrhookx?

Does DDRX have an external xml config like any recent bemani games? Wondering if the hook here is required.

IIRC someone posted on the pig stall that this is related to some shader code that is not working/compatible on nvidia cards.

Nit: I would expect to have this in the config-gfx module as it doesn't require anything of this hook module. Same applies to ddrhookx_d3d9_init_config

Ok, I just noticed here that this hook is for DDR X only. To stick to BT5's naming scheme, I suggest to rename this one to ddrhook1 and the other hook supporting 12 to 16 to ddrhook2.

That part isn't entirely clear to me. Does this imply that the bootstrapping for DDR X is different compared to most other bemani games that are currently supported by BT5? If yes, do you mind pointed out those differences in the comment?

Naming nit: ddrhookx_master_insert_hooks to scope it to the module and make it easier to identify in symbol tables for debugging purposes.

#ifndef DDRHOOKX_MASTER_H
#define DDRHOOKX_MASTER_H

char *ddrhookx_get_launcher_path_parts();

void ddrhookx_filesystem_hook_init();

Explains the purpose of this function nicely. Want to move that to the definition in the header file? Increases visibility for that imo.

I suppose that's the "earliest entry point" you could use to bootstrapping hooking as early in the main program flow as possible? If applicable, do you mind pointing that out with a comment?

Appears to be unused? Searching for that variable doesn't show any read usage and this being the only write.

This one also seems to be unused.

How's that part of USB support and/or why change the default? Same for ddr-11.conf.

Resolved once I got to the commit that changes this to hijack main.

Streamrolling changeset. Amazing work.

Remark regarding my review: I did my best to understand the changes and questioning if these make sense (based on my experience with other bemani titles). However, the whole usbmem module is just one big mystery to me and I skipped most of it. If there are still specific parts you want me to look at, please point them out and I try my best to review them.

Does this need additional QA before you would want to merge the changes and have them released? If yes, which games and what type of environments/setups would you require?

In GitLab by @33c17f40 on May 19, 2022, 03:15

Commented on src/main/ddrhookx/avs-boot.c line 127

No, DDR X is pre-XML configuration so these hooks are required.

In GitLab by @33c17f40 on May 19, 2022, 03:17

Commented on src/main/ddrhookx/master.c line 57

That comment is copypasted directly from ddrhook's master.c. I didn't really put too much thought into why they did that honestly.

In GitLab by @33c17f40 on May 19, 2022, 03:20

Commented on src/main/ddrhookx/dllmain.c line 111

Cruft from the original code. standard_def and _15khz are defined as extern in ddrhook-util/p3io.c and defined in the dllmain.c. Speaking of which, I forgot to expose _15khz in the configuration it seems.

In GitLab by @33c17f40 on May 19, 2022, 03:21

Commented on dist/ddr/ddr-11-us.conf line 23

Forgot to revert it. I automatically have it unzipping the ddr-11.zip into my game directory after builds for testing so I defaulted it to windowed or else the game would resize everything.

In GitLab by @33c17f40 on May 19, 2022, 03:31

I'll do the code changes later today.

Does this need additional QA before you would want to merge the changes and have them released? If yes, which games and what type of environments/setups would you require?

I gave a quick go through all of the DDR games to make sure I didn't break anything but ideally if someone else could give a quick run through of important games + verify the usbmem changes work, that would be reassuring.

For usbmem: launch the game once to create directories (or create the structure usbmem/p1 and usbmem/p2), run the USB memory under I/O test menu and it should say "NO DATA", extract the contents of the edit data (http://file.blog.fc2.com/dgddru/DDR_EDIT_3REN.zip) into the p1 or p2 folder (file path should become usbmem/p1/DDR_EDIT/DDR_EDIT_J.DAT for example), then run the USB memory test again and it should say "OK" this time. If you go in-game you should see the edit data show up as a new category in DDR X, X2, X3, and 2013 (I think they changed to e-amuse for 2014?).

A series games are also affected by usbmem changes but there's no active uses of it afaik so testing that is optional. As long as it boots it's fine, but it exists in I/O test menu too.

In GitLab by @33c17f40 on May 19, 2022, 04:02

Commented on src/main/ddrhookx/Module.mk line 1

I chose ddrhookx because it was only ever used by DDR X. I am fine with ddrhook1 though. This can't be merged with ddrhook due to all of the extra requirements (.conf configuration, manually hooking libavs/ea3 for booting, etc).