Previous: [Beginning decompilation: the Init function and the Actor struct](beginning_decomp.md)
## Now what?
Following the scheme we gave last time, we have three options:
-`func_80A87BEC`
-`func_80A87C30`
-`func_80A87F44`
Another option is to look at `Destroy`, which for smaller actors can often be done straight after Init, since it usually just removes colliders and deallocates dynapoly. However, glancing at the three given functions' assembly, there is an obvious standout:
```MIPS
glabel func_80A87F44
/* 00744 80A87F44 AFA40000 */ sw $a0, 0x0000($sp)
/* 00748 80A87F48 03E00008 */ jr $ra
/* 0074C 80A87F4C AFA50004 */ sw $a1, 0x0004($sp)
```
This is a classic "function with two arguments that does nothing". So we can simply comment out the appropriate pragma and put
Destroy will be a dead end, but we might as well do it now. Remaking the context and using mips2c on it (main 4 function, so change the prototype to use `EnJj* this`!) gives
Again remember to return the first argument to `Actor* this` and put `EnJj* this = THIS;` in the function body. Based on what we know about this actor already, we expect this to be another switch. Rearranging it as such, and removing the likely fake `temp_v0` gives
Using `./diff.py -mwo3 EnJj_Destroy` shows that this matches already, but it seems like the temp usage should be more consistent. A little experimentation shows that
also matches, with no need for the `GlobalContext*` temp.
## Action Functions
### `func_80A87BEC`
Of the two functions we have available, `func_80A87BEC` is shorter, so we do that next. Since we haven't changed any types or header file information, there is no need to remake the context. mips2c gives
We see that this function just sets another action function when Link is close enough to the actor. All we have to do to this is remove the `&`, and prototype `func_80A87B9C` to be an action function. Notably, this time it is not used before it is defined, so we don't need a prototype at the top: putting a placeholder one in at the function position will do, i.e. our total edits this time are
We can now either follow this chain of action functions down, or start with the other one. We will go down in this case: it is usually easier to keep track that way.
### `func_80A87B9C`
We can remake the context, but it's simpler to just stick the function prototype we made at the bottom of the context. Either way, mips2c gives us
Here's a new variable for our actor struct! Don't be deceived by the `s32` cast in the comparison: mips2c always does that if it's not sure. The reliable one is the `s16` cast lower down. (An `s32` doesn't fit in the space we have anyway). So the actor struct is now
```C
typedef struct EnJj {
/* 0x0000 */ DynaPolyActor dyna;
/* 0x0164 */ SkelAnime skelAnime;
/* 0x01A8 */ Vec3s jointTable[22];
/* 0x022C */ Vec3s morphTable[22];
/* 0x02B0 */ ColliderCylinder collider;
/* 0x02FC */ EnJjActionFunc actionFunc;
/* 0x0300 */ Actor* childActor;
/* 0x0304 */ char unk_304[0x4];
/* 0x0308 */ s16 unk_308;
/* 0x030A */ s16 unk_30A;
/* 0x030C */ char unk_30C[0x2];
/* 0x030E */ s8 unk_30E;
/* 0x030F */ s8 unk_30F;
/* 0x0310 */ s8 unk_310;
/* 0x0311 */ s8 unk_311;
/* 0x0312 */ char unk_312[0x2];
} EnJj; // size = 0x0314
```
We can eliminate the temp since it's use in a simple way one after the other. `this->unk308 = (s16) (this->unk308 - 0x66);` can be written as `this->unk308 -= 0x66;`.
In the `func_8003EBF8` we see that we should have at least made `this->childActor` a `DynaPolyActor*`, so that the last argument is its `bgId`. To avoid compiler warnings, we also need to cast the `Actor_SpawnAsChild` as such in Init,
It's not obvious why this doesn't match: the branching is correct, but it's loading stuff in the wrong order. Now is therefore a good time to introduce the permuter.
### The permuter
The permuter is a useful program for when you run out of ideas: it assigns a function a score based on how much it differs from the original code, then does random stuff that is known to improve matching, keeping ones that give better scores. It is not going to fix your control flow, but it does reasonably well on instruction differences and regalloc. It also has an option to worry about stack differences as well (`--stack-diffs`).
To use the permuter, clone the decomp-permuter repo from the link given in Discord. First, import the C file and MIPS of the function to compare using
```sh
./import.py <path_to_c><path_to_func_name.s>
```
It will put it in a subdirectory of `nonmatchings`. You then run
```sh
./permuter.py nonmatchings/<function_name>/
```
to produce suggestions. There are various arguments that can be used, of which the most important initially is `-j`: `-jN` tells it to use `N` CPU threads.
Suggestions are saved in the directory it imported the function into.
Hooray, that worked. The function now matches (as you can check by running `make -j`). In this case we are lucky and got a couple of 0s almost immediately. This will often not be the case, and you may have to go through multiple iterations to improve things. Or you get more ideas for what to do without the permuter necessarily doing everything for you.
However, the hex values look a bit strange, and it turns out the decimal equivalents look less strange, so it's a good idea to translate them:
If you know anything about this game, this is obviously the function that begins the process of the swallowing Link cutscene. Performing minor cleanups reduces us to
There are three things left to do to this function:
- prototype the new action function, `func_80A87CEC`. This one is used before its definition, so needs to be prototyped at the top of the file.
- extern `D_80A88CF0`, and since the arguments of `Math_Vec3f_DistXZ` are `Vec3f`s, convert it to floats. To do float conversion, either use an online converter, or get an extension for VSCode that can do it. The data becomes
- replace the mysterious `globalCtx->unk1C44 + 0x24`. The first part is so common that most people on decomp know it by heart: it is the location of the Player actor. `+ 0x24` is obviously an offset that leats to a `Vec3f`, and if you look in the actor struct, you find that this is the location of `world.pos`. To use `Player`, we put `Player* player = GET_PLAYER(globalCtx)` at the top of the function.
**NOTE:** mips_to_c will now output something like `&globalCtx->actorCtx.actorLists[2].head` for the Player pointer instead: this makes a lot more sense, but is not so easy to spot in the ASM without the characteristic `1C44`.
One issue we have swept under the carpet thus far is what `unk_30C` is: we still have it as padding. For this we have to look at the MIPS, since mips2c hasn't told us. Scanning through the file, we find
```MIPS
/* 00498 80A87C98 A60E030C */ sh $t6, 0x030C($s0) ## 0000030C
```
which tells us that `unk_30C` is an `s16`, filling another gap in the struct:
```C
typedef struct EnJj {
/* 0x0000 */ DynaPolyActor dyna;
/* 0x0164 */ SkelAnime skelAnime;
/* 0x01A8 */ Vec3s jointTable[22];
/* 0x022C */ Vec3s morphTable[22];
/* 0x02B0 */ ColliderCylinder collider;
/* 0x02FC */ EnJjActionFunc actionFunc;
/* 0x0300 */ DynaPolyActor* childActor;
/* 0x0304 */ char unk_304[0x4];
/* 0x0308 */ s16 unk_308;
/* 0x030A */ s16 unk_30A;
/* 0x030C */ s16 unk_30C;
/* 0x030E */ s8 unk_30E;
/* 0x030F */ s8 unk_30F;
/* 0x0310 */ s8 unk_310;
/* 0x0311 */ s8 unk_311;
/* 0x0312 */ char unk_312[0x2];
} EnJj; // size = 0x0314
```
The diff now looks fine for this function, but it gives compiler warnings about `CollisionCheck_SetOC(globalCtx, &globalCtx->colChkCtx, &this->collider);`: the last argument is the wrong type: we need to give it `&this->collider.base` instead, which points to the same address, but is the right type. So the matching function is
-`func_80A87EF0` is another action function. Again it is defined below, so needs a prototype at the top.
- We have another unknown symbol, `D_80A88164`. This is the massive chunk of data that makes up the cutscene (of Jabu swallowing Link). We will worry about it when we have decompiled the rest of the actor. For now just extern it as `UNK_TYPE`.
- We can remove the casts from `(u8)1U` and just leave `1`.
-`gSaveContext.unkEDA` we have dealt with before: it is `gSaveContext.eventChkInf[3]`. This is a flag-setting function; it can be written more compactly as
```C
gSaveContext.unkEDA |= 0x400
```
- The last function is an audio function: we can look up the argument in `sfx.h` and find it is `NA_SE_SY_CORRECT_CHIME`
It remains to work out which, if any, of the temps are real. Based on our previous experience, we expect `temp_v1` to be real. `sp1C` is unused and hence unlikely to be real. `temp_v0` is only used in the short if and so probably not real. Checking the diff shows that our suspicions were correct:
matches, but generates a complier warning for `func_8005B1A4`, which it can't find. To fix this, add it to `functions.h`, in as near as possible the correct position in numerical order. Some detective work with VSCode's Search shows that this function lives in `z_camera.c`, and its prototype is `s16 func_8005B1A4(Camera* camera)`, so add this line to `functions.h` at the bottom of the camera functions part.
Lastly, we prefer to limit use of early `return`s, and use `else`s instead if possible. That applies here: the function can be rewritten as
and still match. (Early `return`s are used after an `Actor_Kill` and in a few other situations, but generally avoided for `else`s elsewhere if possible. Talking of which...)
Now we're a bit stuck: this tells us that `this->unk_304` is an `Actor*`, but we know nothing else about it. So just make it an actor for the time being. As before, `this->unk_304` is a pointer, so we should compare `temp_a0` to `NULL`.
We also find in the MIPS
```MIPS
lhu $v0, 0x030A($a0)
```
which at last tells us that `unk_30A` is actually a `u16`. We can now eliminate `temp_v0`, and replace the ` == 0` by a `!(...)`, which leaves
although we are as yet none the wiser as to which actor `unk_304` actually points to.
Again we have run out of action functions. The rules suggest that we now look at Update.
## Update
Update runs every frame and updates the properties of the actor, and usually runs the action functions once per frame. As before we change the first argument to `EnJj* this` to get it to use our actor's struct. mips2c gives
This has several problems: firstly, the action function is called with the wrong argument. We should thus be suspicious of previous functions this actor calls, and decompile them mith mips2c without context if necessary, if only to find out how many arguments they have. We find that `func_80A87D94` definitely takes `EnJj* this, GlobalContext* globalCtx` as arguments. Again, put this prototype at the function location above to avoid compiler warnings.
`unk40` of an array of `Vec3s`s is `0x40 = 0x6 * 0xA + 0x4`, so is actually `this->skelAnime.jointTable[10].z`
Lastly, what is `globalCtx->unk1D94`? It is at `globalCtx->csCtx + 0x30`, or `globalCtx->csCtx.npcActions + 0x8`, which is `globalCtx->csCtx.npcActions[2]` since this is an array of pointers. Hence it is a pointer, and so should be compared to `NULL`. Looking up the sfx Id again, we end up with
From this we can read off that `unk_30F` is a `u8`, `unk_30E` is a `u8`, `unk_310` is a `u8`, and `unk_311` is a `u8`. Giving mips2c new context and trying again,
and all the weird casts are gone. Eliminating the temps, replacing the hex, discarding pointless definitions, and replacing early `return`s by `else`s, we end up with
You will also find that the permuter is essentially useless here: most solutions it offers look very fake. But there's already something a bit weird in here: why does it do `this->unk_30E & 0xFF` explicitly in the comparison? It turns out if you remove this, the function matches: the compiler does this calculation automatically when doing comparisons with `u8`s anyway.
Firstly, we are now comparing with the value of `globalCtx->unk1D94`, not just using a pointer, so we need the first thing in `globalCtx->csCtx.npcActions[2]`. This turns out to be `globalCtx->csCtx.npcActions[2]->action`.
The if structure here is another classic indicator of a switch: nested, with the same variable compared multiple times. If you were to diff this as-is, you would find that the code is in completely the wrong order. Reading how the ifs work, we see that if `temp_v0` is `1`, it executes the outermost else block, if it is `2`, the middle, if `3`, the innermost, and if it is anything else, the contents of the innermost if. Hence this becomes
(notice that this time we need a `default` to deal with the innermost if contents). If you try to replace `0x206D` in the `Audio_PlayActorSound2`, you will find there is no such sfxId in the list: this is because some sound effects have an extra offset of `0x800` to do with setting flags. Adding `0x800` to the sfxId shows that this sound effect is `NA_SE_EV_JABJAB_BREATHE`. To correct this to the id in the function, we have a macro `SFX_FLAG`, and it should therefore be
As usual, most of the remaining temps look fake. The only one that does not is possibly `phi_v1`. However, the way in which they are used here makes it hard to tell if they are fake, and if so, how to replace them. I encourage you to try this yourself, with the aid of the diff script; the final, matching result, with other cleanup, is hidden below
With that, the last remaining function is `EnJj_Draw`. Draw functions have an extra layer of macroing that is required, so we shall cover them separately.
