1264 FX Loading #208
1264 FX Loading #208
Conversation
Now we should be able to atleast handle 1264 FXB file loading without making them crash.
There was a problem hiding this comment.
Hi @UTengine, thanks for the PR.
As far as I know we're still not using FX from v1264 version. The client shouldn't crash there unless you made local changes that are not exists in the official repository.
On a different topic, my plan was to replace the entire FX folder with USKO's, since they still preserve older FX, just in their newer formats. That way we don't have to worry about future updates. Also note that FX implementation preserve versioning, hence the idea of using latest USKO's FX folder.
| @@ -457,6 +457,15 @@ bool CN3FXBundle::Load(HANDLE hFile) { | |||
| ReadFile(hFile, &m_bStatic, sizeof(bool), &dwRWC, NULL); | |||
| } | |||
|
|
|||
| if (m_iVersion == 3) { | |||
There was a problem hiding this comment.
I don't know if there's any real intention of supporting them, but I would personally make this m_iVersion >= 3 to allow for future versions since there's no real cost for doing so, and only unintentional complications down the track as a result of the explicit check.
| @@ -228,6 +228,10 @@ bool CN3FXPartMesh::Load(HANDLE hFile) { | |||
| ReadFile(hFile, &m_vUnitScale, sizeof(__Vector3), &dwRWC, NULL); | |||
| } | |||
|
|
|||
| if (m_iVersion == 6) { | |||
There was a problem hiding this comment.
For the same reason as the other case (no real cost to allowing for future versions), I would also make this >=
There was a problem hiding this comment.
Actually there is I got this all working already and 1298 fx loading up in other repo/branch where I completed this before ragequiting due to stress on snoxdko discord. About the other if m version 3 is because I intended it to be last major version to be supported and that's why it was ==3. For this one I can't comment much on it have to double triple check my earlier work.
There was a problem hiding this comment.
No I get that, I'm just saying that there's really no downside to just swapping that to >=. All it does is it futureproofs it with no real downside.
There was a problem hiding this comment.
Mgame has a bad habit of doing things weirdly. I've come across parameters that were removed or changed to a different position that's why I probably did this it's hard to tell without the src I had on a diff branch/repo where it was working as in atleast loading and displaying it properly for 95%. Just sometimes scaling or newer functions that aren't added yet but atleast it solved all the crashing.
There was a problem hiding this comment.
But you are probably right anyway. Hey were talking to 2stars afteral. At this point it was all a WIP and I don't know asm so I had to brainstorm to come so far.
| } | ||
|
|
||
| if (m_iVersion >= 7) { | ||
| ReadFile(hFile, &m_bOnScreen, sizeof(bool), &dwRWC, NULL); // is this on screen bool? |
There was a problem hiding this comment.
Unless I'm crazy, I think these 2 are mixed up? (i.e. m_bOnScreen should be m_bRotationRate and vice versa)
There was a problem hiding this comment.
Could be like I said I don't have pdbs or asm/reversing experience. This is half of the pr the other was somewhere else to correct few issues. If you want I Can try to search for the src and go over it together?
There was a problem hiding this comment.
It's a little hard to tell without any implementation behaviour, but based on naming alone (specifically the rotation one), the way I've named them both personally fits with them the other way around (which tracks with their actual implementation behaviour), which makes me think you possibly accidentally mixed up the order here. It's just a little hard to tell from this alone though if you accidentally mixed up the order or the names perhaps aren't the clearest.
There was a problem hiding this comment.
Probably could have mixed them the other way around very highly you're right. For the naming they are the original naming convention except for versioning which I named myself OFC. I'll try to get them today so you can take a look at it.
There was a problem hiding this comment.
Bool on screen came later so you must be right...
There was a problem hiding this comment.
I now understand the order you're pulling them from is the order in the leaked project files. This doesn't mean that they're handled in that order; these project files don't care about the versioning whatsoever, so they're not even currently output in a strict version order, meaning they don't really have any reason to stick to that and throw them in at the bottom.
Now that I understand the reasoning behind the names (leaked effect project files) and why you put them in that order (appearance in the saved project file, which is no guarantee of order in the binary), I can definitely say that they're backwards without question here.
Now we should be able to atleast handle 1264 FXB file loading without making them crash.
WIP Because we can't cast skills ingame yet so I will have to create a WIP branch to skip the pesky itemgroup checks on left/right hand for skills/skilltree/hotkeybar fxmanager?.
💔Thank you!