Improved PR compared to #1704 .
We decided that unkB8 shouldn't be part of the Ext since it appears to
apply to all entities.
Also discovered some issues with the ET_GurkhaHammer struct, so resolved
those as well.
Now all accesses to the unkB8 offset go directly to `self->unkB8`.
Unrelated to all this unkB8 stuff, I found that there was an extra C
file in DRE which was not being used. Its code had been moved to
succubus.c, and it looks like the original file was accidentally left in
place. I deleted it here.
Primary goal here was to eliminate `ext.generic.unkB0`. It turns out
that the only entity that reaches up into this unkB0 region is the
Subweapon entity.
I went through and changed these all to `ext.subweapon.subweaponId`, but
also did some analysis on all the uses to make sure this was an accurate
change to make.
Other highlights:
- Naming CheckSubwpnChainLimit and its RIC counterpart
- Moving the RIC version of that function to the next file, split makes
more sense this way.
- Made both versions of that function `static`
- renamed subweapondef's unk6 to be chainLimit
- Changed g_unkGraphicsStruct.unk0 to be called `pauseEnemies`. It's
true when the stopwatch subweapon is used, and in cutscenes. Found this
due to looking at the stopwatch subweapon setting this value. Also made
it a bool since it's only ever used as a bool.
- Added some playersteps enum uses where they were still using hex
values
- Studied RicEntityStopwatchCrashLightning in order to give it that name
I think that's about it. Overall just some modest cleanup, but continues
to make the game code more readable.
Decompilation of func_us_80171864
Adding ET_Ghost for the Entity.ext union.
Thanks to @sozud for setting up a PSP environment for me.
Thanks to @joshlory for figuring out the last bit.
Main motivation here was to eliminate `ext.generic.unkB2`, which no
longer exists and has been removed from the struct. While I was at it, I
modernized the holy water function.
The argument is "self" instead of "entity"
We use FIX() where relevant
Function is adjusted to match on PSP
Variables are renamed
Factory calls are commented
I think those are the main highlights. Mostly small stuff, but
everything comes step by step.
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(),
```
`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 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.
`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.
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`.
I went through every use of g_CastleFlags and created enum members for
the different flags. This will allow us to better keep track of what
flags exist and how they are used. This will also aid in tracking the
arrangement of the overlays in the flags, as there seems to be some
level of organization. As we add more gameplay overlays we can hopefully
learn how these all work together.
Some functions were using local `const int` declarations to give names
to flags. For example, there was `const int jewelSwordRoomUnlock = 51;`.
For these ones, which had flag purposes identified in code, I went ahead
and gave their global castle flag a similar name (in this case,
`JEWEL_SWORD_ROOM`). For all other flags, I kept the name as a
placeholder of `CASTLE_FLAG_###`; later someone else can identify what
the individual flags are.
This should be a good step in making the castle flags more
understandable, as well as to connect the different functions across the
game and properly see which ones are touching the same flags. When a
castle flag is written as 0x32 in some places, and 50 in others, it
makes it harder to search. Now that we can use CASTLE_FLAG_50, hopefully
we will have better understanding of all the uses of the flag.
I added some of the correct labels to the unknown field on the
RoomTeleports. The unknown field is used by the CD Rooms to reload the
tileset if you move half way through the room but then decide to turn
back. When not used in a CD Room I don't know if it has any effect. But
some warps that aren't used in a CD Room do assign this field.
Based this off other Josh's work on https://decomp.me/scratch/ncrbI
Noticed there was already a define in `libgte.h` for Vector0 which I've
used, but also filled out the other two.
Hope this is ok for a first PR 😄
Let me know if anything needs changing.
Co-authored-by: Josh Schreuder <jschreuder@mediaocean.com>
`sotn-lint` will now convert `flags` and `drawFlags` fields from
integers to enum values.
`sotn-lint` will no longer overwrite files that haven't changed, so
running `make format build` only has to rebuild files that are modified.
The logic from `DrawModeTransformer` has been pulled out into
`BitFlagLineTransformer` which will handle any type of bitmasked enum
type.
Not a big PR. The highlights are `g_Player.timers` and
`entity->hitParams`. The last one is a bit weird as it looks like a mix
between `attackElement` and `hitEffect`.
The `var_s4` on `RicMain` is uninitialised and it leads to an undefined
behaviour. Recompiling the code for PSX by moving things around, that
variable will not be `0` at the beginning but some random value. This
prevented Richter's run, blade dash and high jump to work. I added an
`#ifdef` in case people wants to mod the overlay.
This is probably the last batch of changes for RIC for a while.
List of the most important changes:
* The two fields in `FACTORY` are now reversed; what used to be
`FACTORY(0x2000, 44)` is now `FACTORY(44, 0x20)`
* Introduce `WFACTORY` to imply `((g_HandId + 1) << 0xC)` in it
(`((g_HandId + 1) << 0xE)` is not yet cracked)
* Remove `assets/ric/blueprints.json` in favour of baking the info into
a C file to easily link enums
* Add `B_MAKE` to define a blueprint entry
* Document almost all RIC entities
* Document almost all RIC blueprints
From the [public
announcement](https://discord.com/channels/1079389589950705684/1079395108501331990/1276310695964835910)
on Discord:
> We've been progressing very far with the project and I started
wondering what would be the best course of action to protect the work of
existing and future contributors. I personally consider contributions to
a decompilation project as creative work. Therefore it is my desire to
protect this creative work from being re-claimed by other people. This
is especially true in the light of the ability of modding the existing
decompiled code and create derivative work.
>
> I talked privately with the current most active contributors. And me,
@sozud , @bismurphy and @joshlory so far agreed on pushing through the
AGPLv3 license for all the decompiled code in `sotn-decomp/src`,
excluding any third party code and decompiled code from the PSX SDK. But
I've been wanting to ask all of you if you are fine your code to be
licensed and protected under the AGPLv3 terms.
>
>
https://raw.githubusercontent.com/Xeeynamo/sotn-decomp/89686514916cabd0dd88ae0387f749a889c19e05/src/LICENSE
>
> I put and all the contributors, which can be accessed to the page
https://github.com/Xeeynamo/sotn-decomp/graphs/contributors . Again,
this will work for existing and future contributors. In the license I
put my full name as I will be responsible of any legality. And I
understand not every contributor wants to disclose their identity.
Please give an approval to my latest PR if you do not have any
objection. If not, please write here publicly.
>
> PS. I understand past contributors will either not see the message
above or decide to ignore it. If I do not hear from anyone of the
contributors for more than one week between those who did not interact
with me or the server, I will still go ahead and apply the license. It
is not in my intention to ignore past contributors. But I do not want
contributors who have been inactive for months or years to be a blocker.
Please do not mistake this as an act of malevolence.
---------
Co-authored-by: Joey Murphy <tjmurphy@mit.edu>
Follow-up to #1569 .
Loads of changes, where the most prominent one is synchronising the
`self->ext.*.subweaponId` field across entities to make it work on
`RicSetSubweaponParams`. I added some static assertions with the new
macro `SYNC_FIELD`, which is easier to read.
The new `enum RicSubweapons` decouples the subweapons of Richter from
those found in DRA. This is important because
`assets/ric/subweapondefs.json` is different from the definition stored
in DRA.
I discovered new entities and renamed them accordingly. I am following
the format `RicEntity` for each entity. Then `RicEntitySubwpn` or
`RicEntityCrash` for the appropriate category.
The BSS section has been assigned to the right places and `ric.h` has
been cleaned as most of the declarations are no longer necessary. More
data and functions have been marked as `static`.
`src/st/st0/2DAC8.c` had a reference to `.ext.agunea`, which was
probably put there by mistake. The fake symbol `D_801758D0` has been
removed and I think I found a way to fix the stopwatch subweapon without
the `#ifdef`.
```c
#ifdef VERSION_PC
#ifdef _MSC_VER
{
s32 temp_x = (rcos(primLine->angle) << 8);
s32 temp_y = (-(rsin(primLine->angle) << 8));
primLine->velocityX = *(f32*)&temp_x;
primLine->velocityY = *(f32*)&temp_y;
}
#else
primLine->velocityX = (f32)(rcos(primLine->angle) << 8);
primLine->velocityY = (f32) - (rsin(primLine->angle) << 8);
#endif
#else
primLine->velocityX = (rcos(primLine->angle) << 8);
primLine->velocityY = -(rsin(primLine->angle) << 8);
#endif
```
this one has been resolved by just using `velocityX.val`.
I documented all the `PLAYER.step` for Richter and got the meaning of
most of the functions with their parameters. There are some additional
renaming and magic numbers documented that are not worth to mention
individually.
I plan to get rid of `EntityTypes` as it is not sustainable to document
256 combination of entities. Instead I am thinking it would be a better
approach to document the `g_Entities` indices as ranges. For example
from `32` to `47` we have particles, from `48` to `64` we have player
entities that can hit the stage entities and so on.
I re-organised and re-imported some of the data close to the function
that use them. This will allow me to later split entities into their own
units as separate C files.
In addition to decompiling the function, two big changes were needed:
- Decompiling this function uncovered a maspsx bug with generating `nop`
padding instructions; that is now fixed and this PR includes a maspsx
update.
- We also discovered (thanks @mkst !) that g_Tilemap.bg was not actually
a member of g_Tilemap. I renamed it to g_BgLayers, and did a
find-and-replace across the repo to change all references. This appears
to have no impact on any existing function, but leads to the correct
register loading on this function.
It's a weird one, and was tricky to get matching (actually, this was a
super old decomp.me browser tab I discovered was still open, which is
why I came back to it), but these tricky ones are great for discovering
where we have mistakes in our structure of the game's data.
This one was already pretty good, but mostly needed to have
`ext.generic` removed.
I noticed the structure of this file is identical to e_bone_scimitar, so
I decided to change the bone scimitar to use this Skeleton `ext` since
the skeleton is a more generalized enemy. Now that we see two enemies
both using this one, this almost looks something like object
inheritance, if that were a thing that existed.
So far NZ0 is the only overlay with the skeleton enemy, but I'm sure
once we pull in other overlays we'll find more skeletons all over. Then
this will be a .h file, but for now it's a normal .c file.
This file was all decompiled, but overall had a lot of "old-looking"
traits to it. I went through and updated it in many ways. All the
functions are now named with real names (no `func_whatever`), several
constants are used in different places, I've added names to the list of
entity ID numbers in the .h file, etc.
One thing of interest: I went through starting to document the `.ext`
members, and noticed that they seemed to "keep away" from each other,
even across unrelated entities. Therefore, at least for now, I decided
to create an experimental `ext` which covers all the entities that
needed defining in this file. Maybe down the line we can split them up
to their own individual entities, but I find it interesting that some
entities use the unk80, unk84 offsets, while others only use unk88. I
don't see why you would skip to unk88 unless unk80 and unk84 were taken,
and the only way they're taken is if they share ext members.
The big thing is that nothing uses generic anymore. It looks like this
file was from the era when we were trying to use generic for everything
so there is lots of ugly casting, but that's all gone now, which is
nice.
A minor PR that only aims to give a meaningful name to two of the
unknown variables in the `ET_ZombieSpawner` struct:
- `spawnDelay`: Number of frames until the next time the Zombie Spawner
will attempt to spawn another zombie
- `spawnSide`: Which side of the player the Zombie Spawner will attempt
to spawn the next zombie (zero for left, non-zero for right)
