Rename all non-ultra functions to snake case

docs/challenge7bug.md

@@ -6,35 +6,35 @@ Thanks to the decompilation project we can easily see all the differences betwee
 
 ## The Root Cause
 
-The root cause is in a function that the decomp project has named bgReset which can be found in [src/game/bg.c](src/game/bg.c). This function is somewhat complex so we'll just cover the important parts here.
+The root cause is in a function that the decomp project has named bg_reset which can be found in [src/game/bg.c](src/game/bg.c). This function is somewhat complex so we'll just cover the important parts here.
 
-bgReset is called while the screen is black after accepting the challenge. It's responsible for loading the background (BG) file for a stage. BG files contain the stage polygon data, room information, a texture list and lighting information among other things. The exact format of the file is not important here, but you just need to know that it contains several sections and some of those sections are compressed, similar to a zip file.
+bg_reset is called while the screen is black after accepting the challenge. It's responsible for loading the background (BG) file for a stage. BG files contain the stage polygon data, room information, a texture list and lighting information among other things. The exact format of the file is not important here, but you just need to know that it contains several sections and some of those sections are compressed, similar to a zip file.
 
 The problem starts when loading the section 2 data, approximately half way into the function. Section 2 is a section of the BG file that is very simple; it's just a compressed list of texture IDs. Let's start by looking at the fix the developers made:
 
     #if VERSION >= VERSION_NTSC_FINAL
         // Fixed version
-        section2 = mempAlloc(inflatedsize + 0x8000, MEMPOOL_STAGE);
+        section2 = memp_alloc(inflatedsize + 0x8000, MEMPOOL_STAGE);
         scratch = (u32)section2 + 0x8000;
     #else
         // Buggy version
-        section2 = mempAlloc(inflatedsize + 0x800, MEMPOOL_STAGE);
+        section2 = memp_alloc(inflatedsize + 0x800, MEMPOOL_STAGE);
         scratch = (u32)section2 + 0x800;
     #endif
 
 This code is allocating space in memory for section 2. It's intentionally overallocating it and setting up a scratch pointer into a later part of that space. We can see that the buggy code was adding an extra 0x800 to the allocation, and their fix was to bump this up to 0x8000. This doesn't give any answers though, so let's see how this allocation is used.
 
     // Load compressed data from ROM to scratch
-	bgLoadFile((u8 *)scratch, section2start + 4, ((section2compsize - 1) | 0xf) + 1);
+	bg_load_file((u8 *)scratch, section2start + 4, ((section2compsize - 1) | 0xf) + 1);
 
-bgLoadFile's arguments are u8 *destination, u32 bgfileoffset, u32 length. It loads part of a BG file from the ROM and writes it to the destination, in this case the scratch pointer. The + 4 in the second argument is just skipping past a section header so it copies the body of the section only. And the last argument is the compressed size, rounded up to the next 16-byte boundary. This rounding is required because the N64 requires loads from the ROM to be in increments of 16 bytes. To put it simply, this function is copying the compressed data from the ROM to the scratch pointer.
+bg_load_file's arguments are u8 *destination, u32 bgfileoffset, u32 length. It loads part of a BG file from the ROM and writes it to the destination, in this case the scratch pointer. The + 4 in the second argument is just skipping past a section header so it copies the body of the section only. And the last argument is the compressed size, rounded up to the next 16-byte boundary. This rounding is required because the N64 requires loads from the ROM to be in increments of 16 bytes. To put it simply, this function is copying the compressed data from the ROM to the scratch pointer.
 
 Hold up. Let's do some math. How much space is available in the scratch? For the buggy version the pointer is 0x800 into the allocation, and the allocation is inflatedsize + 0x800, so we know we have inflatedsize available to place the compressed buffer. For the fixed version the pointer is 0x8000 into the allocation, and the allocation is inflatedsize + 0x8000, so we end up with... also just inflatedsize. The exact same amount of space is available in both versions. All it's done is move the scratch further away from the start of the allocation and increased the allocation accordingly.
 
 	// Inflate section 2 to the start of the buffer
-	bgInflate((u8 *)scratch, section2, section2compsize);
+	bg_inflate((u8 *)scratch, section2, section2compsize);
 
-bgInflate's arguments are u8 *source, u8 *destination, u32 compressedlength. It decompresses the source to the destination, which in this case is decompressing it to the start of the allocation. So maybe the buggy code had the two streams too close to each other, and the inflated stream was overwriting the compressed stream while it was still being decompressed? Sounds plausible. For this to happen, the inflated size would have to be at least 0x800 bytes. Maybe the Warehouse stage (used by Challenge 7) is the only file with a section 2 this big? Let's take a look.
+bg_inflate's arguments are u8 *source, u8 *destination, u32 compressedlength. It decompresses the source to the destination, which in this case is decompressing it to the start of the allocation. So maybe the buggy code had the two streams too close to each other, and the inflated stream was overwriting the compressed stream while it was still being decompressed? Sounds plausible. For this to happen, the inflated size would have to be at least 0x800 bytes. Maybe the Warehouse stage (used by Challenge 7) is the only file with a section 2 this big? Let's take a look.
 
 0x3c. The inflated size of the texture list is just 0x3c bytes. That's less than 1/30th of the allocation. There's no way this is writing over the compressed stream.
 
@@ -73,7 +73,7 @@ The inflatedsize value in the header is 0x3c, which after alignment becomes 0x40
 
 And the section2compsize value in the header is 0x44, which is bigger than the inflated size. That's not surprising - the data is a list of unique texture IDs. There's no repetition and not much data, so this wouldn't compress very well.
 
-    section2 = mempAlloc(inflatedsize + 0x800, MEMPOOL_STAGE);
+    section2 = memp_alloc(inflatedsize + 0x800, MEMPOOL_STAGE);
     scratch = (u32)section2 + 0x800;
 
 Because inflatedsize is 0x40, the allocation is 0x840. And scratch points at 0x800 into the allocation, which is 0x40 from the end.
@@ -86,9 +86,9 @@ Prior to this allocation being made, the memory system's onboard bank has the fo
 
 The allocation is made from the onboard bank, which has just the amount of free space that's needed and is now completely full.
 
-	bgLoadFile((u8 *)scratch, section2start + 4, ((section2compsize - 1) | 0xf) + 1);
+	bg_load_file((u8 *)scratch, section2start + 4, ((section2compsize - 1) | 0xf) + 1);
 
-section2compsize is 0x44. But the value being passed to bgLoadFile is after alignment which is 0x50. So it's copying 0x50 bytes into a buffer of size 0x40. This normally wouldn't be an issue because the space to the right is usually unallocated, but because the allocation is flush against the end of the onboard memory bank the space to the right is in use and 0x10 bytes of it is being inadvertently overwritten.
+section2compsize is 0x44. But the value being passed to bg_load_file is after alignment which is 0x50. So it's copying 0x50 bytes into a buffer of size 0x40. This normally wouldn't be an issue because the space to the right is usually unallocated, but because the allocation is flush against the end of the onboard memory bank the space to the right is in use and 0x10 bytes of it is being inadvertently overwritten.
 
 As stated earlier, the allocations on the right side of the bank are for texture data. The format of this data is not well understood yet, but it contains pointers. The 0x10 overflow is overwriting one of these pointers. Then during gameplay the game is trying to read this value as if it were a pointer, leading to a crash.
 
@@ -102,7 +102,7 @@ A better fix would be to change the allocation size so it's just the aligned inf
 
 The one line tl;dr: When loading the background data for a stage, if the background file contained a specific section that decompressed to a smaller size than the compressed data itself, an overflow would occur due to a miscalculated buffer size. For the overflow to cause any problems the allocation had to be made from the first memory bank and be the exact same size as the remaining space in that bank. If this criteria was met, the overflow would overwrite a pointer in texture data which would cause a crash. The fix that was made increases the allocation to point where it can never be made out of the first bank, therefore avoiding the issue.
 
