Use hsd assert macros by MarkMcCaskey · Pull Request #2988 · doldecomp/melee

MarkMcCaskey · 2026-07-26T17:14:06Z

@ribbanya suggested looking into this for a clean up.

It looks like the "0" case is perhaps a new concept like a PANIC wrapper macro or something. The case about __FILE__ not being directly called is weird but Claude's saying there's precedent for this already.

Worth calling out from the wall of text below but LbHeapStatus_Destroy is an inferred name, it does not come directly fro the DOL.

From Claude:

Names recovered from the assert strings: It_PKind_Start/It_PKind_Terminate, Pl_ItemLog_Terminate,LbCardNewTaskArray_Max, LbHeapStatus_Create,Gm_Player_NumMax,INIT_ADD_PARTS_RANGE, Gr_Homerun_Parts_Max, and the locals jobj (gmregenddisp) and pointp (grzakogenerator).

lbfile.c stores its file name in an array rather than emitting a fresh __FILE__ literal, so it respells both macros around that array, following the existing precedent in mnvibration.c.

One name is inferred rather than recovered: LbHeapStatus_Destroy for value 1. Only LbHeapStatus_Create appears in the binary; the pairing comes from the file's own lbHeap_DestroyOffsetViewIfCreated / lbHeap_CreateOffsetViewIfDestroyed. Happy to drop it and leave status == 1 numeric.

Two notes for the wider cleanup:

.clang-format already lists HSD_ASSERT and HSD_ASSERTREPORT under WhitespaceSensitiveMacros, so conditions whose original spelling is not clang-format canonical (71*1024) survive stringification unchanged.

The OSReport sweep does not turn into HSD_ASSERTREPORT as often as expected. About 40 sites are if (bad) { OSReport(...); HSD_ASSERT(N, 0); }, where the binary's assert text is literally "0" -- those are already correct, and HSD_ASSERTREPORT would replace "0" with the stringified condition. The remaining OSReport work is the messages held in data symbols (lbmemory, mnvibration, mndiagram, ftkirby).

Left for a follow-up: _p(field), the original module-private accessor. It is not a decomp invention -- it appears in binary assert strings for cmsnap, lbcardgame, lbcardnew, lbsnap and lbmemory, and video.c already defines #define _p. Adopting it unblocks 9 more HSD_ASSERTMSG sites and most of lbmemory.c's 11 raw asserts, but it is a visible style change across five files, so it seemed worth asking about first.

The mechanical part was a script that computes the C stringization of each condition and rewrites the site only when it is byte-identical to the existing message. The rest was per-site: recover the name, apply it, recompile the whole translation unit, and compare section contents against a master worktree rather than raw bytes, since MWCC renumbers the anonymous literal pool.

decomp-dev · 2026-07-26T17:19:31Z

Report for GALE01 (`efce319` - `fea3e40`)

✅ 2 new matches

Unit	Item	Bytes	Before	After
`main/melee/it/it_3F2F`	`.data`	+9472	0.00%	100.00%
`main/melee/it/it_279C`	`.data`	-9472	100.00%	100.00%

📈 1 improvement in an unmatched item

Unit	Item	Bytes	Before	After
`main/melee/lb/lbmemory`	`lbMemory_80015320`	+3	96.11%	96.70%

ribbanya · 2026-07-26T17:40:42Z

Is Claude actually using melee-replace-symbols? Would be easier to use that and then fix false positives than to 'manually' perform renames.

