trigger-segfault / OpenLRR

An open source re-implementation of LEGO Rock Raiders 🪨⛏
65 stars 5 forks source link

Implemented the Vehicle_ functions #61

Closed jackorobot closed 1 year ago

jackorobot commented 1 year ago

This PR attempts to implement the Vehicle_ functions. The correct functionality is however unknown for most functions, since they seem to not be referenced anywhere else.

trigger-segfault commented 1 year ago

I'm sorry this took so long for me to get to. It really defeats the purpose of being an open source project that's dependent on user contributions if I take this long to review a single PR. :(


When implementing functions, usually they need to be hooked in interop.cpp. For example, in interop_hook_LegoRR_Vehicle, you would add the line(s):

// used by: LegoObject_UpdateRoutingVectors_FUN_004428b0, LegoObject_SetPositionAndHeading,
//          LegoObject_FP_UpdateMovement, LegoObject_UpdateWorldStickyPosition
result &= hook_write_jmpret(0x0046d640, LegoRR::Vehicle_SetPosition);

The "used by" comment is there just to note what functions are dependent on this function's implementation. Function usage can be found by going to the function signature in Ghidra, or by searching the Ghidra source dump for the function name. If all of the "used by" functions are also implemented, then the statement to hook Vehicle_SetPosition can be commented out, because no native LRR code would be trying to call the original function.

I'm guessing this made it a lot harder to debug many of the Vehicle_ functions you implemented, since the game was resorting to calling the original functions instead. Again, this is mainly on me, since I haven't written proper instructions for implementing and decompiling functions.


For issues, there's just a handful of similar logic errors, and one missing math statement. Good job catching the difference between contWheels and wheelNulls, since the field names for those are basically swapped in the Ghidra source. The optimization in Vehicle_SetPosition for doing everything at the end in one loop is also a nice touch, as none of those 4 loops cause side effects with the other wheels. :)

jackorobot commented 1 year ago

No problem at all! People are busy individuals, and having someone contribute to your project might take some adjustment on your end as well.

Thanks for looking into this PR. Good thing you spotted that the hooks are not in place, that is indeed something I did not notice before. This does explain why my code appeared to be functioning properly every time 🤦. In short I will go over these changes again to make sure they are actually called and are without bugs.

Will update with your comments as well, thanks for that. And will look into how you hooked into the original executable, to learn.

trigger-segfault commented 1 year ago

Sorry, I didn't realize that the review comments weren't visible to anyone else before. You should be able to see them now. The UX for PR reviews is pretty bad.

jackorobot commented 1 year ago

Updated the code with your comments. Mostly added the hooks, which now indeed have called the reversed functions. This was tested working with the vehicle training mision.

trigger-segfault commented 1 year ago

Everything looks good. 👍