libultraship icon indicating copy to clipboard operation
libultraship copied to clipboard

gSPBranchList is broken and crashes any DL that uses it

Open RevoSucks opened this issue 2 years ago • 2 comments

Per a conversation had about it, "gSPBranchList should be the equivalent of gSPDisplayList(), followed by gSPEndDisplayList()". Libultraship's implementation of it can be found here, and it is not a correct implementation:

Instead of:

} else {
    cmd = (Gfx*)seg_addr(cmd->words.w1);
}

It should probably be something like:

} else {
  cmd++;
  uint64_t hash = ((uint64_t)cmd->words.w0 << 32) + cmd->words.w1;
  cmd = (Gfx*)ResourceGetDataByCrc(hash);
  --cmd; // increase after break
}

Thanks for the suggested fix, @Rozelette .

RevoSucks avatar Jun 10 '23 21:06 RevoSucks

would this change break the way gsSPBranchListOTRFilePath is being used in https://github.com/HarbourMasters/Shipwright/pull/2962/files ?

briaguya0 avatar Jun 10 '23 21:06 briaguya0

I do not think it should, @briaguya-ai . The erroneous case is G_DL_OTR_HASH, so, a gSPBranchList to a DL that is referenced to by a hash. gSPBranchList to DLs that are referenced to by a filename or segment address should not be affected as they should already work as intended. To be honest, though, G_DL_OTR_HASH, G_DL, G_DL_OTR_FILEPATH are all different opcodes and I do not know in which contexts they are used. As in, I do not know which the gbi.h macros use, and the ones the macros we override use, and the ones ZAPD uses.

Rozelette avatar Jun 11 '23 04:06 Rozelette