Issues: 16
Error (16)
    Semantic Issue (16)
        required (16)
            src/melee/gr/grkinokoroute.c (2)
                531:35: use of undeclared identifier 'Pokemon_Random'
                546:35: use of undeclared identifier 'Pokemon_Random'
            src/melee/pl/plbonuslib.c (1)
                534:14: use of undeclared identifier 'Pokemon_Lucky_Egg'
            src/melee/it/items/ithinoarashi.c (1)
                269:18: use of undeclared identifier 'Pokemon_Hinoarashi_Flame'
            src/melee/it/items/ithitodeman.c (1)
                382:18: use of undeclared identifier 'Pokemon_Hitodeman_Star'
            src/melee/it/items/itlugia.c (6)
                338:31: use of undeclared identifier 'Pokemon_Lugia_Aeroblast'
                355:31: use of undeclared identifier 'Pokemon_Lugia_Aeroblast2'
                360:31: use of undeclared identifier 'Pokemon_Lugia_Aeroblast3'
                533:10: use of undeclared identifier 'Pokemon_Lugia_Aeroblast'
                536:10: use of undeclared identifier 'Pokemon_Lugia_Aeroblast2'
                539:10: use of undeclared identifier 'Pokemon_Lugia_Aeroblast3'
            src/melee/it/items/itmatadogas.c (4)
                101:35: use of undeclared identifier 'Pokemon_Matadogas_Gas1'
                103:35: use of undeclared identifier 'Pokemon_Matadogas_Gas2'
                251:10: use of undeclared identifier 'Pokemon_Matadogas_Gas1'
                254:10: use of undeclared identifier 'Pokemon_Matadogas_Gas2'
            src/melee/it/items/itunknown.c (1)
                246:18: use of undeclared identifier 'Pokemon_Unknown_Swarm'

ribbanya · 2026-07-26T17:58:33Z

@MarkMcCaskey Might be useful for Claude to know to read through the wiki (gh repo clone doldecomp/melee.wiki) every once in a while.

MarkMcCaskey · 2026-07-26T18:04:48Z

@MarkMcCaskey Might be useful for Claude to know to read through the wiki (gh repo clone doldecomp/melee.wiki) every once in a while.

Yeah I missed the tool, I'll update my local guidelines with that.

As for this PR, Opus 5 has been the main model on it so far and I'm seeing patterns that it's just a notably weaker model than the others.

HSD_ASSERTMSG only exists so an assert can print text that differs from the code it checks. 46 of the 76 uses do not need that: the message either already spells the condition, or names the variable/constant the condition should have been written with. Recovered from the message strings: It_PKind_Start/_Terminate, Pl_ItemLog_Terminate, LbCardNewTaskArray_Max, LbHeapStatus_Create, INIT_ADD_PARTS_RANGE, Gr_Homerun_Parts_Max, and the locals jobj (gmregenddisp) and pointp (grzakogenerator). INIT_ADD_PARTS_RANGE also removes grhomerun's f32 gobjs_num temporary, which only existed to hold the asserted expression. .clang-format already lists HSD_ASSERT under WhitespaceSensitiveMacros, so conditions whose original spelling is not clang-format canonical (71*1024) survive stringification intact. All nine affected translation units compile to byte-identical objects.

Fifteen sites across nine files. Several carried a todo saying no byte-matching macro form was known; the plain forms do match, and the only object delta is anonymous literal-pool renumbering. HSD_ASSERT(line, 0) after a separate OSReport reproduces every 'OSReport then __assert(..., "0")' pair (grkongo, grmutecity, lbaudio_ax, toy, gmtoulib). HSD_ASSERTREPORT reproduces the pairs whose assert text is a real condition (plbonuslib, lbfile). In plbonuslib this removes the var_r0 flag m2c emitted for the ternary, and recovers Gm_Player_NumMax, which a todo in that file already suspected. lbfile.c stores its file name in an array rather than emitting a fresh __FILE__ literal, so it respells both macros around that array, following the existing mnvibration.c precedent. func_800163D8_inline then becomes dead: the wrapper only existed to supply the inline layer HSD_ASSERTREPORT provides. Not converted: stage.c 360, whose fake zero-byte padding sets where the next .data object lands, and mnhyaku 360, where HSD_ASSERTREPORT bills 8 more bytes into the frame of the function that inlines allocMenu. All nine translation units keep identical section contents.

