This took me a lot of manual work. But I think I confirmed a pattern
that will help me to automate all of this for all the next stage
overlays that will be imported in the repo.
I noticed some stages with only one room having more than two layers or
more than two tile definitions, it might be either debugging or unused
content? I did not have time to explore any of that.
I modified the `tiledef` splat extension to greatly minimise the set-up
and noise.
Wow. This is perhaps one of the hardest functions I've ever done. It
does disgusting things with pointers that I've never seen anything like
before - some of them are the fault of the programmers, some are the
compiler.
Once I was like 98% done, I found func_801B69F8 which is very similar
(and decompiled), but wasn't picked up by the duplicate finder, so that
was a little sad. Once I found that though, the last of my mysteries
were quickly solved.
This is a huge function and it's nice to get it cracked.
Given the fact that sharing compiled C objects is not exactly possible
(code heavily copy&pasted maybe? this initiative is now abandoned 👉53a566f075)
I decided to keep pressing forward with shared headers. Thanks a lot to
@hohle for making our life much easier by cross-referencing symbols.
The file split on WRP seems to be the closest file split we might have
compared to the original source code (still speculating here). I think
it would be a good idea to start splitting other overlays too with the
same approach.
My idea is to have a file split like the following:
```
st/
cen/
e_particles.c
e_misc.c
st_common.c
nz0/
e_particles.c
e_misc.c
st_common.c
wrp/
e_particles.c
e_misc.c
st_common.c
e_particles.h
e_misc.h
st_common.h
```
each of those C files will just be a one-line `#include
"../the_shared_code.h"` as usual. Right now we create individual headers
for single functions, sometimes for more than one function when we think
grouping makes sense. But I think we can start merging some of those
headers and consolidate the code. This can be done gradually. For
example `src/st/e_particles.h` is still importing function headers under
the hood. That is okay for now, but later on I wish to import those
headers functions into their respective parent headers.
Another important aspect to consider that will validate a correct file
split is to start importing the data inside these new C files. Right now
we have floating data such as `src/st/wrp_psp/wrp_data_EA00.c` or
monstrosities such as `src/st/wrp/6FD0.c`. An example of a (possibly)
correct migrated data is what this PR does with WRP PSP and NZ0 PSX,
with data pointing in `src/st/*/e_particles.c`.
I think I did this right, but this is my first time de-duplicating a
function, so please point out any mistakes :)
ST0 not included because it has different logic internally - will work
on decompiling that one next.
This change renames functions and global stage variables uniformly
across the stages so that these functions can be pulled out and shared
across all of the stages. Based on some other tests there are 12 or so
functions that this will allow to be pulled out of each stage. Since
these implementations are shared, an additional 12 asm functions can be
eliminated in a subsequent pass.
**Vars**
* `g_pStObjLayoutHorizontal` - a horizontally sorted array of stage
entities
* `g_pStObjLayoutVertical` - a vertically sorted array of stage entities
* `g_LayoutObjHorizontal` - a pointer to a `LayoutEntity` in
`g_pStObjLayoutHorizontal`
* `g_LayoutObjVertical` - a pointer to a `LayoutEntity` in
`g_pStObjLayoutVertical`
* `g_LayoutObjPosHorizontal` - the direction last traversed in
`g_LayoutObjHorizontal`
* `g_LayoutObjPosVertical` - the direction last traversed in
`g_pStObjLayoutVertical `
**Functions**
* `FindFirstEntityToTheRight` - given an `x` position, update
`g_LayoutObjHorizontal` with the first entity to the right of `x`
* `FindFirstEntityToTheLeft` - given a `x` position, update
`g_LayoutObjHorizontal` with the first entity to the left of `x`
(backwards)
* `CreateEntitiesToTheRight` - given an `x` position, create all
entities to the right (mutates `g_LayoutObjHorizontal`)
* `CreateEntitiesToTheLeft` - given an `x` position, create all entities
to the left (mutates `g_LayoutObjHorizontal`)
* `FindFirstEntityAbove` - given an `y` position, update
`g_LayoutObjVertical ` with the first entity to the above of `y`
* `FindFirstEntityBelow` - given an `y` position, update
`g_LayoutObjVertical ` with the first entity to the below of `y`
* `CreateEntitiesAbove` - given an `y` position, create all entities
above (mutates `g_LayoutObjVertical`)
* `CreateEntitiesBelow` - given an `y` position, create all entities
beneath (mutates `g_LayoutObjVertical`)
* `UpdateRoomPosition` - look at the current game loop scroll delta and
create any entities given the room layout
I believe all of these implementations are shared across all stages
(including `InitRoomEntities`, and two more `CreateEntity` functions)
(in my initial tests I had a small difference in `DER`, but I believe
that had to do with an incorrect symbol table change).
Co-authored-by: Jonathan Hohle <jon@ttkb.co>
Thanks a lot to @SestrenExsis for doing the majority of the function
matching! I only take the credit for cross-referencing it with the PSP
counterpart and massaging the code until I got a match on PSX.
Decompile and de-duplicate EntityStageNamePopup for all the overlays the
function is in.
I also detected a helper function I renamed as `PrimDecreaseBrightness`
that was originally accepting a completely wrong parameter type. This
helped me to get rid of `Unkstruct_80128BBC` too.
Some deduping and renaming that might help later
- Dedupe DestroyEntity
- Dedupe ST EntityIsNearPlayer
- Dedupe DestroyEntitiesFromIndex
- Dedupe ST CollectHeart
- Dedupe ST UnkEntityFunc0
- Decompile + rename rwrp EntityExplosionSpawn
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.
Allows to not hard code the location in-memory of decompiled functions
and imported data if not required. This allows to relocate the
referenced symbols when editing the original code or game data. If you
were getting the offset of those symbols from the symbol list in
`config/` I suggest to use the built `build/us/*.map` file instead.
As I finished importing the data in WRP, I came with a possible new
pattern when de-duplicating functions. I kept using a header file
(easily indexable from VS Code) in `src/st`. But this time it gets
included by `entity_relic_orb.c`. All the sections `.data`, `.rodata`
and `.text` are migrated into `entity_relic_orb.c` but the file itself
just includes `src/st/entity_relic_orb.h`. Thinking forward, once we
will be able to fix the problem present in #464 it will just be a matter
of renaming `entity_relic_orb.h` into `entity_relic_orb.c` and remove
each individual `entity_relic_orb.c` that currently acts as a proxy.
~~This PR is a draft. If you agree this is a good pattern I will proceed
to do the same with the remaining overlays after #528 is good to be
merged.~~
Used by `EntityRelicOrb` and `EntityUnkId0E` to pre-render the text to
show as a texture.
I am pretty sure the function is bugged. When I use it in the debug
module it runs fine. But when I copy&paste the matching decompiled
function and use it, the game crashes here:
```c
for (i = 0; i < FontHeight; i++) {
ptr = chPix;
ptr += i * FontStride;
for (j = 0; j < 5; j++) {
ptr[0] = ptr[1];
ptr++;
}
*ptr = 0;
}
```
The debug module is using a much more modern compiler, so it is entirely
possible that there is an undefined behaviour in the current logic. I am
poorly convinced about the `if (ch == MINSCODE)` and skeptical of the
`if (i != FontHeight)` condition. The crash happens when a non-wide
character is used (e.g. `l`, `i`, `e`, `d`). It matches though, so there
you go.
The font the game uses is found in the game BIOS and it is retrieved by
`Krom2RawAdd`. According to
[Gemini](https://www.romhacking.net/forum/index.php?topic=17691.20) the
font width is resized in SOTN. This is done by `g_api.func_80106A28(ch,
1)`. Although good luck at decompiling it:
https://decomp.me/scratch/Lulhn
Co-authored-by: @Mc-muffin for figuring out the `while(true)` loop
Co-authored-by: @sonicdcer for figuring out the regallocs
Took me a while. I had to move a good chunk of the data from 6FD0 due to
`const char* D_80180F84[] = {"Obtained "};` as it declares data in both
`.data` and `.rodata`. Due to its strategic location, I did not yet find
a clean way to have it matching in other overlays without starting to
import data.
The function is not very clean and there are loads of temp variables and
fake stuff. I ask to spend a bit of time to make improvements where
possible. I want to get it in a good shape before committing myself to
de-duplicating it for all the other overlays.
https://decomp.me/scratch/kyBr8
This is an unknown function. I don't know what it does. It is duplicated
across most overlays. It is called in CollectGold, EntityEquipItemDrop,
and TestCollisions.
It seems to take in a string, and then do a bunch of things with
primitives, but I don't know what. We don't directly reference that
array, we only read and write to it based on a pointer which moves
through the array. I tried to remove the pointer but didn't get
anywhere.
Fully-matching scratch is here: https://decomp.me/scratch/UV2K2
I am starting with only this one version of the function, so that if
changes need to be made (variable names, control flow, etc), I can
update them once here, and then copy them to the other places, instead
of making changes in every place. Once we are happy with the state of
this function, I will add the duplicates to this PR prior to merging.
---------
Co-authored-by: bismurphy <bismurphy@users.noreply.github.com>
After attempting this de-duplication process I see that it's pretty
slow, tedious and error-prone. I think it would make sense to have an
intermediate phase where we split stuff into #includes like this PR
does. Eventually this will be combined back into C files with proper
file splits. I think a process that is incremental and would get us
there eventually could be like this:
Split single function into a header, #include it.
Rename any D_XXXXXXX variables to UNK_Whatever#, add to symbols.*.txt
Once the above is done for all instances of the function, perform file
splits (if we know there's a sequential block of functions, wait to
split until the whole block is done)
Combine individual.h files into a c file, integrate building
st/shared_#.c into Makefile
I'm probably just going to do a little bit on this each day to try and
chip away at it, it's too tedious to do a big push.
Thanks to the disassembly of the Saturn version (kudos to @sozud), I
realised how we got some fake matches in the PSX all along.
e567920005/src/saturn/game.c (L716)
Following the pattern from the Saturn version I was able to confirm my
theory to use `objectId - 1` and fix some oddities I found while I was
importing the `.data` section from PSX WRP:
e567920005/src/st/wrp/6FD0.c (L200)
This function was already decompiled for ST0, it was reviewed and merged
in a previous PR, The one for the MAD overlay is a duplicate of the same
func. As for the rest of the overlays, the function calls SetStep
instead of just modifying the Current Entity step and step_s directly,
so with that little change the "normal" version of the function is now
decompiled.
This pull request matches the following functions, which are all
duplicates of previously decompiled functions:
- DRE - `EntityAbsorbOrb` - Renamed to `EntitySoulStealOrb`
- MAD - `EntityAbsorbOrb` - Renamed to `EntitySoulStealOrb`
- NO3 - `EntityAbsorbOrb` - Renamed to `EntitySoulStealOrb`
- NP3 - `EntityAbsorbOrb` - Renamed to `EntitySoulStealOrb`
- NZ0 - `EntityAbsorbOrb` - Renamed to `EntitySoulStealOrb`
- ST0 - `EntityAbsorbOrb` - Renamed to `EntitySoulStealOrb`
- WRP - `EntityAbsorbOrb` - Renamed to `EntitySoulStealOrb`
- RWRP - `func_80194590` - Renamed to `EntitySoulStealOrb`
