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.
I've been meaning to do this for a long time now.
The analyze_calls script running in the CI creates a .md file which acts
as a directory pointing to all the function graphs that were generated.
It also generates an HTML file. Because Github does not natively render
HTML files, but does render Markdown files, it is preferrable to have
this for the sake of easy viewing and linking within the gh-duplicates
branch.
We need to just move the generated file into the proper directory, which
will hopefully make it show up at
https://github.com/Xeeynamo/sotn-decomp/tree/gh-duplicates. I'm very
hopeful that this will work, and not be yet another "Fix CI" commit...
I also added a space to an output string. Doesn't really matter but
might as well make things look a tiny bit nicer.
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.
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.
This pull request fixes typo python function helper texts in
`tools/splat_ext/spritesheet.py`. I kindly request the repository
maintainers to review and merge it. Thanks! ❤️
Fixes a very annoying issue it has been bothering me for a while. Since
the introduction of the sotn-str tool in #663 the line output has been
wrong and I failed to realise it immediately. This caused issues like
`differ` reporting the wrong C line number for the correspondent
assembly output, wrong debug info and so on.
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.
This is some code I wrote a while ago and I think it might make sense to
add it to `decompile.py`.
This function takes in an assembly file, and will scan through it for
any jump tables. It will then find them in the asm folder, and move the
jumptable information into the assembly file for the function. This
means that users do not have to seek out rodata and paste it into the
asm file manually.
As an example, the user can now run `dec EntityAlucard` and it will
successfully decompile the function and insert it to the .c file,
because now when it passes the assembly to m2c, the rodata is there at
the bottom of the assembly function. Overall this has been a big
convenience for me and I hope it will be helpful for others.
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.
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>
A small, but hopefully useful, update to maspsx. Per
https://github.com/mkst/maspsx/issues/26 maspsx did not handle `nop`
injection when assembly macros immediately followed a load instruction.
The latest change adds some basic support - i.e. it fixes this known
issue with Frogger.
I've tested it against the master branch of sotn-decomp and got the 🆗
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.
As part of an upcoming PR, I would like to call decompile.py from an
external Python script. Therefore, to make this easy, I have moved the
main behaviors of decompile.py to a dedicated `main()` function, which
gets called with the arguments from `argparse`. The parsing, rather than
being out in the open in the file, is handled in the `if __name__==
__main__` block, since we shouldn't need to do parsing when calling
externally.
This change had one small breaking result, that `check_injected_code`
wasn't working since it was relying on `func` being an accessible global
variable. Now `func` is passed as a function argument, so it will not
error out. Otherwise, this PR should hopefully be completely "invisible"
when using decompile.py as it was used previously, but should open the
door to better external calling.
## What
Enhance the existing `Makefile` to build new rules for the Saturn side
of the decomp. This should allow more flexibility when adding new
overlays or when tuning existing rules.
## Changes
I separated part of the Saturn build process in the separate file
`Makefile.saturn.mk`. I realise that naming it `saturn.mk` would have
been enough, but I pre-pended `Makefile.` so it can be found right below
the main `Makefile` when listing files in an alphabetic order. I plan to
do the same with the psx and psp toolchain, therefore you will find
`include Makefile.*.mk` in the main `Makefile`.
I deleted all the game building process done with Docker. Now that we
have an established way to do it natively I think it is no longer
required. We can always run the entire buildchain within a Docker
container instead of having `_native` and `_docker`. Now all the
`_native` references are removed. `build_saturn_native` is now
`build_saturn`.
`check_saturn` is no longer responsible of stripping the ELF into a
binary. That is now part of `build_saturn`.
I removed the references to `_li.o` (e.g. `alucard_li.o`) and used
`.elf` instead, which is closer to how the PSX build chain works. If
`_li.o` was a better preference, please let me know.
I am no longer using `./compile_dosemu.sh`. Instead I am using the new
`$(DOSEMU)` to directly invoke the tool within the Makefile. I have done
that to reduce the amount of dependent files.
I tried minimising duplication as much as possible. We now have a list
of overlays found in `SATURN_OVL_TARGETS`. Each expected output triggers
a series of dependencies so seamlessly build everything. `Makefile` is
smart enough to call `$(SATURN_TOOLCHAIN)` only once. If the game was
already built but just one source or symbol file changed, triggering a
new `build_saturn` will only compile the modified overlay.
The Saturn ADPCM files are now extracted in `assets/` instead of
`build/`. I think `assets/` should contain all the uncompressed and
uncooked files. The list of PCM file is not hardcoded. Instead I am now
using `$(wildcard disks/saturn/SD/*.PCM)`. This now means the tool tries
to convert PCMs from `SDD0.PCM` to `SDF0.PCM` with no success. As the
tool is silently failing I decided to leave it as I wrote it.
## Problems
I rewrote everything thinking about concurrency in mind. But `make -j
build_saturn` gives some unexpected output on `stdout`. I did not dig
too much into it. I suspect it might be dosemu. This is not a stopper as
we were not using `-j` when building the game anyway.
I also noticed doing `VERSION=saturn make build` calls
`mipsel-linux-gnu-ld` for `stage_02`. I suspect it is calling the rule
`$(MAIN_TARGET).elf: $(MAIN_O_FILES)` and simply moving `include
Makefile.*.mk` above it should fix it. But then it would cause the same
problem if I split the PSX rules into their own separate file. We never
used `make build` by setting the env variable `VERSION`, so this is not
either a breaking change or a stopper.
## Post thoughts
I am happy with what I achieved so far. I used the knowledge I
accumulated when maintaining the PSX counterpart. Since I now better
understand how `make` works, I was able to make some better decisions in
the Saturn counterpart. For example triggering a new build when the
symbol list changes is something the PSX build chain lacks of. I think
in the future it would be nice to trigger `make extract` when either the
YAML or the symbol list changes.
This probably fixes the debug module, it crashes instantly when I use
the bat familiar which I'm guessing is related to the compiler
differences found previously. I also added it to the CI since it's often
not able to compile.
[As
mentioned](https://discord.com/channels/1079389589950705684/1135205782703570994/1154948845638254672),
since #595 some tools are not able to pick up some function names as the
symbol names are gone. This PR adds a quick tool to re-generate that
list from a map file and integrates it in the CI flow.
EDIT: CI is failing because the running Linter is from `master`, where
`mapfile-parser` is not installed.
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.
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.
Markdown file is currently not being created in the CI. This rearranges
how we manage that, and also adds a debugging line which might help. I
don't know the nature of the error, but this will attempt to solve it,
and if not, hopefully provide some kind of information to suggest why it
isn't working.
Small change here, just linking to the raw SVG so it's clickable. The
github preview makes the bubbles no longer operate as links.
While we're at it, would it be okay if I moved the function_graphs.md up
a directory, so that it sits alongside duplicates.md and functions.txt,
rather than being mixed in the function_calls folder with all the svgs?
Function was still using a placeholder even though its purpose is fairly
obvious.
I also changed it to take the relicid as an enum for its argument, so
that hopefully these will fill in more automatically when decompiling. I
also went through all the existing calls to this function, and changed
them to use the relic name enum in their calls, as well as treating them
as booleans. Some lines will show all three of these changes. A good
example is:
`func_800FE3A8(0xE) == 0` which becomes:
`!IsRelicActive(RELIC_HOLY_SYMBOL)`. We change the function name, we
change the argument to be the enum, and we change the ==0 to a simple !.
Overall I think this makes the code for these relics much more readable,
and definitely much more searchable.
When `analyze_calls` creates an SVG file, the name of that file consists
of the overlay name, then a dot, then the function name. This is
intended to ensure that we get a unique name for the file, given that
the function name alone will often lead to a duplicate. The function
finder was using an incorrect filename-finding logic.
This works on my machine, hopefully it will work in CI too.
Unfortunately this is the type of PR which we won't know if it works
until fully merging. The logic is fairly straightforward though, so
hopefully this performs as intended.
Following up #454 , #453 and related from @sozud , I started to convert
some previously defined headers into proper shared C code. The advantage
from the header is the C file is completely independent, it will have
its own `.data` and `.rodata` and it will be compiled once.
I also took the opportunity to move `include/st/AnimateEntity.h` to
`src/st/animate_entity.h` for consistency. I think the change is small
enough to be included as part of this PR.
Since we know the signatures of all the `Weapon` overlays and I get to
know what `Equipment.unk10` stands for, I renamed a bunch of stuff to
keep everything more coherent.
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 very strange.
All of the "normal" overlays have a function called TestCollisions which
is extremely large and complicated. I noticed this one in the
functions.md list, and specifically that this one is fewer instructions
than the others. I thought decompiling this one might help as a
simplified version to solve the other ones.
I was surprised to find that this is a completely different function,
and does nothing related to collisions at all. It only does a bunch of
Primitive handling. It is not relevant for the real TestCollisions, but
I figured I should decompile it anyway. Here it is.
I do not know what this does. The function call graph
https://raw.githubusercontent.com/Xeeynamo/sotn-decomp/gh-duplicates/function_calls/sel.TestCollisions.svg
does not indicate any callers. We should either determine what this does
and give it a useful name, or revert it to `func_801B410C`.
The behavior with the "scaled_b014" variable is unusual; I'm not sure if
it's possible to change some types somewhere to eliminate its usage.
I think those are all the main important points, more than happy to make
changes to this very weird function.
Another deficiency identified in maspx by the vib-ribbon project (see
https://decomp.me/scratch/QnOyB). As part of that
[fix](92c02bf8b6)
I reworked/simplified some of the logic inside maspsx. I've tested
against your master branch and got the 🆗 so am raising this PR with
the hope that it'll prevent you guys from encountering the same (or
similar) issue(s) - and that it'll be easy to debug any future bugs that
arise in maspsx.
Any questions/concerns, please just shout :)
In the interest of improving the diffs shown in "Update reports" commits
(example here:
50e507f0c0),
we recently changed the SVG generation code to be more deterministic,
such that any differences are only the result of actual changes to the
source code of the repo.
While perusing this latest diff, I noticed that the index.html is
showing as a "large diff", which is because the generated HTML code does
not contain any newlines. To be clear, HTML has its own tags which
control the lines in the final rendered page, but the actual text in the
HTML file does not care about whether the code is on multiple lines or
is all in one line.
For the sake of generating diffs, it is heavily preferred for the HTML
to run as multiple lines. With this change, the raw HTML file will be
more intelligible and diffs should be able to properly show individual
changes, rather than struggling to deal with changes that are
technically all within a single-line file.
I rewrote the logic on how `spriteparts` and `spritepartslist` were
extracted. They are now extracted under the new module `animset`, which
handles a list of Sprite Parts in the same JSON file. It is extracting
the information as a JSON and it is rebuilding it back into a binary
usable in-game. The extracted assets will have the `.animset.json`
extension.
It took me a lot of time to find a pattern that would work with the WRP
and the WEAPON overlays. I made myself sure to create a solution that
would work in all the case scenario I found so far. What I wrote will
work across all the overlays. It takes a while to find where the animset
banks are located and I think I will deal with it in a separate PR.
With this it would be possible to preview animations without using #235
but we would need to either convert it into something other programs can
read (HTML5+CSS, Aseprite, DarkFunction) or parse it with a custom-made
GUI tool.
As requested by Sozud, this will sort the callees and callers
alphabetically when generating the call graphs. Therefore, between two
runs of the tool, the result should be more deterministic.
This also has the nice side effect that, because sorting places capital
letters first, decompiled functions with names with capital letters
(such as **A**llocEntity) will be placed before those with func_, so
they are highlighted.
Of course, if a function's name changes, it will move around in the call
graphs, but hopefully this change will mean we only see movement in the
graphs between two CI runs in the places where relevant changes have
occurred.
