-
Notifications
You must be signed in to change notification settings - Fork 70
Iidx hv and bistro #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General direction looks fine to me, I didn't see anything that would be a dealbreaker. There's only CPU arena in this, right? No player-to-player arena? Is that because of missing lobby functionality or is there something more complicated that needs to be done? Don't forget to update the readme with the additionally supported games. Thanks for tackling this by the way!
@@ -1099,7 +1099,7 @@ def __place(self, tag: Tag, operating_clip: PlacedClip, prefix: str = "") -> Tup | |||
# Get rid of the objects that we're removing from the master list. | |||
operating_clip.placed_objects = [ | |||
obj for obj in operating_clip.placed_objects | |||
if not(obj.object_id == tag.object_id and obj.depth == tag.depth) | |||
if not (obj.object_id == tag.object_id and obj.depth == tag.depth) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was like "woah somebody else is working on the AFP renderer?!" and then I was like ....oh.
if song.artist == 'event_data' and song.genre == 'event_data': | ||
continue | ||
self.__songs[songdata[11]] = (song, songoffset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my god this code is horrible LMAO what was I thinking when I wrote this?!
@@ -72,6 +76,111 @@ def get_duplicate_id(self, musicid: int, chart: int) -> Optional[Tuple[int, int] | |||
16102: 21258, | |||
16101: 21262, | |||
14100: 21220, | |||
28100: 27115, | |||
# Tons of leggendarias here... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my god what a pain. Thank you for figuring this out.
@@ -1752,6 +1761,7 @@ def __revivals(self, songid: int, chart: int) -> Optional[int]: | |||
21258: 16102, | |||
21262: 16101, | |||
21220: 14100, | |||
27115: 28100, # Fucking Fire Beat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funny, that's what I say whenever I'm trying to do hacks for KBM/BMIII/etc...
@@ -1811,6 +1785,79 @@ def __revivals(self, songid: int, chart: int) -> Optional[int]: | |||
if old_id is not None: | |||
return old_id | |||
|
|||
legg_map = { | |||
1017: 1100, 4005: 4100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The double-things-on-line threw me, but I don't think its worth fixing. Honestly we really should just say "We are formatted with black", do a bite-the-bullet pass and move on from there.
# Profile data | ||
pcdata = Node.void('pcdata') | ||
root.add_child(pcdata) | ||
pcdata.set_attribute('id', str(profile.extid)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure hope you got to generate this automatically...
No online arena, but local arena should work by default. I personally don't find enough of a use for online lobby to bother implementing it for any game... With games like iidx, I have to wonder if it might be more advisable to make a separate project just for scraping the musicdbs with charts, and then just having some json blobs stored in this repository with which to seed the database. Because of how much manual mangling has to be done, I just have to wonder if using the chart parser to create a unique id for each chart and using that to determine the server id would be better than what we currently do. For example, with the modern to legacy stuff, I'm honestly not 100% certain the list is completely accurate, and checking for compete accuracy would be a pretty painstaking process if done manually... |
Having a repo that is good at one thing: scraping metadata from ANY game ANYWHERE would actually be a pretty useful thing to have. If its structured right, bemaniutils can just depend on it and have a simple wrapper or something. I have, however, shied away from any scraped or copyright data in this repo, hence why you need to seed your installation from another active one or from your own sourced game files. Wouldn't be against somebody taking the read code, putting it in its own repo, cleaning it up and also checking in per-game blobs. I just won't be the one to do that ;) |
legg_map = { | ||
1017: 1100, 4005: 4100, | ||
4001: 4101, 5014: 5100, | ||
11032: 11100, 11012: 11101, | ||
12002: 12100, 12016: 12101, | ||
13010: 13100, 13038: 13101, | ||
14009: 14100, 14046: 14101, | ||
14022: 14102, 15023: 15101, | ||
15007: 15102, 15061: 15103, | ||
15004: 15104, 15045: 15105, | ||
16050: 16101, 16045: 16102, | ||
16031: 16103, 16015: 16104, | ||
16002: 16105, 17060: 17101, | ||
17028: 17102, 18025: 18100, | ||
18011: 18103, 19063: 19100, | ||
20100: 20103, 20039: 20104, | ||
20068: 20105, 20024: 20106, | ||
20019: 20107, 21012: 21100, | ||
21059: 21101, 21069: 21102, | ||
21073: 21103, 21052: 21104, | ||
21048: 21105, 21050: 21106, | ||
21029: 21107, 21089: 21108, | ||
1005: 21204, 4020: 21205, | ||
5007: 21206, 6013: 21207, | ||
7038: 21208, 8023: 21209, | ||
8024: 21210, 9001: 21211, | ||
9051: 21212, 9033: 21213, | ||
11028: 21215, 12010: 21216, | ||
12052: 21217, 12053: 21218, | ||
12054: 21219, 23054: 23100, | ||
14053: 21222, 15000: 21223, | ||
15001: 21224, 23031: 23101, | ||
15014: 21227, 24041: 24100, | ||
15015: 21228, 15016: 21229, | ||
15020: 21230, 23070: 23102, | ||
15025: 21232, 15026: 21233, | ||
15032: 21234, 15041: 21235, | ||
15054: 21236, 24011: 24101, | ||
16000: 21238, 16001: 21239, | ||
16011: 21241, 16016: 21243, | ||
16017: 21244, 16018: 21245, | ||
16020: 21246, 16021: 21247, | ||
16022: 21248, 16024: 21249, | ||
16025: 21250, 16028: 21251, | ||
16030: 21252, 14012: 21264, | ||
16034: 21254, 16038: 21255, | ||
16040: 21256, 16042: 21257, | ||
16047: 21259, 16049: 21260, | ||
15005: 21261, 15008: 21263, | ||
22008: 22101, 22013: 22102, | ||
22024: 22103, 22027: 22104, | ||
22031: 22105, 22089: 22106, | ||
22006: 22107, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bit is not properly indented.
28007 CANVAS feat. Quimär & BEMANI Sound Team "L.E.D.-G" | ||
28017 Pārvatī | ||
28050 POLꞰAMAИIA | ||
28084 Ignis†Iræ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another tab needs to be inserted at the end of the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gonna be honest, I underestimated just how much my ability to work on this would drop once I started working a "real" job. Thanks for the notes though. If you wanted to take over the pull request you would have my thanks tbh.
I've been running this branch for a little while. It seems okay for now, but I've spotted few issues within the code. Suggestions above are what I've found. Also, |
replaced by #58 |
Missing traffic tests. I can't think of any obvious issues with the pull request in its current state though so I'm submitting as is for now. I'll do traffic tests in a bit (Already recorded packet logs for them I just need to sit down and do the rest). If you have any design objections feel free to bring them up though! :) As an aside, it might be advisable to update the bemapi spec to account for the new iidx charts...