Per ribbanya: The It_PKind_ prefix the assert recovers belongs on ItemKind's Pokemon entries, not on a new enum. The 32 Poke Ball entries become It_PKind_*, with It_PKind_Start and It_PKind_Terminate as inline range markers, and the 18 Pokemon-related articles beyond the range take the ordinary It_Kind_ prefix. Both blocks gain the /* 0xNN */ value comments the style guide asks for, which is also what makes the two markers checkable by eye. lbfile.c no longer respells HSD_ASSERT/HSD_ASSERTREPORT. Deleting the explicit lbFile_803BA508 declaration lets __FILE__ emit the same string; the only object-level change is that the literal is now anonymous. All 17 affected translation units keep identical loadable section contents.

The It_PKind_Start/It_PKind_Terminate pair the assert recovers already existed in the tree under invented names. it_279C.c's PK_START/PK_END macros are exactly those two values, as are the open-coded start/end locals beside them, the pokemon_spawn_weights array base and bound, the it_803F2ED0 terminator, and the loop that breaks on it. Same for Gm_Player_NumMax, which duplicated PL_SLOT_MAX, and Pl_ItemLog_Terminate, which duplicated plbonuslib's 0x27. Also from the assert strings: grhomerun's four rear*_gobj fields are one bg_gobj[4] array, and gobjs and jobj_gobjs are parts and back. grshrineroute's symbol array is reached as u.map.symbol; Map_GroundVars already carries it at the same offset (gp+108) and nothing else used it. grbigblue's per-entry jobj is the one the assert names, so the stage model's own jobj moves into the block that uses it. All changed translation units keep identical loadable section contents.

Redone with melee-replace-symbols rather than by hand: it walks asm, docs and config as well as src, so it cannot miss a reference the way an ad-hoc pass over src/**/*.{c,h} did. All 35 translation units this branch touches keep loadable section contents identical to upstream/master.

lbmemory.c: all 11 raw __assert sites become HSD_ASSERT/HSD_ASSERTREPORT. The assert strings recover the module's _p() accessor idiom, the free_heap/free_mem/a_arenaLo/a_arenaHi field names, the memp_kouho and p locals, and lbMemFreeToHeap's original name (from its error message). The 0x14F/0x188 string-pool order requires the mgr setup to be a static helper defined above lbMemory_80015320, which also reproduces the target's destructive allocator-base reuse (96.1% -> 96.65%). mnhyaku.c: the allocMenu helper is a decomp invention; flattening it into mnHyaku_8024CB94 lets HSD_ASSERTREPORT match exactly (the local is named user_data, which is why the message says so). grshrineroute.c: the (u32) cast only exists to keep the two symbol[ix] address computations from merging; moving it to the unconstrained mid line makes the assert string-exact.

MarkMcCaskey · 2026-07-26T18:45:27Z

Looks like the lbmemory cases required the use of _p. Perhaps best for a follow up but this could be applied more widely

ribbanya · 2026-07-26T18:46:09Z

Looks like the lbmemory cases required the use of _p. Perhaps best for a follow up but this could be applied more widely

Should be fine in this PR.

it/forward.h: the ItemKind section markers become Doxygen member groups, and the stray blank line before the closing brace is gone. Gr_Homerun_Parts_Max moves into a gr/forward.h enum. With _p() approved, the remaining HSD_ASSERTMSG sites whose messages spell it convert to HSD_ASSERT: cmsnap (recovering the status field and CmSnapStatus_Sleep), lbcardgame, lbcardnew, and lbsnap. lbsnap's four line-410 asserts are one original helper; reconstructing lbSnap_FormatTime(chan, index, text) makes all four string-exact, including the jndex call site. Every touched object keeps its pre-refactor function scores.

MarkMcCaskey

Looks pretty reasonable to me

r-burns

💯

MarkMcCaskey added code-quality ai-assisted Utilizes a LLM to do the heavy lifting labels Jul 26, 2026