-But what about the requirement of having 3 players? Why does it not happen with 4? Because each additional player causes an additional player structure to be allocated, and these are made prior to bgReset being called which means they affect the amount of free space in the first bank when bgReset is run. With 1 or 2 players there's going to be more memory available so the issue does not occur. And when playing with 4 players the expansion pak bank starts to be used sooner so the issue does not occur there either. Using 3 players on this particular stage made it total to the exact value that was needed for the onboard bank to be full at this point. In contrast, changing other settings like the number of bots or the weapon loadout would have no effect on this because those are allocated after bgReset is called.
+But what about the requirement of having 3 players? Why does it not happen with 4? Because each additional player causes an additional player structure to be allocated, and these are made prior to bg_reset being called which means they affect the amount of free space in the first bank when bg_reset is run. With 1 or 2 players there's going to be more memory available so the issue does not occur. And when playing with 4 players the expansion pak bank starts to be used sooner so the issue does not occur there either. Using 3 players on this particular stage made it total to the exact value that was needed for the onboard bank to be full at this point. In contrast, changing other settings like the number of bots or the weapon loadout would have no effect on this because those are allocated after bg_reset is called.
 
 Just to be clear, this situation was incredibly unlucky. Adding or deleting a couple of lines of code anywhere else in the game would have made this a non-issue because it would change the heap size (the heap is sized based on what's left over after code and data). And even if the memory system is in the danger territory, there's a 1 in 455 chance that any number of player structure allocations would line it up in a way that can trigger the bug (each player structure is 7280 bytes and there's a 16 byte window where the bug can occur).
 

docs/piracychecks.md

@@ -24,7 +24,7 @@ The decomp project wraps all decompiled piracy checks in `#if PIRACYCHECKS` stat
 
 **Payload:** Writes 40 bytes of 0xff to 0x80095210. This appears to be related to sound effects but has no obvious effect.
 
-### cheatMenuHandleDialog
+### cheat_menu_handle_dialog
 
 **When Called:** When the Cheats menu dialog is opened.
 
@@ -34,7 +34,7 @@ The decomp project wraps all decompiled piracy checks in `#if PIRACYCHECKS` stat
 
 ---
 
-### doorFinishClose
+### door_finish_close
 
 **When Called:** When a door finishes closing.
 
@@ -42,25 +42,25 @@ The decomp project wraps all decompiled piracy checks in `#if PIRACYCHECKS` stat
 
 **Payload:** Rewrites the start of `func0f08f968` so it immediately returns false. This makes it impossible to open doors.
 
-### botPickupProp
+### bot_pickup_prop
 
 **When Called:** When a simulant in multiplayer picks up an item.
 
-**What It Checks:** Checksums `doorFinishClose` to make sure it hasn't been modified.
+**What It Checks:** Checksums `door_finish_close` to make sure it hasn't been modified.
 
-**Payload:** Rewrites the start of `chrCheckCanSeeTarget` so it immediately returns true. This makes all guards able to see Jo through walls.
+**Payload:** Rewrites the start of `chr_check_can_see_target` so it immediately returns true. This makes all guards able to see Jo through walls.
 
-### chrUncloak
+### chr_uncloak
 
 **When Called:** When a guard uncloaks.
 
-**What It Checks:** Checksums `botPickupProp` to make sure it hasn't been modified.
+**What It Checks:** Checksums `bot_pickup_prop` to make sure it hasn't been modified.
 
-**Payload:** Disables the ability for the player and other characters to go up or down slopes. This is done by nopping the `jr ra` instruction in `cdFindGroundInfoAtCyl`, which causes it to flow into the following function, which unconditionally returns false and only exists for this purpose.
+**Payload:** Disables the ability for the player and other characters to go up or down slopes. This is done by nopping the `jr ra` instruction in `cd_find_ground_info_at_cyl`, which causes it to flow into the following function, which unconditionally returns false and only exists for this purpose.
 
 ---
 
-### chrsCheckForNoise
+### chrs_check_for_noise
 
 **When Called:** When a guard hears you.
 
@@ -70,7 +70,7 @@ The decomp project wraps all decompiled piracy checks in `#if PIRACYCHECKS` stat
 
 ---
 
-### lvGetSlowMotionType
+### lv_get_slow_motion_type
 
 **When Called:** On each tick when unpaused.
 
@@ -78,25 +78,25 @@ The decomp project wraps all decompiled piracy checks in `#if PIRACYCHECKS` stat
 
 **Payload:** Corrupts the rspboot microcode, causing a crash.
 
-### lvReset
+### lv_reset
 
 **When Called:** When loading any stage (including title screen).
 
-**What It Checks:** Checksums `lvGetSlowMotionType` to make sure it hasn't been modified.
+**What It Checks:** Checksums `lv_get_slow_motion_type` to make sure it hasn't been modified.
 
 **Payload:** Writes a filesystem terminator file to the start of EEPROM. This disables the cartridge's save data, removing the ability to read existing files and create new ones.
 
-### bodyAllocateEyespy
+### body_allocate_eyespy
 
 **When Called:** When loading any stage that uses the eyespy.
 
-**What It Checks:** Checksums `lvReset` to make sure it hasn't been modified.
+**What It Checks:** Checksums `lv_reset` to make sure it hasn't been modified.
 
-**Payload:** Nops `_memaFree` entirely, so any time that function is called it'll flow into the following function, which just returns. The effect this has is that the system is unable to free individual mema allocations which makes it unable to load rooms as you move throughout the level.
+**Payload:** Nops `_mema_free` entirely, so any time that function is called it'll flow into the following function, which just returns. The effect this has is that the system is unable to free individual mema allocations which makes it unable to load rooms as you move throughout the level.
 
 ---
 
-### bgReset
+### bg_reset
 
 **When Called:** When loading a normal stage (eg. CI Training).
 
@@ -104,53 +104,53 @@ The decomp project wraps all decompiled piracy checks in `#if PIRACYCHECKS` stat
 
 **Payload:** Copies 64 bytes from a random location in ROM to a random location in RAM.
 
-### chrConsiderGrenadeThrow
+### chr_consider_grenade_throw
 
 **When Called:** When a chr decides to throw a grenade.
 
-**What It Checks:** Checksums `bgReset` to make sure it hasn't been modified.
+**What It Checks:** Checksums `bg_reset` to make sure it hasn't been modified.
 
 **Payload:** Surrounds the player in infinite explosions.
 
 ---
 
-### tagsReset
+### tags_reset
 
 **When Called:** When loading a normal stage (eg. CI Training).
 
-**What It Checks:** Calls `mtxGetObfuscatedRomBase` to get the value of `osRomBase` then compares it with a known value.
+**What It Checks:** Calls `mtx_get_obfuscated_rom_base` to get the value of `osRomBase` then compares it with a known value.
 
 **Payload:** Copies 4KB from a random location in ROM to a random location in RAM.
 
-### bgunTickGunLoad
+### bgun_tick_gun_load
 
 **When Called:** When equipping any weapon.
 
-**What It Checks:** Checksums `tagsReset` to make sure it hasn't been modified.
+**What It Checks:** Checksums `tags_reset` to make sure it hasn't been modified.
 
-**Payload:** Corrupts `tagsReset` by writing 4 bytes of 0xff.
+**Payload:** Corrupts `tags_reset` by writing 4 bytes of 0xff.
 
 ---
 
-### menuTickTimers
+### menu_tick_timers
 
 **When Called:** On every frame except credits.
 
-**What It Checks:** Checksums `mtxGetObfuscatedRomBase` to make sure it hasn't been modified.
+**What It Checks:** Checksums `mtx_get_obfuscated_rom_base` to make sure it hasn't been modified.
 
-**Payload:** Corrupts `bgReset` by writing 16 bytes of 0x12 to a random address within that function.
+**Payload:** Corrupts `bg_reset` by writing 16 bytes of 0x12 to a random address within that function.
 
-### bgTickCounter
+### bg_tick_counter
 
 **When Called:** On every frame.
 
-**What It Checks:** Checksums `menuTickTimers` to make sure it hasn't been modified.
+**What It Checks:** Checksums `menu_tick_timers` to make sure it hasn't been modified.
 
-**Payload:** Corrupts `bgBuildTables` by adding a fixed amount to 16 bytes within that function.
+**Payload:** Corrupts `bg_build_tables` by adding a fixed amount to 16 bytes within that function.
 
 ---
 
-### glassDestroy
+### glass_destroy
 
 **When Called:** When the player breaks glass.
 
@@ -158,10 +158,10 @@ The decomp project wraps all decompiled piracy checks in `#if PIRACYCHECKS` stat
 
 **Payload:** Sets the audio frequency to a high value which makes everyone sound like chipmunks.
 
-### explosionAlertChrs
+### explosion_alert_chrs
 
 **When Called:** When there's an explosion.
 
-**What It Checks:** Checksums `glassDestroy` to make sure it hasn't been modified.
+**What It Checks:** Checksums `glass_destroy` to make sure it hasn't been modified.
 
 **Payload:** Makes all explosions huge.