NVRAM config overwrite wasn't working and people were getting confusing crashes as a result of the game failing to initialise.
Error messages weren't being rotated, which is really just annoying
First PR to the github, let me know if I've missed anything new!
tl;dr:
1. NVRAM config overwrite wasn't working and people were getting confusing crashes as a result of the game failing to initialise.
2. Error messages weren't being rotated, which is really just annoying
First PR to the github, let me know if I've missed anything new!
Relative path to current working dir, is that fine? If yes, why?
Why are you using CreateDirectoryA and not the AVS filesystem functions? Using the AVS functions, I am wondering if this might benefit from the whole avs-config thing where you can configure your file systems and paths.
Remark: I have no idea how this works on jubeat and what the game expects.
Got a few questions here:
- Relative path to current working dir, is that fine? If yes, why?
- Why are you using `CreateDirectoryA` and not the AVS filesystem functions? Using the AVS functions, I am wondering if this might benefit from the whole `avs-config` thing where you can configure your file systems and paths.
Remark: I have no idea how this works on jubeat and what the game expects.
Suggestion: It might make sense to output the data that failed verification here, so you know exactly which data failed verfication and how it looked in memory. Same for the EAMID below.
Suggestion: It might make sense to output the data that failed verification here, so you know exactly which data failed verfication and how it looked in memory. Same for the EAMID below.
Truthfully, no reason apart from "avshelper.dll does it, so I will do the same". If F: doesn't exist, that's exactly what the game does. We could instead match the other games and shift this dir creation out to the .bat files. Thoughts?
Truthfully, no reason apart from "`avshelper.dll` does it, so I will do the same". If `F:` doesn't exist, that's exactly what the game does. We could instead match the other games and shift this dir creation out to the .bat files. Thoughts?
I've actually got some interesting feedback on this whole thing - the PCBID that is provided in config is totally different from the PCBID any server gets (bytes are shuffled about, checksum is removed, prefix is forced), and it's extremely confusing why on earth this happens to the server administrator.
I think it'd be better to just force 0140 as the prefix (which is what the game does), auto-calculate the checksum, and send that instead (because the checksum doesn't make it to the server at all).
This isn't just a server thing, but the test menu's SYSTEM page displays the same "broken" PCBID.
I've actually got some interesting feedback on this whole thing - the PCBID that is provided in config is totally different from the PCBID any server gets (bytes are shuffled about, checksum is removed, prefix is forced), and it's extremely confusing why on earth this happens to the server administrator.
I think it'd be better to just force 0140 as the prefix (which is what the game does), auto-calculate the checksum, and send that instead (because the checksum doesn't make it to the server at all).
This isn't just a server thing, but the test menu's SYSTEM page displays the same "broken" PCBID.
Well, sticking to what the game does is probably not a bad default if you cannot see any issues with that. I guess we can also iterate that when needed. The current solution sounds reasonable as it also avoids complexity. Let's keep it like that for now.
Well, sticking to what the game does is probably not a bad default if you cannot see any issues with that. I guess we can also iterate that when needed. The current solution sounds reasonable as it also avoids complexity. Let's keep it like that for now.
Huh, that's a problem I ran into also with some of the old IIDX games. You brought this up in a chat already and the suggestion was to handle the unscrambling etc. on the hook side, so the game gets a correctly ordered and non broken ID.
Since handling of PCBID/EAMID is already known to be different across different games and also versions of the same game series, I agree to the approach that we also stick to what jubeat does and implement that specifically and only for the jubeat hooks.
To summarize, that means:
Allow users to set any clean PCB/EAM ID in the hook config. Stick to the, what I believe is, "standard base format", e.g. 0101020304050607083F: first 01 being the header, 3F the checksum and 01020304060708 being the actual ID.
The hook takes care of any game specific needs, in this case for jbhook:
Shuffle bytes so the server gets the ID in exactly the same way as the user provided it in the hook config
Enforce 0140 as prefix and throw error to user that PCBIDs need that prefix
Checksum verification and tell the user if mismatch
Does that make sense? Anything missing?
Huh, that's a problem I ran into also with some of the old IIDX games. You brought this up in a chat already and the suggestion was to handle the unscrambling etc. on the hook side, so the game gets a correctly ordered and non broken ID.
Since handling of PCBID/EAMID is already known to be different across different games and also versions of the same game series, I agree to the approach that we also stick to what jubeat does and implement that specifically and only for the jubeat hooks.
To summarize, that means:
* Allow users to set any clean PCB/EAM ID in the hook config. Stick to the, what I believe is, "standard base format", e.g. `0101020304050607083F`: first `01` being the header, `3F` the checksum and `01020304060708` being the actual ID.
* The hook takes care of any game specific needs, in this case for jbhook:
* Shuffle bytes so the server gets the ID in exactly the same way as the user provided it in the hook config
* Enforce `0140` as prefix and throw error to user that PCBIDs need that prefix
* Checksum verification and tell the user if mismatch
Does that make sense? Anything missing?
I think the user shouldn't need to provide the checksum - with the 0140 prefix, the checksum is invisible as far as the server is concerned, with a remaining 8 bytes (16 characters stringified) available for the "actual" PCBID.
To be clear, the PCBID: 01020304050607080926
Will be displayed in the test menu, and sent to any server as: 01400208070605040309
I think the user shouldn't need to provide the checksum - with the `0140` prefix, the checksum is invisible as far as the server is concerned, with a remaining 8 bytes (16 characters stringified) available for the "actual" PCBID.
To be clear, the PCBID:
`01020304050607080926`
Will be displayed in the test menu, and sent to any server as:
`01400208070605040309`
Hmm, that would differ from how other games supported in bemanitools require those IDs to be provided by the user as well as the "official way" in an ea3-config.xml on newer games. I expect that telling the user to strip the existing IDs to be quite error prone.
Hmm, that would differ from how other games supported in bemanitools require those IDs to be provided by the user as well as the "official way" in an `ea3-config.xml` on newer games. I expect that telling the user to strip the existing IDs to be quite error prone.
I think we could argue that we've been doing it wrong this whole time - I've now talked to 3 server operators who essentially said "PCBIDs on old games get mangled when they're sent to us".
I think this could be adapted to be different logic -
If the checksum is valid, assume the user has an old config, and use the old behaviour
Otherwise, enforce new "matches the test menu" behaviour (aka 0140 prefix)
I think we could argue that we've been doing it wrong this whole time - I've now talked to 3 server operators who essentially said "PCBIDs on old games get mangled when they're sent to us".
I think this could be adapted to be different logic -
- If the checksum is valid, assume the user has an old config, and use the old behaviour
- Otherwise, enforce new "matches the test menu" behaviour (aka `0140` prefix)
Can you elaborate which games and versions specifically are known to be affected by this?
Changing the logic on other games, e.g. IIDX, to find common alignment is a change with a larger blast radius. I suggest to push that back for now until we understand how many different ways we have to deal with and which games and versions are using them.
I might be open to accepting this solution just for jubeat for now, but I want us to be aware of the impact and have a direction forward for the PCBID/EAMID problem in general.
Can you elaborate which games and versions specifically are known to be affected by this?
Changing the logic on other games, e.g. IIDX, to find common alignment is a change with a larger blast radius. I suggest to push that back for now until we understand how many different ways we have to deal with and which games and versions are using them.
I might be open to accepting this solution just for jubeat for now, but I want us to be aware of the impact and have a direction forward for the PCBID/EAMID problem in general.
Yeah look, that's fair. It's going to be a breaking change, considering it's been acceptably broken for so long.
I can revert the security key stuff and postpone that to a separate commit instead of bundling it with jubeat.
As far as I can tell, it affects p3io games (DDRX, jubeat), ezusb games (IIDX...?). I almost want to generalise and say "all roundplug games" but it might be closer to "all roundplug2 games" since I noticed a few different versions of round plugs in the source.
Yeah look, that's fair. It's going to be a breaking change, considering it's been acceptably broken for so long.
I can revert the security key stuff and postpone that to a separate commit instead of bundling it with jubeat.
As far as I can tell, it affects p3io games (DDRX, jubeat), ezusb games (IIDX...?). I almost want to generalise and say "all roundplug games" but it might be closer to "all roundplug2 games" since I noticed a few different versions of round plugs in the source.
Ok, cool. I like the approach to de-couple this from the other changes in this PR that I am happy to merge. I suggest to also lift this to an issue where we can continue discussing about how to address this on other games. One of the key problems I see is that we also need a solution, potentially a migration path, for server operators supporting already the "broken way" of doing this. I expect we have to keep supporting this for a long time, probably forever on the games where we are supporting it already.
Ok, cool. I like the approach to de-couple this from the other changes in this PR that I am happy to merge. I suggest to also lift this to an issue where we can continue discussing about how to address this on other games. One of the key problems I see is that we also need a solution, potentially a migration path, for server operators supporting already the "broken way" of doing this. I expect we have to keep supporting this for a long time, probably forever on the games where we are supporting it already.
snuck in an extra little change to close the crash issue. I checked bootstrap.xml and we are indeed missing it. Technically it should be crashing in copious too, but I note that most of the new games don't have that in their gamestart.bat, so I imagine konami changed their logic at some point.
snuck in an extra little change to close the crash issue. I checked bootstrap.xml and we are indeed missing it. Technically it should be crashing in copious too, but I note that most of the new games don't have that in their gamestart.bat, so I imagine konami changed their logic at some point.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
tl;dr:
First PR to the github, let me know if I've missed anything new!
Got a few questions here:
CreateDirectoryAand not the AVS filesystem functions? Using the AVS functions, I am wondering if this might benefit from the wholeavs-configthing where you can configure your file systems and paths.Remark: I have no idea how this works on jubeat and what the game expects.
Suggestion: It might make sense to output the data that failed verification here, so you know exactly which data failed verfication and how it looked in memory. Same for the EAMID below.
lgtm though awaiting your feedback on the two comments. But no major concerns right now that we can merge this then.
Truthfully, no reason apart from "
avshelper.dlldoes it, so I will do the same". IfF:doesn't exist, that's exactly what the game does. We could instead match the other games and shift this dir creation out to the .bat files. Thoughts?I've actually got some interesting feedback on this whole thing - the PCBID that is provided in config is totally different from the PCBID any server gets (bytes are shuffled about, checksum is removed, prefix is forced), and it's extremely confusing why on earth this happens to the server administrator.
I think it'd be better to just force 0140 as the prefix (which is what the game does), auto-calculate the checksum, and send that instead (because the checksum doesn't make it to the server at all).
This isn't just a server thing, but the test menu's SYSTEM page displays the same "broken" PCBID.
Well, sticking to what the game does is probably not a bad default if you cannot see any issues with that. I guess we can also iterate that when needed. The current solution sounds reasonable as it also avoids complexity. Let's keep it like that for now.
Huh, that's a problem I ran into also with some of the old IIDX games. You brought this up in a chat already and the suggestion was to handle the unscrambling etc. on the hook side, so the game gets a correctly ordered and non broken ID.
Since handling of PCBID/EAMID is already known to be different across different games and also versions of the same game series, I agree to the approach that we also stick to what jubeat does and implement that specifically and only for the jubeat hooks.
To summarize, that means:
0101020304050607083F: first01being the header,3Fthe checksum and01020304060708being the actual ID.0140as prefix and throw error to user that PCBIDs need that prefixDoes that make sense? Anything missing?
I think the user shouldn't need to provide the checksum - with the
0140prefix, the checksum is invisible as far as the server is concerned, with a remaining 8 bytes (16 characters stringified) available for the "actual" PCBID.To be clear, the PCBID:
01020304050607080926Will be displayed in the test menu, and sent to any server as:
01400208070605040309Hmm, that would differ from how other games supported in bemanitools require those IDs to be provided by the user as well as the "official way" in an
ea3-config.xmlon newer games. I expect that telling the user to strip the existing IDs to be quite error prone.I think we could argue that we've been doing it wrong this whole time - I've now talked to 3 server operators who essentially said "PCBIDs on old games get mangled when they're sent to us".
I think this could be adapted to be different logic -
0140prefix)Can you elaborate which games and versions specifically are known to be affected by this?
Changing the logic on other games, e.g. IIDX, to find common alignment is a change with a larger blast radius. I suggest to push that back for now until we understand how many different ways we have to deal with and which games and versions are using them.
I might be open to accepting this solution just for jubeat for now, but I want us to be aware of the impact and have a direction forward for the PCBID/EAMID problem in general.
Yeah look, that's fair. It's going to be a breaking change, considering it's been acceptably broken for so long.
I can revert the security key stuff and postpone that to a separate commit instead of bundling it with jubeat.
As far as I can tell, it affects p3io games (DDRX, jubeat), ezusb games (IIDX...?). I almost want to generalise and say "all roundplug games" but it might be closer to "all roundplug2 games" since I noticed a few different versions of round plugs in the source.
Ok, cool. I like the approach to de-couple this from the other changes in this PR that I am happy to merge. I suggest to also lift this to an issue where we can continue discussing about how to address this on other games. One of the key problems I see is that we also need a solution, potentially a migration path, for server operators supporting already the "broken way" of doing this. I expect we have to keep supporting this for a long time, probably forever on the games where we are supporting it already.
By reverting the pcbid/eamid changes, I think this is good to merge as-is
snuck in an extra little change to close the crash issue. I checked bootstrap.xml and we are indeed missing it. Technically it should be crashing in copious too, but I note that most of the new games don't have that in their gamestart.bat, so I imagine konami changed their logic at some point.
lgtm. Let me know if it's fine to merge it.