pret / pokeheartgold

Decompilation of Pokemon HeartGold/SoulSilver
311 stars 97 forks source link

move overlay files to their own folders #266

Open red031000 opened 10 months ago

red031000 commented 10 months ago

lucky made me do this

all files that belong to overlays should be within their own folders, preferably those folders being inside an overlay folder

preferably the format should be

src/overlay/battle/{src,include}/file.c

this can be debated

lhearachel commented 10 months ago

I don't like breaking the paradigm of the top-level folder split being src/include; embedding that deeper seems like a breach of C norms that isn't really worth it and is potentially just confusing.

I think the overlay subfolder is also unnecessary.

How will this work for linked overlays? e.g., the battle code is split across multiple different overlays.

red031000 commented 10 months ago

ideally it would be so that each overlay has its own folder, so it might need to get specific for stuff like battle

lhearachel commented 10 months ago

I don't mind the idea of, e.g., src/(overlay/){battle_eng,battle_ui,battle_ai} (overlay subfolder optionally omitted, depending on how the full structure looks).

luckytyphlosion commented 10 months ago

Issue name shouldn't be an implementation task when consensus hasn't been decided.

lhearachel commented 6 months ago

I would like to propose the following structure, using a few example overlays for illustration. For brevity, only include is shown here, but src would follow the same structure.

project
│   README.md
│   ...
│
└───files
│   ...
│
└───include
    │   global.h
    │   main.h
    │   ...
    │
    └───berry_pots_app
    │   │   berry_pots_app.h
    │   └───berry_pots_app_internal.h
    │
    └───battle
        │   battle.h
        │   battle_system.h
        │   battle_controller_player.h
        │   ...
        │   
        └───trainer_ai
        │   └───trainer_ai.h
        │   
        └───link_battle
            └───link_battle.h

This structure clarifies in the directory hierarchy that some overlays are coupled to others, and thus a child overlay (e.g., trainer_ai) requires a parent overlay to also be loaded (e.g., battle) in order to be functional. Additionally, due to the hierarchical nature, we can select names that avoid overall namespace pollution while still keeping related overlays close to one another; if we went with a flatter directory structure of one folder to one overlay, we would need to employ some sort of prefixing system to achieve that (e.g., battle_trainer_ai).

The downside to this is that nested directories can get messy to fully integrate with a build system. But I think the trade-off of effort investment to usability is worthwhile.

github-actions[bot] commented 1 month ago

This issue has had no activity for 60 days and will be marked stale. If there is no further activity, it will be closed in 30 days.