Closed mid-kid closed 1 year ago
@vulcandth The only change I'm not confident about I've outlined above in the previous comment, though it isn't part of this PR. That said, I'd like for @Rangi42 to take a cursory look as well.
Looks good to me! Nice work!
pokegold will need this and the last few commits ported too.
Oh, right. Pokegold. Will do.
@mid-kid if you have time can you take a peak at pokered. I think many of these improvements to the link code can be brought over there too. I can do that work. Just wanted you to give a quick glance since the link code is fresh on your mind.. and hopefully you can identify some potential improvements we can backport over there. If you don't have the time no worries.
I'll be honest I've hardly looked at the gen1 parts of the link code, though it works similarly, so I might check it out.
Looks good to me! Nice work!
pokegold will need this and the last few commits ported too.
@Rangi42 I've been keeping pokegold up to date already. The last few commits of pokecrystal are not applicable to pokegold, the ones that were I've already ported.
As a side-note, it's very tempting to nuke the wLinkData unions (Specifically, link data members, link mail data, received link mail data). Primarily because wLinkData is a generic temporary buffer for various operations, and its layout is slightly floaty (depending on how the data rolls in), and the layout of the data during sending is very different than the one used during reception. This same symptom can be seen in "link mail data" vs "received link mail data". It's very confusing to follow code that uses the different union sections interchangeably, since it's not always clear that the data being consumed is the same that was received just a second ago, not some new data.
Only one label is used within the "link data members" section: wLinkPlayerData (and its alternate name wTimeCapsulePlayerData). Similarly, we don't have labels for the layout used in e.g.
Link_PrepPartyData_Gen2
/Link_PrepPartyData_Gen1
/FixDataForLinkTransfer
. Trying to fit these into another union section, especially considering both functions have slightly differing layouts (REDMON_STRUCT_LENGTH
) feels very awkward.