Found this function in RIC, it's very similar to the one in DRA, which
is nice.
Patterns of zeros in the rodata indicated that this should be the start
of a new file, which also makes sense since CreateEntFactoryFromEntity
comes with it.
RIC has its own array of blueprints, I'll probably add that to the splat
later on but that seems like material for a separate PR. For now we
focus on the entity function.
This appears to control Richter using an item crash.
We add an enum for the subweapon IDs. This should be useful in other
places but for now makes the switch much more readable.
Loops in EntityAlucard and UpdateEntityRichter indicate that these 16
elements of g_Player are an array of 16 values. The known ones appear to
be player status effect timers.
While those functions have a long-standing issue preventing them from
matching, they provide enough evidence to suggest that we should make
this change. I have changed all existing uses, and changed the symbols
so that any remaining non-decompiled functions will use the proper form.
It would probably be beneficial to make an enum for the 16 elements of
this new array, but I'll hold off on that for now until we can identify
the purpose of more of them.
---------
Co-authored-by: Luciano Ciccariello <Xeeynamo@users.noreply.github.com>
This one has a corresponding DRA function. I believe these relate to the
fading shadows that appear behind the player, but I'm not confident
enough about that to go ahead and rename things. For now, it's
decompiled at least.
I had the suspect menu strings were part of a string of arrays since a
while. So I come up with a `g_MenuStr[]`, which removed the need of many
fake symbols. I did not yet come with a bunch of `#define`s to resolve
some magic numbers. Luckily you can guess which string is which from the
context of the function. Resolving the HD strings might give me more
clues on how to come up with good defines.
I decided to go for the route of having a `lang_us.c` and a `lang_jp.c`
for the sake of maintaining things simpler. This is also due to the fact
the HD build orders the japanese strings in a different way, making
things a bit harder. Any modder can just come up with their own
`lang_XX.c`, translate the menu in the language they want and just link
it, without touching the original files.
For the test app things were tricky. I've been debating how to implement
it as SOTN-encoded strings are not pre-encoded on compilation time like
the PSX toolchain does. Instead they are burnt into the executable as
UTF-8 characters. Instead of encoding them on-the-fly in `MenuDrawChar`,
I decided to re-use `AnsiToSotnMenuString` to pre-encode all the strings
and re-route each `g_MenuStr` pointer to the converted strings. This is
both efficient at runtime and prevents to modify the existing matching
code. Now the menu looks amazing.
I thought this was going to be much harder to do. I ended up doing
incremental steps in each commit to ensure having healthy checkpoints in
case things were going south. The tool ` ./tools/sotn-str.py parse
disks/us/DRA.BIN 0xOFFSET` saved me sooo much time. But we need to tweak
it to deal with Japanese characters.
This one was very complicated. It has 4 different switches in it, all
with their own rodata!
It also interacts with a lot of different parts of the game, including
various unknown members of structs like g_Player, so it might be a
useful resource for starting to understand what some of that stuff is.
One goto left in it that I couldn't get rid of, the jump is between two
unexpected locations that seem very different from each other.
Scratch here if anyone would like to attempt improvements:
https://decomp.me/scratch/0k84D
Not totally sure what this one is for, but it's connected to saving the
game.
D_801379C8.vy is very heavily used, I'm not sure what the purpose of
that SVECTOR is. But anyway, it matches.
Mostly a proof of concept to extract resources from `BIN/F_GAME.BIN`.
There are tons of hacks to make this work:
* Tweaked png2s to decode binaries into PNGs
* Add a `config/gfx.game.json` that describes how to extract different
sprites out of a binary file
* Coded the palette location by reading the `clut` parameter in the
source code. A clut value of `0x196` corresponds to `"palette": "0x96"`
or `"palette": 150`.
* Coded the bits per pixel for a couple of 8-bit images
* Coded the palette location of those graphics that expects the palette
from DRA.BIN instead of F_GAME.BIN
* For all the undiscovered palette, I called the files `unk` and
defaulted their palette to grey
The expectation is that sprites will slowly be documented and added into
`config/gfx.game.json` by modders and enthusiasts. This does not yet
pack back these PNGs into the binary file. The way these assets are
extracted is probably not final or perfect, but we have to start from
somewhere.
The next subweapon entity. This is for the lightning subweapon. Happy to
take name suggestions but for now we have a comment on the function so
doesn't really matter if we name it immediately.
Remove a bunch of fake symbols that were overlapping with `D_80097488`.
I created a new struct called `Pos`. Since it is very similar to
`Camera` I placed them close-by as in future we might want to merge
them.
I created some symbol hints like `D_8009748A_x_i_hi` as there are a few
functions that still use them.
Nothing super special here, just another RIC function. I believe this is
a helper function for Richter's state machine, since it switches on
PLAYER.step_s.
This was done by comparing the src/servant/tt_000/spriteparts.c file to
the data I found in RIC 170AC.data.s. I then wrote a Python script to
take the data in the data file and output a C file which has all the
same formatting as that existing spriteparts.c file. This is a big chunk
of the .data section for RIC, so it's nice to have it pulled out to a C
file now.
The `const int PRIM = -5;` is very weird, I am not sure what's going on
there. The struct makes sense on the surrounding functions. I am not
even sure if the function is ever called as it involves the use of a
cutscene dialogue, never seen in the SEL overlay.
Mostly a duplicate of a DRA function but enough different logic to be
fairly different.
Interesting that this uses the "dead player" string which matches HD but
is different from in US DRA.
There are 3 strings in DRA that were being left in rodata and printed
with `FntPrint(&whatever)`, this fixes two of them (the third will first
require other stuff in rodata before it to be extracted).
I also discovered that HD uses a different string in one of those spots
which was a big surprise! Weirdly, US has a string written in Japanese
(though using Latin characters) and HD has a string written in English.
The function that keeps track of if Richter has pressed down-up-cross to
do a High Jump move.
This is the first time we have a button combo being tracked outside of
DRA, so I moved the ButtonComboState struct from dra.h to game.h.
When the input succeeds, a function is called, so I've named that
function DoHighJump and also made general changes to that function to
bring it up to date with our current code standards (like FIX()).
I think those are the main highlights!
More data importing and more documentation as data is providing more
context.
I also found a good amount of "fake" symbols, like in `DrawHud` and
`DrawHudSubweapon`.
Quite big PR that aims to document most of `menu.c` and make the code
more portable.
I had to declare `u16* func_80106A28(u32 arg0, u16 kind);` on the top of
`main.c`. Without it the compiler assumes it returns an `int`, which is
not big enough for 64-bit pointers. It took me a while to realise that
was the cause of the crash. We will have more of them in the future.
EDIT: Ignore the commit `******* FIX`. It was meant to be squashed for a
clearer commit history but I forgot to do it.
Highlights:
* Move `MenuContextInit` into `main.c`
* Document `Unkstruct_800A2D98` as `EquipMenuHelper`
* Use shorter global variable names
* Change `D_801375CC` type on Saturn
* `g_MenuInit` is different between JP and US due to different window
arrangement and text width differences
As per title. I also added a new tool called `dirt_patch`. As I
mentioned in our Discord server there is some left-over data from
previous dev builds in DRA.BIN and potentially elsewhere too. The tool
uses the new file `config/dirt.us.json` which stores a list of patches
to avoid crazy hacks and `#ifdef` for the sake of getting a match. I
hope this tool will not be abused.
Extract the memory card icons out of DRA.BIN and SEL.BIN.
I also took the opportunity to extract `g_MemcardPortMask` into its own
source file `save_mgr_pre.c`. Unfortunately this is required to keep the
original data order. This suggests that most likely the icon palette and
bitmap were baked into the original `save_mgr.c` as byte arrays. I
decided to take a different approach and extracted them as PNGs for
better moddability.
I had to spin-off Splat into a new fork due to some breaking changes on
0.18.0.
This is actually the last function in RIC (by which I mean "There are no
functions in RIC which are located later in the ROM than this one"), so
that's neat. There was a mistake with the splat, where the nop in the
delay slot for the return instruction of this function was being treated
as the first 4 bytes of SBSS, rather than as the final instruction of
this function, but luckily I worked that out and fixed it in the splat.
This is extremely similar to the one in DRA. However, it stores to 0xB0
in the entity. This is interesting, I've seen that being a significant
address in other RIC functions too. I added it to the extension that was
used for this entity in DRA.
Next entity function in the same file I've been chewing through. Only
one left in here!
There's a really weird block with a bunch of trig functions that I can't
make sense of. Might be doing some kind of weird 3D transformation? Not
sure, but it matches. Would have never gotten it without the permuter.
Scratch here if you want to play with anything, especially if you can
remove the temp_a1 being used in the block of trig.
https://decomp.me/scratch/xdNx7
temp_v0 is also a bit funny, not sure why it would use lhu to load a
variable which is explicitly an s16 and we're negating, but whatever.
Everything matches so submitting now.
Refactoring with no functional changes. `5087C.c` is currently a bloated
file that contains both a game state machine to handle the gameplay and
the menu handler.
This PR splits the menu handler into its own new file `menu.c`.
The way I detected the file split is the amount of `.rodata` strings
defined between the new last function of `5087C.c` and everything else
related to the menu.
This is a pretty big function, and I'll admit that my code for it is
pretty low quality. But it matches. Scratch is here if you'd like to
make improvements:
https://decomp.me/scratch/xok2Z
A lot of the things that I tried to remove the gotos and temp variables
didn't work.
The entity extension for this uses f32 variables. Those are defined in
`game.h`, but `game.h` is not accessible to `entity.h`, so I moved the
definition of f32 to `types.h` which is accessible in all places and
overall seems like a better place for it anyway.
After finding a copy of GetFreeDraEntity in RIC, I decided to rename the
function (in both DRA and RIC) to GetFreeEntity. Similarly, the function
right after it is GetFreeEntityReverse (since its logic runs in
reverse).
I also found CreateEntFactoryFromEntity in RIC, so I renamed that to
match the version in DRA. We will need to go through and use the FACTORY
macro for its uses, but I'm not going to tackle that quite yet.
Seems there is a lot of work to be done making RIC catch up to DRA,
we'll see if I keep working on that moving forward, it's neat to find
the ways the two match.
Another entity function in RIC, continuing to work through these to
remove the rodata padding.
Still no knowledge of what this function is for but once I get these
done I'll focus on getting the entity factory system working in RIC like
we have it in DRA, I've found a lot of the important functions already.
This appears to be an entity updating function unique to RIC. I do not
know what it is for yet.
Main reason for doing it is that it has rodata and the file it is in
currently requires a rodata padding to be present. Having this one
working and parsing the rodata into it means we can move the padding,
and once we finish everything in this file, we will be able to remove
the padding entirely.
As mentioned in the previous PR, this removes a few rodata_padding
variables by simply un-splitting the files involved here.
There are two more paddings I found in other places; I will look into
those next but don't want to do too much file reshuffling in one PR.
This PR takes 3 files and merges them into one, removing the padding in
the process.
Another function, not sure what it really does besides messing with a
bunch of primitives.
Some clues in the rodata (particularly 00 00 00 00 bytes) indicated that
we should merge these C files, so I did.
An issue with maspsx was discovered in the process of decompiling this
one. I submitted a PR to maspsx which was accepted, and therefore I also
updated maspsx in this PR in order to capture the fix.
For some reason this was included as:
```
#if defined(VERSION_US)
INCLUDE_ASM("asm/us/dra/nonmatchings/627C4", func_801028AC);
#elif defined(VERSION_HD)
INCLUDE_ASM("asm/hd/dra/nonmatchings/627C4", func_801028AC);
#endif
```
But that wasn't actually needed, the functions are identical. Not sure
why it was that way but it's fixed now.
- Main focus is an update of func_8015D020. That had a TODO to remove a
variable, which I have now removed. I also used enums for player steps,
and changed the speeds to use FIX(). I also cleaned up the control flow
to remove the goto.
- While working on that function, I noticed that it calls an unnamed
function. The duplicates list at
https://raw.githubusercontent.com/Xeeynamo/sotn-decomp/gh-duplicates/duplicates.txt
indicates that SetSpeedX was missing from RIC. I added that in now and
adjusted all uses in RIC, including the ones in func_8015D020.
That's about it; these are two different changes but given that we're
just changing the format of them, I'm assuming this will not be an issue
to have them in one PR.
Had to add PlayerSteps into RIC since we're using enums for Player.step
and SetPlayerStep. I think maybe these should be moved out of dra.h and
into somewhere more generic, but for now copying it over works well
enough. If there are steps that are reused between Alucard and Richter
for mutually exclusive moves (ie, step N is an Alucard-specific move for
Alucard and a Richter-specific move for Richter) then we will need to
maintain separate lists. This works for now.
Handles a bunch of the internal logic of saving the game. There is a
small group of functions that deal with the process of saving; once we
get them all decompiled it makes sense to rename them once we understand
the individual steps involved.
This appears to generate the menu that appears when you use a save room.
This is obvious by the strings present in the many calls to
`func_800F9D88`.
Appears to be closely related to some other functions which may make the
logic a bit more obvious. Cool to have this working though, might be
useful for some types of research into how saving works. I wonder if all
these messages have actually showed up before for real players.
Many of the strings feature Shift-JIS characters (specifically, the
periods, question marks, and the 0, 1, and 2 numerals). Big thank you to
Sozud for helping me understand those weird bytes in the strings!
Parsing rodata for strings is pretty fun!
Happy to change the name of this function, but wanted to at least
replace the numerical placeholder.
Follow-up to #797 by decompiling the MAD counterpart and share all the
functions within the same file. I renamed the function as
`UnkPrimHelper` as I do not know what it does.
This is an old one that I had almost-matching, I set the permuter
running for a few hours while I was doing something else and came back
to a match!
It appears to be responsible for managing some type of alternate player
status. This is evidenced by the
```
if (!(g_Player.unk0C & 0x10000)) {
DestroyEntity(entity);
return;
}
```
That variable holds statuses like PLAYER_STATUS_STONE,
PLAYER_STATUS_POISON, PLAYER_STATUS_CURSE. We do not have 0x10000
identified yet. The entity is made by an entity factory which currently
does not have any decompiled calls, so hopefully when that happens, we
will know exactly what this entity is responsible for.
Not sure what this one does, but it's in all the overlays.
I couldn't get `mad` to work, since it gives errors when I reference
functions like `RotMatrixZ` (which isn't even one of the new `gte_`
defines). I figured we can leave that for later since `mad` is its own
weird thing.
I also found an issue with the splat for `no3` where some boundaries
were being drawn in the wrong places; this is now corrected which allows
the function to work in that location.
The function takes a primitive as its argument and does some stuff with
it. I don't know what it actually does. It appears that the different
overlays use it in different places (and some overlays don't call it at
all). The function loads an external SVECTOR from a variable, which I
added to the `.h` file for each overlay. The vector is in a weird
location in some overlays, especially in no3 where it appears to be the
last few bytes of the rodata.
Anyway, happy to take naming ideas for either the function or the
SVECTOR that it loads. And I imagine we'll end up renaming those sp
variables.
We have a lot of casts and especially LOH calls in this one. Maybe we're
interpreting the primitive wrong. It doesn't match up with the FakePrim
we've been using lately, so I don't know what to make of this.
I think those are the main highlights! There aren't many functions that
use the GTE and it's nice that there is so much duplication so we can
deal with a lot of them all at once. I expect a lot of changes on this
one so please don't hold back :)
Here's the scratch for one instance of the function, in case you'd like
to play around with anything: https://decomp.me/scratch/N5RR5
This was a weird one. This uses the Geometry Transformation Engine,
which I don't think any previous function has done. It was confusing to
get it working but it seems to work.
I think I put the macros in the right place, but let me know if not.
Overall this is the first PR of this type, so please do make sure
everything looks good so we don't pollute the repo. If this goes in, I
might try to find more of the gte functions so we can crack those all
away. Making this work was difficult and required a lot of SDK research,
so I'm hoping I can do more functions before the knowledge disappears
from my brain :)
---------
Co-authored-by: sozud <122322823+sozud@users.noreply.github.com>
This is a cool one to have working.
It uses the FakePrim struct which we discovered a little while ago. It
is only the second one we've found to use it, and happily, the fields in
it match.
I'm wondering if FakePrim might represent a single pixel. The Vibhuti is
a set of white pixels that get thrown out, and therefore they don't need
a full set of multiple r,g,b values. Once we find a couple more users
for FakePrim, we might be able to give it a proper name. We may need to
consider making Primitive a union. But that's a decision for the future.
Very interestingly, this function and `func_8011E4BC` (the previous
function which used FakePrim) both call func_800EDB58 in the manner that
other functions call AllocPrimitives. This function seems to allocate
FakePrims.
Second last function of TT_000 decompiled.
* Add `printf '\x00' | dd of=$@ bs=1 seek=40959 count=1 conv=notrunc` to
ensure the output binary is 40960 bytes long (useful once BSS is
migrated)
* Add `$(BUILD_DIR)/tt_%.ld` so every time the splat YAML changes, doing
`make tt_000` triggers the extract again
* All the variables from BSS are now in `10E8` and in order. Removing
`extern` from will properly create the BSS section
* Fix 7A4D0.c to not use the familiar entity extension
* Delete the fake symbols `D_80170660` and `D_8017065C`
* Replace all the instances of `ext.generic` as `ext.bat` when possible
NOTES:
* For `SquareRoot12((...) << 0xC) >> 0xC;` I was not able to use the
macro `FLT`
*
Occasional maintenance:
* asm-differ: latest commit
* m2c: latest commit
* maspsx: latest commit
* spimdisasm: 0.18.0 latest
* splat: 0.17.3
The latest version of splat is 0.19.1 but a breaking change in
[0.18.0](https://github.com/ethteck/splat/pull/294) is preventing me to
upgrade further ([discussion
here](https://discord.com/channels/710646040331681844/813939516385525790/1172978921000669274))
Some YAML were malformed and since splat 0.17.0 there are additional
checks to ensure they are compliant. There are also new checks that
prevents a malformed symbol list including duplicates, which I fixed
too.
This has been one of the most weird functions I ever decompiled. I will
share a few tricks I learnt. In short, I tried inlining as much as
possible by removing all the temps.
---
```c
switch (self->step) {
...
case 2:
isEntityAlive = 0;
if (self->step != 2) {
return;
}
```
This was the output from M2C. As there is no way that `self->step` is
different than `2`, deleting had no effect on the matching.
---
```c
var_v0 = self->step;
switch (var_v0) {
case 1:
...
if (statement) {
self->step++;
}
...
D_SOME_VARIABLE = var_v0;
}
```
This was another very weird one. I couldn't understand why
`D_SOME_VARIABLE` was assigned that way much further down the `case 1`.
The way I fixed it is that `var_v0` was always `1` due to `case 1:`. By
doing `D_SOME_VARIABLE = 1` I got a match.
---
```c
temp_a0_2 = D_80174C2C + 1;
...
D_80174C2C = temp_a0_2 & -(temp_a0_2 < 0x10);
```
To understand this madness I used a random C compiler I found online and
tested in a `for` loop what's the output for all the given `temp_a0_2`.
It seemed the value never changed but over `16` the value was always 0.
I logically re-written that statement into something that made logically
more sense for me and it matched, even if it looks very different from
the original:
```c
D_80174C2C++;
D_80174C2C = D_80174C2C >= 16 ? 0 : D_80174C2C;
```
Fairly straightforward one here I think.
Unfortunately the subweapon name does not appear in the definition
(actually... are subweapon names in the game at all? I don't recall them
being in there, I think it's all just in the manual?) so it's hard to
map these definitions to the subweapons. Might be good to think about
whether we could insert comments somehow. Anyway, this works for now and
might help with collecting the subweapons in one place to understand the
remaining members of the definition.
Primarily decompiling the entity function for the rebound stone, but
also some slight cleanups here and there for things that relate to it.
Very nice to be able to understand things better! Seems that subweapon
entities aren't very well decompiled so that might be nice to dig into.
After decompiling the bat fireball, I wanted to decompile all the places
where it can be created, so I did this one, which contains the entity
factory creation function call for the bat familiar to shoot its own
fireball.
Kind of neat to learn that the player's own fireballs and the familiar's
fireballs appear to be identical since they use the same function and
everything.
This required a bit of shuffling of other code in other places; happy to
elaborate on any of those changes but they seem fairly benign.
This is the entity produced when you use Fire of Bat.
Function is decompiled, and I also added some slight improvements to
ControlBatForm. I also tried to make a descriptive comment about this
function and how it is created - I'm hoping that, moving forward, we can
increase the amount of descriptive, guiding comments exist on things, to
help improve our ability to identify various values in things.
Take a look at the weird if-statement in `case 0`, no idea why it
exists. Seems like old code that someone adjusted and left stuff behind.
A while ago, we renamed Entity.facing to Entity.facingLeft so that the
binary state would be obvious at a glance.
The symbol for PLAYER_facing was never renamed. When I create scratches,
one of the first things I do is a find-and-replace for `PLAYER_` to
become `PLAYER.`, and having the symbol match the new entity member name
will make that much more convenient. Ultimately, this PR is tiny and
just makes things a bit more consistent.
Decompiled this one a little while ago, but spent some time on entity
factory stuff before putting it in the repo, so here it is.
Not sure why the string in rodata isn't working, but I couldn't get it
to work, maybe someone else can fix it.
Occasional maintenance:
* asm-differ: latest commit
* m2c: latest commit
* maspsx: latest commit
* spimdisasm: 0.18.0 latest
* splat: 0.17.3
The latest version of splat is 0.19.1 but a breaking change in
[0.18.0](https://github.com/ethteck/splat/pull/294) is preventing me to
upgrade further ([discussion
here](https://discord.com/channels/710646040331681844/813939516385525790/1172978921000669274))
Some YAML were malformed and since splat 0.17.0 there are additional
checks to ensure they are compliant. There are also new checks that
prevents a malformed symbol list including duplicates, which I fixed
too.
With the decompiling of the entity factory framework being started, I
went through and replaced some placeholders that have accumulated over
time. Originally I was going to try to decompile the whole sequence of
events that happens when you cast Hellfire, but I think the core of that
is in EntityAlucard, since we set a player step.
Overall, this should have no functional changes and should all just be
renaming things to improve based on new knowledge.
While the purpose of the function is clear, how it impacts the flow of
the game is unclear to me. Since a few variables are coupled with the
`LoadFileSim` function, I expect this callback effectively does nothing.
The name `VSyncHandler` comes from one of the PSY-Q SDK samples I found.
Other names were `cbvsync`, which is a bit too similar to the callee
`VSyncCallback` for my tastes. None of the samples from the SDK even
remotely look like this function I decompiled.
This is the last function from the `47BB8.c` file that had to be
matched, and the last blocker to keep importing data and rodata.
I managed to decompile a function I was stuck months ago. I left a
comment above the function name to explain what the function is and why
it exists. The rest of the code and changes should be self-explanatory
enough.
I renamed `DEBUG_PTR` to `SIM_PTR` because `0x80280000` is only used on
devkits as a memory area to load files from the simulator device. The
name "DEBUG" was a bit too vague.
This is one I first looked at many months ago, finally I got a match.
That being said, no idea what it does.
This also includes what I think is an innocent unrelated change to
func_8010E168; if I'm being honest, I just forgot to remove that change
from my local repo. I can push a new commit to remove it if desired but
for now I'll leave it since it's an improvement and not worth its own
whole PR.
Continuation of #751 . A few functions from `gameover.c` has been moved
to the new file `loading.c` as I noticed a clear separation in logic
between the two files and I got the faint hint they might have really
been separate files. The alternative is the `Handler*` functions were
part of a big massive C file, which I doubt.
I took the opportunity to clean-up a bit of the code. I had to stop and
make a PR because I cannot keep importing until `func_800E7E08` is
matched.
For the sake of simplicity, the global variables that are only used in a
single file or in a single function, and that can respect the order they
are stored in DRA.BIN, can be removed from `dra.h`. Eventually I
envision `dra.h` to only store prototypes of functions and global
variables that are strictly shared between files as we import the data.
Issue was raised on discord, this should correct for it.
Previously assets.py would output raw binary data. This makes it more
difficult for the build chain to understand.
Now, it outputs an assembly file (which is just a big list of `.byte`
entries) which will represent the parsed data, and that assembly data
then gets assembled. Therefore, the symbols now exist.
In order to make this work, I adjusted the format of the splat, so now
we have lines like `[0x7718, assets, accessory, g_AccessoryDefs]` where
the entries are [Address, assets, configuration json file to use, name
of the output symbol].
I think this addresses the issue, but I am more than happy to iterate on
this if needed.
`func_8011AC3C` appears to be a very important function when it comes to
spawning entities. From what I can tell, it seems to be responsible for
creating all of the entities listed in `asm/us/dra/data/CF4C.data.s`
from 800AD0C4 through 800AD1D0, which is the game's main entity updating
function lookup table.
When func_8011AC3C is called, it uses its `self.params` to index into a
table which contains spawning information for entities. For example, if
func_8011AC3C is called with `self.params` of 0x21, it will create a
child entity with entityID of 25, which is the entity for the Hellfire
spell (which I recently decompiled and will submit as a PR soon).
I think func_8011AC3C is going to be useful for understanding entity
spawning logic, so I have gone ahead and taken the data table that it
uses at D_800AD1D4, and created it as an asset which will now be
extracted by splat. This way, we can more directly view the data, rather
than just seeing it as a giant array of u8 values. I am hoping that
having this available as json will be helpful for understanding the
entity creation logic. Overall, this PR is meant as a stepping stone to
fully understanding func_8011AC3C and what it does to create the
entities that it creates.
Doing this has helped me understand one struct member's purpose, which
is to set the ID of the child entity, so I've added that to the struct,
both the entity extension for func_8011AC3C and the struct which this is
an array of, which for now is Unkstruct_800AD1D4.
This took me a while. I used a [throw-away
script](https://gist.github.com/Xeeynamo/58da1ff8f3831d0ba5d23da27cbca025)
to help me with the quest, but I still had to manually check every
single YAML subsegment.
I transported over what we were able to decompile from main. For a few
functions I added the signatures and documented existing DRA code,
especially on the sound department (got inspired by the recent @sozud
PRs).
All the `.text` part from `main.exe` is now completely extracted as
C/ASM where appropiate. Almost all the functions have their original
PSY-Q names but 7 of them: `func_80012DBC`, `func_80012F84`,
`func_80017008`, `func_80017078`, `func_8001929C`, `func_80021F0C` and
`func_800286E0`. I did not feel confident enough to rename them, so I
left them be. The rest of the functions I am 99% sure they are all
accurate.
I am now excluding the
[asm/](https://github.com/Xeeynamo/sotn-decomp/tree/master/asm/us/main/psxsdk)
folder from the repo. It was useless.
This research confirms me the game uses the PSY-Q 3.5 libraries, with
the exception of `LIBGPU.LIB`, which is from PSY-Q 3.0 for some unknown
reason.
EDIT: `make format` was not taking care of duplicated symbols in our
symbol list. To speed-up my work, all the duplicate symbols (duplicates
= name AND offset) are now removed.
Not sure if this was a typo or what, but this file was at the wrong
address. Because the first function was already decompiled, it didn't
matter that the first 8 bytes of assembly were missing. Anyway, this
fixes that.
Running `make force_extract` would previously generate a
`asm/us/dra/nonmatchings/704D8/func_801104D8.s` which had 2 missing
instructions which caused the start of the function to be really weird.
With this change, the assembly file will be generated properly with all
instructions present.
I imagine this could have been a headache for someone in the future but
this will fix that possibility.
Failing to decompile [RenderTilemap](https://decomp.me/scratch/WigVS)
made me realise there were a few fields and structures I have always
been suspicious to be part of the same structure. After
cross-referencing overlapping fields from different structures, I was
able to merge `D_80073088`, `g_Camera`, `D_8007309C`, `g_CurrentRoom`
and `g_CurrentRoomTileLayout` into the new `g_Tilemap`.
I was forced to touch the majority of the code-base, which gave me the
opportunity to standardise some field names (e.g. from
`currentRoomTileLayout`, `roomLayout`, `layout`, `t` into `tilemap`),
remove some fake code, redundant code and adjust some symbols.
While going through dra/7E4BC.c sequentially, I was going to decompile
func_8011EDA8, but then noticed that it is a duplicate from RIC.
Therefore, this PR copies the code from RIC (with different
AnimationFrame constants) to match.
This function calls DestroyEntity, which is in many, many overlays, but
for some reason in RIC was still using a placeholder address name, so I
also went ahead and properly renamed that to be DestroyEntity.
No idea what this one is. It's very complicated though (5 switches!) and
I'm amazed it matches.
It requires a few new unknown structs. There's a bizarre thing that gets
loaded from g_PrimBuf which is not a primitive but acts like one in some
ways so I called it FakePrim for now.
Overall I have a ton of confusion about this one and what the actual
structures are so if anyone recognizes these patterns, I'd love to
update this PR and make it make sense.
In terms of identifying what this function is for: the only thing I've
noticed from emulator testing is that this entity gets called when you
are in a Wing Smash and you run into a sloped surface which redirects
your flight.
Decompiles HandleMenu. The HD version is not yet in and I will submit it
with another commit in this same PR. I wanted to collect some feedback
before decompiling the HD counterpart.
Fairly straightforward one aside from what I mentioned in the comments.
Not sure how we want to deal with step_s values, given that there are so
many possibilities. I'm not inclined to make an enum right now unless we
have a proper system for them.
This has a solid use case for Entity.unk6C as a timer, so that's
interesting. I've seen timers in the extension before but this one has
it within the main entity struct.
Not sure what rotX and rotY are but this one definitely seems to use
them for scaling instead of rotation (tested in emulator, changing it to
decrement by values other than 8 changes how quickly the trails shrink
behind the player).
This is one I've been working on for a very long time, and it is nice to
have it matching now.
The main purpose of this function is to create entities. I believe it
actually creates most of the other entities I've been decompiling
lately.
When this function runs on step 0, it uses its params to read data from
an entity initialization table at D_800AD1D4. This data gets loaded into
this entity's extension. Then, that data is used to store a bunch of
values for the created entity. I think it will be worthwhile to spend
some time studying this function to learn how lots of entities are
created.
This function has rodata, so I needed to split it out to a new .c file.
Rather than just having the file start at this function, I tried to
split them instead at a logical boundary in the types of functions. The
previous file ends with some spell handling functions, and this one
starts with entity allocation functions, so I think it seems like a
reasonable place to draw a line.
Very happy to have this working! Please find corrections wherever they
are needed, this is a complicated one and I'm sure there are things I
could write better.
This is the white/blue laser-beam that comes out of your feet when you
use the Gravity Boots. This was identified through a combination of
setting breakpoints and using the debug mode's entity spawner.
This calls func_800EDB58 which is right before AllocPrimitives and seems
to have a very similar job. We had a weird mismatch where I had to
change the return type for that function, but it works now.
This seems to be entity ID number 3.
My last PR involved decompiling an unknown entity. I was hoping to find
a call to CreateEntityFromEntity which would match, but none has been
decompiled yet.
I noticed that there is a function which calls CreateEntityFromEntity
which is decompiled in all overlays except CEN and RWRP. I figured we
might as well finish it out and have it done in all of them.
The RWRP version calls AllocEntity, which also wasn't decompiled in
RWRP, so I copied that in as well.
Not at all an important PR, and if we're not doing PR's that are only
duplicates then feel free to close this without merging.
Not sure what this one is. The entity is ID 24. This comes right after
EntityHolyWaterFlame in the entity updater functions list. I tried
setting a breakpoint and playing around with subweapons for a bit but
couldn't get the breakpoint to trigger.
I'll keep trying to play with this to figure out what it is, but for now
we can at least check that my code is good.
I had this decompiled for a while but I have been waiting for all the
previous pile of functions to be decompiled before affording to propose
this function here.
Very similar to #693 , this one seems to be more generic and used in
multiple places, so I named it `MenuHandleCursorInput`.
I found a few fields from `MenuData` to be part of a third
`MenuContext`. I cannot extract that `menus[3]` out of that struct or it
will fail to align (@bismurphy explained why this happens, but I do not
remember where to pick the conversation).
Decompiles what I believe is the function that handles the page
scrolling via R1 and L1. It uses `MenuContext.unk16` but I am still not
sure exactly what it does, therefore I left it undocumented for the time
being.
Importing the rodata from HD took me a good amount of experimentation
and trial&error. With the past PRs I was able to decompile all the
functions required to import the `D_800A2D68` array and the jump table
of #689, #688 and #687, which it had to be done all at the same time.
I also took the opportunity of decompiling the HD version of #445 ,
which actually appeared to be simpler than the US counterpart. I tried
removing some fakes from the US function but without luck. But I managed
to un-fake #447 by keep importing the `rodata` and by using
`D_800DC6EC`.
There are now five functions left in 5298C, including a very big one.
With HD aligned to US, finishing this source file will be a walk in the
park.
I am not sure why I thought it was a good idea to populate the symbol
table with so much useless stuff. It has been like this since the [first
commit](https://github.com/Xeeynamo/sotn-decomp/commit/fcf1f077#diff-ad26526355e8bad3a3db379e8b039623e45c8620831de04ac21b76b9da6b7f4e),
probably indicating some level of inexperience on how decompilation and
linker works.
This frees some symbols from hardcoded addresses, allowing more
relocation flexibility now that DRA.BIN is getting relatively close to
completion.
Decompiles the function responsible of drawing all the menu. I am still
not sure to understand how it works.
Thank you very much to @bismurphy's original scratch, which helped me
getting this match.
EDIT: The `HD` version is matching, but due to the missing rodata import I
still cannot proceed to import it. I need `func_800FBC24` decompiled
before importing the `HD` rodata.
First function as part of my next goal to decompiling the entire
`5298C.c` file. As this effort includes the decompilation of
`func_800FBC24`, I noticed there was a wrong signature. I took the
opportunity to fix that as a separate commit as part of this PR, even if
the change is unrelated to the function I am submitting.
Decompiles the function `LoadPendingGfx` and add comments to document
its behaviour. That allowed me to understand and refactor
`func_800EAF28`, which is now called `LoadGfxAsync`. I was able to
document a previously unknown structure and discover the struct type of
`entityGfxs`, now renamed as `gfxBanks`.
I noticed a good amount of improvements in the repo between solving fake
matches and cleaning up previously unknown structures by reusing known
structures.
Just another boring function, nothing interesting here...
Haha! This is one of the most important functions in the game, it
manages a whole bunch of "running the show" and calling all the various
functions that control everything.
One of the biggest changes is that, due to a single CPU instruction, I
was able to determine that g_zEntityCenter is a member of a struct. I'm
not sure what that struct actually looks like, but we have the start of
one, for now. Creating that struct meant adjusting every other place in
the game that g_zEntityCenter is used, which is why this PR shows so
many files changed.
Overall this function has a ton to discover and I'm sure by studying it
we will find out more about how the game works.
Please look over it carefully for anything I may have missed like enums
or anything else. Very excited to have this one working, especially
after the disappointment of getting so close with EntityAlucard!
Just another function.
It originally had 5 temps and casts everywhere. I got it down to one
temp that I couldn't get rid of (need an s32 to force lw instructions
when storing to a s16, I think). Decompme here if anyone wants to try to
remove that temp: https://decomp.me/scratch/bbfhK
Otherwise pretty straightforward.
I started importing the data in SEL also as a proof of concept we can
also import the SOTN-specific formatted strings. For that I integrated
the new tool `tools/sotn-str.py` into the build chain. It works like the
following:
* `./tools/sotn-str.py parse disks/us/ST/SEL/SEL.BIN 27598`: read the
binary content in a specific offset and parse it as a readable Unicode
string.
* `cat src/st/sel/2C048.c | ./tools/sotn-str.py process` converts the
Unicode string into a byte sequence that can be understood by the SOTN
engine.
There is a dirty hack I did to make this work:
```c
#ifndef SOTN_STR
#define _S(x) (x) // Strings processed by tools/sotn-str.py
#endif
```
When invoking `mipsel-linux-gnu-cpp` I am passing -DSOTN_STR,
effectively ignoring `_S` in the pre-processor. Then I am passing it
into `sotn-str` before `iconv` to resolve any string surrounded by `_S`.
I am a bit of afraid this might be annoying when using the permuter. I
tried doing `$(SOTN_STR) -f $< | $(CPP)` but apparently `cpp` does not
work well with content passed via stdin.
After a while I was able to fully document a previously unknown entity
(`unk27` is unused) thanks to my initial work of decompiling
`RenderEntities`. I also noticed @cram0
[scratch](https://decomp.me/scratch/GVVn1) where he was able to use it
`D_800A21B8` in the for loop but with the same match as my previous
stalled scratch. I merged the two, used the new struct `PlayerDraw` and
got a match!
I took the opportunity to go through the code for everything that uses
the new `g_PlayerDraw` and resolve some fake matches.
This is a random entity updating function I found by looking for calls
to IsRelicActive. This appears to do stuff with mist movement.
I was able to improve our knowledge of a random struct, because of the
way we are setting self's position.
We had an extern defined in the C which meant I initially didn't notice
the unkstruct existed; that is now in dra.h.
I happened to find a different spot in a different function where we had
a `const` defined without a type which I'm surprised wasn't a syntax
problem. It's unrelated to the focus of this PR, but it's a tiny change
so I hope it's okay to bundle it in here.
The last one of this set!
Before merging, we should work out how to deal with D_8006CBC4 since
right now we don't have a good way to enum it.
Oddly this didn't use rodata for the switch even though the others did.
It does use rodata for the FntPrint strings though.
Otherwise I think those are all the highlights.
This seems to handle the rotation and scaling of an entity. I am still
trying to figure out how it fits in the overall picture and I have
`RenderEntities` queued next to solve that missing piece of the puzzle.
So far this matches and it is pretty much all documented other than the
function name.
Mostly more of the same.
A bit interesting, most of the timers get set to 24, where the other
spells all used 20. Seems the developers wanted to make Soul Steal
slightly easier, likely due to the complicated input.
Interestingly, for the first button, in case 0, there is a discrepancy
based on whether you start the spell facing left or right. If you start
facing left (so your first press is back, to the right), you get 20
frames, while facing right gives 24. Not sure if this is worthy of a
`@bug` annotation?
Next two spells.
Doing these one by one created a weird difficulty because
g_WasFacingLeft5 is an s16 for some reason. Without getting the next one
in, it was loading as an s32. Not sure why.
Anyway, given that the structure of these functions is mostly the same
as the previous ones, I hope it's not a problem to be doing two
functions in one PR.
Interesting that hellfire has a FntPrint.
Now that assets.py exists, it's a whole lot easier to extract assets!
In order to make g_EnemyDefs get added to assets/dra/enemydefs.json,
only a few changes are needed.
1) Realign the bounds of the splat to add a separate line for the end of
g_EnemyDefs. g_EnemyDefs is exactly 400 elements in size. We have a new
data segment at 0xC780.
2) Change the yaml to extract it as "assets", named enemydefs.
3) Previously we would output name_resolved and desc_resolved to the
.json assets; since g_EnemyDefs don't have a description, we now move
that to an if statement that first checks if it exists in the config
file.
4) Create the config file for enemydefs!
Overall this creates a very smooth workflow for extracting more assets.
The biggest limitation we have with this approach is that the config
file is selected based on the name in the third space in the yaml (for
example, `[0x4B04, assets, equipment]` loads
`tools/splat_ext/equipment_config.json` by name). Therefore entries like
`layoutobj` which is used many times in `splat.us.stwrp.yaml` are not
possible to extract using this method, since there are many of them
which use the same config. I'll see if I can work out a solution to
this. Ultimately we're using the same `self.name` property to control
both what config file gets loaded, and what file we output to in
`assets`, but there's no reason those need to be the same parameter. I
just need to figure out how to pass an extra variable from the splat
into the parsing script (`assets.py`). I'll work it out somehow at some
point, and then we'll be even more flexible. For now, it's nice to be
able to browse all the enemies in the game!
---------
Co-authored-by: Luciano Ciccariello <Xeeynamo@users.noreply.github.com>
Coming back to these now.
Decompiling these is pretty fast now that I know the structure that they
all follow. Most of my time is just spent going through all the magic
numbers and changing them to use enums and such. I think I got all of
them for this one :)
When extracting equipment and accessories, we use scripts in
tools/splat_ext called accessory.py and equipment.py, each called with a
different makefile rule. These files have a lot of "data in code", with
function calls matching the structure of the equipment and accessory
structs. They also involve repeating the structure both in the
extracting from the game, and the rebuilding into the compiled
executable.
This PR takes the equipment and accessories, and turns them into a
single generic makefile rule, which calls a new script called
`assets.py`. We change the splat yaml to match. To control the
extraction, we add new _config.json files in the splat_ext. This means
we only need to define the structure of each of these in a single,
localized place.
If we like this new approach (which should be more flexible), I will see
whether it can work for more of the assets in that same area of the
makefile. I will then see about adding extraction of enemies (in
g_EnemyDefs) next.
assets.py is adapted from the old equipment.py with a lot of changes to
make it work more flexibly. I'm hoping it will be a nicer path forward
into the future.
Previously if either the YAML or the symbol list were changed, we were
forced to either manually delete the `asm` folder of the specific
overlay or to invoke `make clean`, where the latter forced us to
re-extract all the overlays via `make -j extract`.
That changes with this PR. How `make` works is `target: dep1 dep2` where
if one of the dependencies has the last modified date greater than the
target, the rule is triggered again. Previously we were extracting
overlays doing `make extract_stcen`. But since every overlay extraction
generates a linker script I now changed the rules to do `make
build/us/stcen.ld` to extract the same overlay. As I explained above, if
either the YAML or one of the related symbols changes, the linker
modified date will be older and the extraction for that overlay to be
trigger again.
The above allowed me to stop polluting the repo root with the linker
scripts as they are now moved into `build/us` or `build/hd` depending of
what you're trying to build. If you do `make extract` twice in a row you
will be welcomed with a `make: Nothing to be done for 'extract'.`.
There are still some instances where you **might** need `make clean`
beforehand, especially when modifying the `segment` section in the YAML
or renaming the symbols. A `rm -rf asm/$(VERSION)/$*` can help but I
want to see if the current solution will be enough.
### Please try removing `make clean` from your workflow, once this is
merged, to quickly detect possible problems with this new approach.
I am also planning to make more substantial changes on our build-chain
like this PR or #660 in the future. I am aiming to small incremental
changes over time in case I break someone's flow or detect design flaws.
I am also considering the breaking changes introduced in make 4.4, which
will most likely be included in Ubuntu 24.04 LTS. I set March 2024 as a
deadline to finish all the new build-chain work.
---------
Co-authored-by: sozud <122322823+sozud@users.noreply.github.com>
This one took a while but it works now so that's nice.
It seems to search for entities matching a certain set of criteria.
The D_80138038 variable seems to only be used in this function. I don't
know what it's used for.
I also don't know what the point of the behavior with temp_a0 and var_v1
is for. We have a lot of dividing and multiplying by 128 (which is also
the size of the array we're looking through).
For some reason without the LOH() I was loading posX.i.hi and posY.i.hi
with `lhu` instead of `lh`. Not sure why, and not sure why I only need
to do that on one of each of those, but either way it works so that's
good.
Overall lots of mysteries with this one but hopefully having it
decompiled will be the first step toward answering those mysteries.
The next one in that same set.
Now the spells are complicated enough that the state machine switch
which tracks the progress through the buttons is complicated enough that
the switch gets implemented in rodata, instead of just as a couple of
BEQ's. Therefore, file splitting needs to happen.
Traditionally, when we need to split files for rodata, we put the newly
decompiled function at the top of the new file. However, given that we
know these functions are a "family", I decided to put the split earlier,
so that the family would be gathered up nicely.
Interestingly, this one's g_wasfacingleft is an s32 while the others
were s16's. Of course, that doesn't matter since it's effectively used
as a boolean.
Case 5 (representing the spell input being done) is a little weird with
its nested if statements, but I don't think there's a good way to
rewrite it.
That's about it, these are getting pretty easy to decompile now that I
recognize the common patterns throughout all of them!
Next one in the sequence.
This, like the previous one, needs a variable to keep track of what
"forward" means from the start of the input sequence. It is right after
the last one, but unfortunately they don't work as an array, so I just
placed a 2 at the end, with a comment, instead of making the name overly
long. Hope that's okay. Otherwise no further comments, hopefully since
we're mostly just using the structs I already established this and
further functions will come quickly :)
The next one of the spell-pattern checking combos.
Before this was only marked as being for the wolf charge, but I found it
being referenced in some weapon handling code too, so I believe this is
responsible for ALL quarter-circle-forward inputs (for example, the
Alucard Sword has a special teleporting move when done with the quarter
circle forward).
Naming comes from this page:
https://critpoints.net/2018/03/04/how-to-perform-fighting-game-motions/
and I abbreviated to QCF in the combos enum, just because I don't want
line lengths to get ridiculous. Made sure to include a comment on the
enum saying the full name though.
This is pretty cool. So there are only a few places left in the game
with non-decompiled calls to IsRelicActive. I picked one of them at
random and started going through it and it turns out it's the gravity
boots check. This function acts as a state machine where state 0 is at
rest, and pressing down initiates the transition to state 1. At state 1
it waits for an UP input, and if it gets it, moves to state 2 to wait
for an X input. At each state, there is a timer limiting your time to
put in the next button input.
Now, we have these two variables that work together (the state, which
tracks how many correct buttons have been entered, and the timer), and
they exist in an existing struct which had two values in it. I believe
the array of this struct holds all the button counters and timers for
all the different spells in the game. However, with my recent
decompilation of the Wing Smash, we see that the Wing Smash is handled
with its own logic. I think most of the other spells will use this same
array to keep track of their overall button counters and timers.
When this function is called in func_80111830 (At the start of this
file:
https://github.com/Xeeynamo/sotn-decomp/blob/master/src/dra/71830.c),
it's in a for loop which goes through several different checks and calls
different functions. I'm betting these are all the spell button inputs.
And none are decompiled. So I will start going through those one by one.
Should be fun!
I had to change some existing accesses of the g_ButtonState which were
set up before we really understood what that was, so let me know if you
have any thoughts on those. I'm excited to be able to get the rest of
these in this set decompiled!
Continuing to finish out w_029 now that we have g_GameTimer % 5 figured
out.
There is a very unfortunate thing here. We are using
`self->ext.weapon.lifetime`, but EntityWeaponShieldSpell just had a
whole mess about 7C and 7D and needing to create ext.weapon29. Clearly,
weapon29 references this location as both a s16, and as two u8's. I
really don't know what to make of that. Happy to adjust things here if
we want to handle that differently, but for now this seems to work. It
will be cool if we can get this strange weapon fully decompiled.
Now that we fixed the `li` instruction, the other functions in this
weapon file can come spilling out :)
There was a jump table that I changed to being rodata. Luckily that went
smoothly.
Interestingly, this does the same %5 as the other function, but there
are two spots where it uses g_Timer (the one that always runs) and one
other where it uses g_GameTimer (the one that pauses when the game
pauses). It's only palettes so it doesn't really matter, but it's neat.
One problem: This stores two bytes to self->7C and self->7D. But we
already had a different weapon that used 7C as a s16. So we may end up
needing to make different `ext` entries for the different weapons. For
now, I'm leaving it generic.
As usual, I don't know what this one does. It seems like it might be
involved with some relatively low-level graphics, with the use of
DRAWENV, but that could be wrong.
It was tricky to get this one to work, especially with the way it passes
full structs (rather than pointers) around.
Decompiling this shows that it does not take arguments; previously we
thought it needed a Primitive, but I now know it is void, so I fixed the
existing call to it.
This function is only called in the existing location, and in
EntityAlucard.
There is a weird cast with `SetDrawEnv(*(s32*)&prim->r1, &sp10);` since
`prim->r1` should be a u8. Not sure what that's about. We've seen lots
of places where the rgb values in Primitives are used to store random
data, so I guess that's what this is.
I am proud to announce I decompiled the biggest function from the game
detected so far! This operates the whole logic of the main menu and
links together all the functions I've been decompiling in the past week.
I did my best to import the `.rodata` function. There's a big catch
though. The symbol `D_801A7620` is used in this function, but I cannot
import it because it is also used by `func_801ACFBC`, found much earlier
than `Update`. This most likely means `2C048.c`, `2D260.c` and `2EED8.c`
were a single source file.
Before committing myself into fully documenting the function, I want to
finish to decompile `func_801ADF94` and `func_801AD66C`, which are
necessary to merge the three files and finish importing the `.rodata`.
Then I need to start to decompile `func_801B2108`, `func_801B3164`,
`func_801B38B4`, `func_801B3A94` and `func_801B3F94` as they are
dependencies of `Update`.
Thankfully, when decompiling this one, I very quickly realized that it
looks extremely similar to `func_801177A0` and `func_8012EAD0`. Only a
couple lines different. Mostly a copy-paste. I'm not sure if there are
more functions in this group, I will try to hunt for them.
Notably, this is the last function which calls dra.DecelerateY.
Interestingly, there are 5 functions which do so: ControlBatForm,
ControlMistForm, and the three functions in this family. These ones do
not take player inputs though. Neat!
Not sure what these 3 actually do. They are all only called by
EntityAlucard. Now that we have all 3, maybe we can start to study them
and work out what they do. Or maybe we will have to wait for
EntityAlucard to get worked out.
Co-authored-by: Luciano Ciccariello <Xeeynamo@users.noreply.github.com>
This is a cool one, this function is responsible for draining your MP
when you're in a transformed state (bat, mist, or wolf).
arg0 specifies the relevant transformation, I think this should be
'form'. 0 = bat, 1 = mist, 2 = wolf. As far as I know, these values only
exist in this function; if we find them used elsewhere they should
become an enum but I'm inclined to hold off on that for the moment (let
me know if I should make an enum right away though).
arg1 is a bool for whether we should actually drain MP, or just check if
we have enough. Not sure what to call this. I've seen this kind of logic
in a few places on a few functions, this could maybe be an enum with
"CHECK_ONLY" or "REDUCE".
Finally, this function returns 0 if all is well, and -1 if there is a
problem (for example, if we need to reduce MP, but the player doesn't
have enough to stay transformed, or if the relevant relic is not
active).
I'm not sure what to call this function. Maybe HandleTransformationMP?
This kind of does three things, it checks our MP state being okay, it
takes our MP away, and it reports whether we can stay transformed, so I
think "Handle" is the best verb. Happy to take other opinions.
Another small step to decompile `func_801B3164`. It is still not exactly
clear what this function is doing. I couldn't import `D_801A7AE4` from
the `.rodata` without committing into a file split. I can import it if I
am also able to decompile `sel.Update`, which is planned after
`func_801B2F50`, `func_801B3164` and, hopefully, `func_801ADF94`.
This one is an absolute beast and I'm amazed it matches!
This is one of the primary functions that runs when you have a weapon
(or other item equipped in your hand) and you press the attack button
(square or circle by default). It handles the special cases for many
weapons, based on their `unk11` property. I have gone through `unk11`
and labeled weapons in the places I was able to identify them. For
example, this code includes the test on rand() & 7 that applies when you
use the Red Rust, and causes it to fail to swing.
There is still a lot of `goto` in this. I suspect some of it could be
cleaned up, but not all. My scratch is here if you would like to play
around with cleaning up that control flow
https://decomp.me/scratch/d1brK
I think documenting this function would be very worthwhile for someone
in the future who is very interested in weapons; for now, I just want to
get it in as it's working.
The former g_BlinkTimer continues running during pauses, and may be used
for more things than just blinks, so it will now be g_Timer.
A previously unidentified variable is a timer which only runs when the
main game is running (when things are unpaused). When loading a save, it
starts as zero. It does not restart when doing things like entering a
new area in the game. This will be called g_GameTimer, since it's the
timer that only runs when in-game. It appears to increment once per
frame.
I have renamed these, changed every instance in which they are used, and
also cleaned up a few instances I found where we had patterns like
`if((variable / value) * value == variable)`, which equates to
`if(variable % value == 0)` which I think makes much more sense. I'm not
sure if I found all those cases, but at least I found a few.
Called every time you select one of the four options in the main menu.
What it does is to hide all the graphics through the newly documented
`MenuHideAllGfx` and display only the sub-menu decorations.
I was torn to name this function either `SelectMainMenuOption` or
`InitSubMenu` as the flow seems to indicate both are good name
candidates. I decided to go for the former as `InitSubMenu` might be
used if we find another function that is responsible of waiting the
memory card to be loaded and slowly show the graphics.
I also took the opportunity to start documenting some magic numbers. I
had to create `MAIN_MENU_CURSOR_INVALID` so the argument `MainMenuCursor
cursor` would be considered as `signed` from the compiler. This is a
dirty hack. I am not sure if we should just go for a few `#define`
instead of an `enum`.
The `SaveSummary.padding` is not padding but technically unknown. I
think it represents the state of a memory card (e.g. inserted, absent,
unformatted) but I am waiting to rename it to something more
appropriate. [func_801ADF94](https://decomp.me/scratch/A177U) is still
holding me back from some renaming.
