Fix spurious differences when using ninja diff
#2294
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TakaRikka
approved these changes
Jan 27, 2025
hatal175
pushed a commit
that referenced
this pull request
Feb 3, 2025
* d_a_obj_TvCdlst matching * d_a_obj_kwheel00, 01, & d_a_obj_klift00 Equivalent (weak func and/or vtable order) (#2291) * d_a_obj_kwheel00, 01, & d_a_obj_klift00 Equivalent (weak func and/or vtable order) * Three rels equivalent * Basic documentation for the three rels * Define explicit constructor for dMdl_obj_c (needed for proper array creation in d_a_obj_klift00) * Modify d_a_obj_bky_rock to account for new explicit constructor of dMdl_obj_c * Specify weak func and/or vtable orders for rels in config.py * Cleanup includes and dol2asm artifacts * * Fix debug build * Restore incorrectly removed l_bmdidx * Fix US retail equivalency * d_a_obj_Turara equivalent (#2289) * Initial d_a_obj_Turara work * More d_a_obj_Turara work * d_a_obj_Turara equivalent except for weak func order of dEvLib_callback_c dtor * Updates to d_a_obj_Turara after feedback from Taka * b_dre equivalent (#2290) * Update d_a_obj_Turara to remove unnecessary comment based on hatal\'s request --------- Co-authored-by: TakaRikka <[email protected]> * Fix spurious differences when using `ninja diff` (#2294) * Fix spurious differences when using ninja diff * Fix some clangd errors/warnings * d_a_obj_waterGate matching + d_a_obj_lv3Water2 equivalent (#2293) * d_a_obj_waterGate matching + d_a_obj_lv3Water2 equivalent * Add simpleCalcMaterial implicit inline to J3DModelData (seen in debug) * Basic documentation for both actors * WIP debug build implementations (note TODOs) * Update configure.py and note weak ordering issues for lv3Water2 * Actually add modified J3DModelData.h & explicitly note fake padding * Add fopAcM_Register... macros (for debug build) (#2295) * Update d_a_obj_Tvcdlst to use LIGHT_INFLUENCE after Taka gave feedback --------- Co-authored-by: Huitzi <[email protected]> Co-authored-by: TakaRikka <[email protected]> Co-authored-by: LagoLunatic <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DTK's
ninja difffeature can be useful for finding which symbol in the dol is causing the hash to not match, but if there are any symbols in linked TUs in the dol that had their padding manually added to them, then DTK just picks those up as diffs (because the size is wrong) and doesn't show the symbol that's actually affecting the hash. I went through and fixed all these padding issues, soninja diffnow doesn't show anything, which means in the future it can be used without showing any false positive padding diffs.Some of these padding issues are because of the stripped abstract base class vtable issue that affects a lot of TUs, I replaced the hacks to add padding there with a new solution using
dummy_abstract_class, which should be pretty similar to the real fix for this, we just don't know the actual names of these classes.