Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: game.h Header Refactor #1406

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft

Conversation

hohle
Copy link
Contributor

@hohle hohle commented Jul 11, 2024

The monolithic game.h pulls in effectively all headers and definitions and is required by nearly every file. When decompiling a new function, this produces a 5K line context.

This refactors game.h into logical groups which are then included back into game.h to produce an equivalent header for when that's necessary or convenient, but allows individual C files to include just the headers they require.

This refactor is not optimal, but does improve context generation for something like src/st/dre/entity_heart.drop.c, which is now reduced to 2.5K line context and stage headers are around 3K.

There are several unknown vars which make it difficult to factor out everything at the moment, so some areas will still require game.h.

This also fixes include guards in many headers to avoid multiple inclusion issues.

By including game.h in stage.h, this can be cleanly split between the header refactoring and the #include changes in src.

@hohle hohle force-pushed the header-refactor branch from a2023dd to fad8af8 Compare July 11, 2024 20:34
@sozud
Copy link
Collaborator

sozud commented Jul 13, 2024

I haven't tried this but if you want to cut down on #includes this might help https://github.com/include-what-you-use/include-what-you-use

I'm not sure I have a good sense of how much headers should be divided up or not. If the issue is big decomp.me contexts you can use the decomp-permuter import to decomp.me feature to automatically cut it down.

This PR does touch a lot of files so I think breaking it up into a few PRs might make sense? I'd recommend getting Xeeynamo's opinion before reworking the PR though

@sozud
Copy link
Collaborator

sozud commented Jul 18, 2024

Going to respond to this post here https://discord.com/channels/1079389589950705684/1079395108501331990/1261704573953904742

getting sotn headers and typedefs out of psxsdk/libcd.h and psxsdk/libetc.h

https://github.com/Xeeynamo/sotn-decomp/blob/master/include/psxsdk/libcd.h
vs.
https://github.com/sozud/psy-q/blob/master/3.5/PSX/INCLUDE/LIBCD.H

Are you talking about getting rid of types.h? So part of the problem is that the types are messed up in the original libraries, the two biggest issues are that sizeof(long) is not the same on various platforms and the O_TAG issue.

and having "mixins" include their own headers to avoid repeating includes across all stages.

Could you explain this in more detail?

@hohle
Copy link
Contributor Author

hohle commented Jul 25, 2024

getting sotn headers and typedefs out of psxsdk/libcd.h and psxsdk/libetc.h

https://github.com/Xeeynamo/sotn-decomp/blob/master/include/psxsdk/libcd.h
vs.
https://github.com/sozud/psy-q/blob/master/3.5/PSX/INCLUDE/LIBCD.H

This is about trying to remove cycles from the dependency graph. types.h is a sotn-decomp header, sotn-decomp relies on psxsdk/PSY-Q, so having psxsdk include a sotn-decomp header (it now depends on sotn-decomp introduces a cyclical dependency. I'm trying to break that cycle and keep psxsdk headers "clean" while matching the types described in the docs.

Are you talking about getting rid of types.h? So part of the problem is that the types are messed up in the original libraries, the two biggest issues are that sizeof(long) is not the same on various platforms and the O_TAG issue.

Since these types aren't 64-bit clean, sized types might be more appropriate. In that case, moving types.h into psxsdk, using stdint.h (and back porting those defs for GCC 2.6) or something similar could be done. I tried finding where the s/u 8/16/32 convention comes from since it seems pretty common in the decomp community, but is not something I've come across outside, but mirrors stdint in everything but type names.

and having "mixins" include their own headers to avoid repeating includes across all stages.

Could you explain this in more detail?

Right now an implementation C file might include something like "../animate_entity.h" to pull in that shared definition. This works similar to what dynamic languages would call mixins. However, the root object source is currently required to also include all headers required for those implementations as well. This means every stage needs to know implementation details of each of this shared files to satisfy their dependencies, resulting in a lot of duplication or unnecessary inclusion.

With the header files split out and consistently guarded, mixins can freely include the header they need to compile without the dependent C file needing to know what those requirements are. For example, instead of having every stage C file set up the includes:

// some src/st/example0/file.c
#include <entity.h>
#include "../animate_entity.h"
// some src/st/example1/file.c
#include <entity.h>
#include "../animate_entity.h"
// src/st/animate_entity.h
u8 AnimateEntity(u8 frames[], Entity* entity) {
⋮
}

The stage feels can simply include the shared implementation which will pull in all of its dependencies:

// some src/st/example0/file.c
#include "../animate_entity.h"
// some src/st/example1/file.c
#include "../animate_entity.h"
// src/st/animate_entity.h
#include <entity.h>
u8 AnimateEntity(u8 frames[], Entity* entity) {
⋮
}

My hope was to find logical boundaries for many of the global vars and types that are shared across main, dra, stages, weapons, and familiar objects and hopefully discover some reasonable dependency graph (e.g. stages know about entities, and a set of core game globals, but certain details of main and dra are "private", likewise, dra depends on a stage overlay definition as its API and doesn't know any additional stage details). I'm not sure if those boundaries exist or are cleanly defined. This was an initial attempt to begin defining those layers in a way that would also (possibly) aid in decomp by reducing the space of types and globals to just what could be possible rather than the entire space.

@sozud
Copy link
Collaborator

sozud commented Jul 26, 2024

In that case, moving types.h into psxsdk, using stdint.h (and back porting those defs for GCC 2.6) or something similar could be done. I tried finding where the s/u 8/16/32 convention comes from since it seems pretty common in the decomp community, but is not something I've come across outside, but mirrors stdint in everything but type names.

My preference is u32 over uint32_t just for brevity.

Xeeynamo gave the PR a heart emoji so it sounds like he's onboard, I'd just recommend splitting up the work into multiple PRs

@Xeeynamo
Copy link
Owner

let's not have a big bang PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants