Various small cleanups on this function.
In the past it was EntityStrongWarg, but I found that this is actually
the Fire Warg.
It is unused in NO3, but is used in RNO3.
Also removed a Multi that was being used that applied across two
different entities.
Various comments, cleanups, renamings, etc
Thought this was going to be more work as duplicate report had it at 93%
and with a jump table, as it turns out I think they're the same besides
a single global memory address
More research on how cutscenes work.
I normalized all the various C files as `cutscene.c`, marked all the
isolated function as `static` and renamed the main entity as
`{STAGE}_CutsceneExec` (e.g. `CEN_CutsceneExec`). I am using the
`OVL_EXPORT` to automate the names.
TO-DO:
- [x] Rename entity as `{STAGE}_EntityCutscene` for consistency
- [x] CEN
- [x] DRE
- [x] NO3
- [x] NZ0
- [x] ST0
- [x] MAR
~~SEL~~
The offset of the portrait data seems to be hardcoded. I have no idea
how to resolve these offsets at compilation time. The entire cutscene
script thing is very sketchy and horribly designed by the original
developers. What a nightmare to integrate into our project.
This is how a cutscene script gets decompiled:
```
LOAD_PORTRAIT(0x80188D8C, 0),
SET_PORTRAIT(1, 0),
SCRIPT_UNKNOWN_11(),
PLAY_SOUND(0x37B),
WAIT_FOR_SOUND(),
SET_SPEED(4),
'T','h','a','t',' ','v','o','i','c','e','!',' ',
SET_WAIT(16),
SET_SPEED(3),
'A','l','u','c','a','r','d',',',
LINE_BREAK(),
SET_WAIT(16),
SET_FLAG(2),
'i','t','\'','s',' ','y','o','u','!',
SET_WAIT(48),
NEXT_DIALOG(),
```
Had an idea, not sure if this is useful or not but I figured it might
help give an indication for newer people or those not in the Discord
loop if code is already duplicated when looking at the function report.
Here's an example output from the current state (minus the scratches
which didn't seem to load for me locally so I skipped them)
[functions.md](https://github.com/user-attachments/files/17162454/functions.md)
@bismurphy guessing this would be your area of expertise 😄
`CutsceneUnk2` uses 8 prims but only 7 are allocated. It looks like at
some point the game writes to the address `0x00000000` but I did not
confirm it with a PS1 debugger. This is a potential bug. `CutsceneUnk2`
signature was also wrong.
I had to get rid of `GfxBank` in `src/st/cen/header.c` and I probably
need to do the same for the other header files. The code reads chunks of
WORDS at the time and a structure would be too large to include
`GFX_TERMINATE` in it.
CEN is there but not linked because it conflicts with some WRP symbols.
I decided to take a much simpler approach compared to what I did with
WRP (which needs to be refactored later on).
There's a cutscene parser. The asset manager is exporting it to
`src/st/cen/cutscene_data.h` if you want to have a look. I know I am
still using `config/assets.us.weapon.yaml` and it probably needs to be
renamed as `config/assets.us.yaml` later on. I am still deciding.
* `ProcessEvent`
* `CreateEventEntity`
* `CalculateAngleToEntity` (the function formerly known as
func_80173F30)
* `GetTargetPositionWithDistanceBuffer` (the function formerly known as
func_80173F74)
There's 4 more duplicates which I'll start on separately so as to not
make this PR too big.
Decompiled libgpu SetDrawEnv
This function actually packs some of the short length fields from a
DRAWENV struct into DR_ENV struct code fields, which are longs.
IE dr_env->code[7] gets packed with env->clip.x in the hi 2 bytes and
env->clip.y in the low 2 bytes.
Perhaps DR_ENV should actually be PACKED_DRAWENV.
Co-authored-by: sozud <sozud@users.noreply.github.com> (main
decompilation)
Co-authored-by: dezgeg <dezgeg@users.noreply.github.com> (getting it
across the finish line)
Did my best to give contextual names where possible based on surrounding
code.
There's still some remaining ones where it wasn't clear what the entity
was referring to so have left some as is.
Export the symbol list from the compiled elf and call `splat split
TARGET --disassemble-all`, so `symbols cross` can correctly
cross-reference the symbols.
Now there is also a warning when symbols cannot be cross-referenced
correctly.
cc. @hohle I noticed this issue on your stream
Before:
```
$ tools/make-config.py --version hd cen
✔ generating psx splat config
✔ splitting config/splat.hd.stcen.yaml
✔ adjusting files at src/st/cen
✔ disassembling matched functions
✔ finding duplicates across overlays
✔ adding cross-referenced function names
✔ renamed 99 functions, splitting again
✔ cross-referencing 0 functions
✔ adding cross-referenced symbol names
```
Now:
```
$ tools/make-config.py --version hd cen
✔ generating psx splat config
✔ splitting config/splat.hd.stcen.yaml
✔ adjusting files at src/st/cen
✔ disassembling matched functions
✔ finding duplicates across overlays
✔ adding cross-referenced function names
✔ renamed 99 functions, splitting again
✔ splitting config/splat.us.stcen.yaml 👈 NEW!
✔ cross-referencing 98 functions 👈 FIXED!
✔ adding cross-referenced symbol names
✔ renamed 56 data/bss symbols, splitting again 👈 56 instead of 0!
```
- Decompiled duplicate function func_us_801714F4
- Decompiled duplicate function func_us_80172904
- Decompiled duplicate function func_us_8017293C (renamed
Tt001UpdateAnim per tt_000 duplicate)
- Decompiled duplicate function func_us_80172B50
- Decompiled duplicate function func_us_80172B88
- Added EntitySearch D_80170EE4[] to servant.h to be used by
func_us_80172B88 similar to duplicate in tt_000
A lot of these are in the same order in tt_000, so I would assume they
could be moved to shared static functions at some point.
this one was trickier and larger than my previous contributions. I'm not
sure what the best way to rename the symbols are, so I expect to receive
some comments on that.
- `column` util was missing from the Docker image, added it and
organised the requirements file so it's more human friendly
- rearranged the USER argument in the Dockerfile to avoid the scenario
where you didnt specify the user and so lose the layers where apt
installed all the packages
- add the python deps at the end to ensure docker can reuse cached
layers if a python dep changes.
- update maspsx (I wanted to test latest maspsx against sotn, so this is
what led to the other changes!)
In analyze_calls, we need to analyze a function's calls, and be able to
track where they go. Given that a `jal` call only has a function name,
it's possible for it to go to a function in any overlay. Therefore, we
need to disambiguate across overlays.
We do this by identifying what overlay every function is in. This comes
from the filename of the assembly file.
Since the overlay might be in, for example, asm/us/dra (a "level 3
overlay" because it has 3 directory names) or asm/us/st/cen (a "level 4
overlay") we had special logic for testing for "st".
Now that we can also have level 4 overlays in "boss" (with Maria and BO3
coming up) we need to test for those too.
While we're at it, we also only run on the "us" version's assembly
files.
The decompilation script did not like to decompile anything from tt_001
because it ended in us.c. This would cause it to find muliple (5) paths
and would halt.
Change moves the code to F84.c in line with the code split in tt_000
Fixed a tab matching issue with the `check` target. These were matching
'\' and 't' as separator characters rather than '↹' (horizontal tab)
Made `tools/make-config.py` executable.
`w_030` expects an `s16` at entity offset 0x90. All of the existing uses
of `unk90` were already using specific extension structs which declare
that offset to be an `s16`. `unk90` was changed from an `s32` to an
`s16` a new `s16 unk92` field was added.
It turned out that `ET_WeaponUnk044` was only split because it was
expecting an `s16` at offset 0x90 and could use `ET_Weapon` now.
A few other extension structures were modified for clarity while trying
to fit them into one of the more generic structs.
Followed [the
guide](https://github.com/Xeeynamo/sotn-decomp/wiki/Decompilation#add-new-overlay)
which seemed to work relatively painlessly for this overlay.
I hit an issue in `make-config.py` where no duplicates were found and
have added a separate commit which fixed that. Otherwise no issues with
guide.
```
josh@JoshsPC:/mnt/c/dev/sotn-decomp$ tools/make-config.py tt_001
✔ generating psx splat config
✔ splitting config/splat.us.tt_001.yaml
✔ adjusting files at src/servant/tt_001
✔ disassembling matched functions
✖ finding duplicates across overlays
Traceback (most recent call last):
File "/mnt/c/dev/sotn-decomp/tools/make-config.py", line 1132, in <module>
raise e
File "/mnt/c/dev/sotn-decomp/tools/make-config.py", line 1126, in <module>
make_config(args.input, args.version)
File "/mnt/c/dev/sotn-decomp/tools/make-config.py", line 1106, in make_config
if found > 0:
TypeError: '>' not supported between instances of 'NoneType' and 'int'
```
I was also able to remove the weird `goto block` on `RicMain`.
The HD version seems to be an older version. I am not entirely sure of
the US changes, but it seem to fix a series of bugs when Richter gets
grabbed by Gaibon? There's a new debug string which ints
`g_Player.unk7A` being named as `run_disable_f` but I did not yet rename
it.
Fixes a few issues found by @bismurphy where certain symbols that were
not supposed to be added, were added.
The major fix is when the second pass found a meaningful symbol name
like `MakeExplosion`, the third pass could add a new symbol with the
same offset but with the default splat name `func_8019055C`. With the
new `add_symbol_unique` the symbol table will stay unique. On top of
that when adding a new symbol `func_8019055C` can be replaced by
`MakeExplosion` but not the other way around. This will ensure only
meaningful names will be resolved.
I went through all of CEN, cross-referencing it with WRP to make as many
files match as possible. This involved pulling in data (which had been
pulled in on WRP but not on CEN yet), renaming shared variables to not
have placeholder names, adjusting file splits, and more.
EntityBreakable seems to be very different here, so I did not do any
deduplication on that one yet. I think we need to get a robust system
for deduplicating across stages so we can share .h files even when the
stages are different. Right now there's no way for an included .h file
to know it's part of CEN (at least, that I know of).
This is similar to other cutscenes but with a few minor differences. Of
note is that the PSX version uses an uninitialized pointer.
Pulls in the remaining duplicates and `func_us_8018C90C` as well
finishing off the remaining functions in `boss/mar`.
not sure where to put these externs, I didnt see a header file for this.
also, the `D_us_8019AF2A[0] ^= 1;` construct is sus for two reasons:
1) this is A bytes away from g_CutsceneFlags, and
2) I could only get a matching decompile by making these arrays and
accessing the first element, even though a ptr would make more sense to
me.
Improve `make-config.py` to internally use the `symbols.py cross` and
cross-reference symbols from duplicate functions. This allows to add
even more symbols for new overlays with a third pass.
This was one of my biggest pain points when I was de-duplicating the
boss `MAR` overlay as I was not able to directly copy&paste duplicate
functions or include shared ones without having to individually fix the
symbols coming from `.data` and `.bss`. This is now automated.
```
$ python3 ./tools/make-config.py ric --version hd
✔ generating psx splat config
✔ splitting config/splat.hd.ric.yaml
✔ adjusting files at src/ric
✔ disassembling matched functions
✔ finding duplicates across overlays
✔ adding cross-referenced function names
✔ renamed 141 functions, splitting again
✔ cross-referencing 141 functions
✔ adding cross-referenced symbol names
✔ renamed 145 data/bss symbols, splitting again
```
I found `estimate_gnu_c_function_begin` might not be reliable all the
times. This happened with RIC HD. I found a way to circumnavigate the
issue for the time being.
Adds a spinner used as an indicator that the tool is working. I broke
down the duplicate function finder into multiple steps to give faster
feedback to the user.
I am now checking the duplicates only between overlays that make sense
to search to. Splitting `ric` will only cross-reference it with `ric
us`. Same for the servants, stages and the `sel` stage. On my machine
this change trimmed down the execution time from 1m25s to just 10s.
The spinner is very easy to use. Just use `spinner_start` instead of
`print`. Use `spinner_stop(True)` to terminate it and go to a new line,
but a new `spinner_start` will do that for you. Use
`spinner_stop(False)` when an operation fails.
This can be merged independently from #1643 and #1644.
