search

zeldaret / oot

Decompilation of The Legend of Zelda: Ocarina of Time
4.58k stars 578 forks source link

New assets system #1904

Open Dragorn421 opened 7 months ago

Dragorn421 commented 7 months ago

Just need a home for the directory layout we agreed upon, so I can find it again

Productive discussion on discord happened from https://discord.com/channels/688807550715560050/688851317593997489/1207388581639880745 to https://discord.com/channels/688807550715560050/688851317593997489/1208159004694413312

The general result is a new top level folder extracted/ for extracted things, including assets, text and baserom segments

I compiled the changes to "future layout", see below

in the following directory structure layouts: [first/part/]second/part paths are where the file does #include "second/part", and CPP resolves it to first/part/second/part (due to first/part being in the include path)

future layout

"future layout v4":

Include path would be like `-Ibuild/VERSION -Iextracted/VERSION` ``` # v4 assets/ # note: everything under assets/ committed objects/ gameplay_keep/ gameplay_keep.c -> [extracted/VERSION/]objects/gameplay_keep/gameplay_keepVtx_00C0A0.vtx.inc , [extracted/VERSION/]objects/gameplay_keep/deku_stick.i8.inc.c gameplay_keep.h text/ nes_message_data_static.c -> [build/VERSION/]assets/text/message_data.enc.h message_data.h -> [extracted/VERSION/]text/message_data.h xml/ objects/ gameplay_keep.xml extracted/ # .gitignore'd VERSION/ baserom/ gameplay_keep objects/ gameplay_keep/ deku_stick.i8.png # written on make setup for reference, but otherwise unused deku_stick.i8.inc.c # written on make setup (does *not* depend on the png) gameplay_keepVtx_00C0A0.vtx.inc text/ message_data.h build/ VERSION/ baserom/ gameplay_keep.o # objcopy'd from extracted/VERSION/baserom/gameplay_keep assets/ text/ message_data.enc.h # processed from assets/text/message_data.h ```

history:

v3->v4: removed extracted/VERSION/text/message_data.enc.h (no clue why it was there) v2->v3: fix oversimplified text handling. cf https://github.com/zeldaret/oot/pull/1730#issuecomment-1948247178 v1->v2: cf discord discussion linked above v1: `assets/_extracted/`, cf https://github.com/zeldaret/oot/pull/1730#issuecomment-1932832284

current zeldaret/main layout

For reference.

"current zeldaret/main layout v4" (since 9def6f4d0d252e45a71d0d2ee5d9042023d84315):

`-Ibuild/VERSION -Iextracted/VERSION` ``` assets/ text/ message_data.h -> [extracted/VERSION/]text/message_data.h nes_message_data_static.c -> [build/VERSION/]assets/text/message_data.enc.h xml/ objects/ gameplay_keep.xml extracted/ # .gitignore'd VERSION/ baserom/ gameplay_keep text/ message_data.h assets/ objects/ gameplay_keep/ gameplay_keep.c -> ./gameplay_keepVtx_00C0A0.vtx.inc , [build/VERSION/]assets/objects/gameplay_keep/deku_stick.i8.inc.c gameplay_keep.h deku_stick.i8.png gameplay_keepVtx_00C0A0.vtx.inc build/ VERSION/ assets/ objects/ gameplay_keep/ deku_stick.i8.inc.c # processed from extracted/VERSION/assets/objects/gameplay_keep/deku_stick.i8.png on make (depends on the png) text/ message_data.enc.h # processed from assets/text/message_data.h baserom/ gameplay_keep.o # objcopy'd from extracted/VERSION/baserom/gameplay_keep on make ```

History:

v3 (since bf37ad1368383f0b5812a7b077915a67df57041d and before 9def6f4d0d252e45a71d0d2ee5d9042023d84315):
`-Ibuild/gc-eu-mq-dbg -Iextracted/gc-eu-mq-dbg` ``` assets/ objects/ # .gitignore'd gameplay_keep/ gameplay_keep.c -> ./gameplay_keepVtx_00C0A0.vtx.inc , [build/gc-eu-mq-dbg/]assets/objects/gameplay_keep/deku_stick.i8.inc.c gameplay_keep.h deku_stick.i8.png gameplay_keepVtx_00C0A0.vtx.inc text/ message_data.h -> [extracted/gc-eu-mq-dbg/]text/message_data.h nes_message_data_static.c -> [build/gc-eu-mq-dbg/]assets/text/message_data.enc.h xml/ objects/ gameplay_keep.xml extracted/ # .gitignore'd gc-eu-mq-dbg/ baserom/ gameplay_keep text/ message_data.h build/ gc-eu-mq-dbg/ assets/ objects/ gameplay_keep/ deku_stick.i8.inc.c # processed from assets/objects/gameplay_keep/deku_stick.i8.png on make (depends on the png) text/ message_data.enc.h # processed from assets/text/message_data.h baserom/ gameplay_keep.o # objcopy'd from extracted/gc-eu-mq-dbg/baserom/gameplay_keep on make ```
v2: since a6f646dc659eb1711901fc05d82704b34a9c23fd and before bf37ad1368383f0b5812a7b077915a67df57041d (didn't write it out, in-between v1 and v3: has text in extracted/ but not baserom files) v1 (before a6f646dc659eb1711901fc05d82704b34a9c23fd):
`-Ibuild/gc-eu-mq-dbg` ``` assets/ objects/ # .gitignore'd gameplay_keep/ gameplay_keep.c -> ./gameplay_keepVtx_00C0A0.vtx.inc , [build/gc-eu-mq-dbg/]assets/objects/gameplay_keep/deku_stick.i8.inc.c gameplay_keep.h deku_stick.i8.png gameplay_keepVtx_00C0A0.vtx.inc text/ message_data.h # .gitignore'd nes_message_data_static.c -> [build/gc-eu-mq-dbg/]assets/text/message_data.enc.h xml/ objects/ gameplay_keep.xml baseroms/ gc-eu-mq-dbg/ segments/ gameplay_keep # written on make setup build/ gc-eu-mq-dbg/ assets/ objects/ gameplay_keep/ deku_stick.i8.inc.c # processed from assets/objects/gameplay_keep/deku_stick.i8.png on make (depends on the png) text/ message_data.enc.h # processed from assets/text/message_data.h baserom/ gameplay_keep.o # objcopy'd from baseroms/gc-eu-mq-dbg/segments/gameplay_keep on make ```
fig02 commented 7 months ago

Im not sure we had a sufficient amount of consensus on the issue of how to handle extracted textures specifically-- I recall only 3 people voicing support for the "extract both inc and png but only use inc" idea.

But this can possibly be discussed again and have a stronger agreement when it comes time to actually make this change.

Dragorn421 commented 7 months ago

Summary of the textures (as png) topic

Current main

In current main,

as an example the directory layout limited to handling a single texture looks like:

assets/
  objects/  # .gitignore'd
    gameplay_keep/
      gameplay_keep.c -> [build/gc-eu-mq-dbg/]assets/objects/gameplay_keep/deku_stick.i8.inc.c
      deku_stick.i8.png  # written by extract_assets.py on make setup
build/  # .gitignore'd
  gc-eu-mq-dbg/
    assets/
      objects/
        gameplay_keep/
          deku_stick.i8.inc.c  # processed from assets/objects/gameplay_keep/deku_stick.i8.png on make (depends on the png)

Chronologically, what happens is

On make setup, textures are converted from N64 format to png and written, like assets/objects/gameplay_keep/deku_stick.i8.png above. The .c files referencing them (such as assets/objects/gameplay_keep/gameplay_keep.c) include a file which path is similar, but with a .inc.c extension, like #include "assets/objects/gameplay_keep/deku_stick.i8.inc.c" This include does not use an absolute path, it is meant to grab the file at full path build/gc-eu-mq-dbg/assets/objects/gameplay_keep/deku_stick.i8.inc.c (using the include path that contains build/gc-eu-mq-dbg)

On make, the Makefile lists png files, converts each %.png to N64 format, and writes the N64 texture data as a C array's contents to build/gc-eu-mq-dbg/%.inc.c. Then the files including these written .inc.c, such as assets/objects/gameplay_keep/gameplay_keep.c, are compiled.

When a png is modified, the corresponding .inc.c is rebuilt accordingly (not the .c including the .inc.c though, until we set up dependencies (which requires splitting headers #1638 to be useful)) This allows the following workflow to change a vanilla texture: 1) Edit assets/objects/gameplay_keep/deku_stick.i8.png 2) touch assets/objects/gameplay_keep/gameplay_keep.c (so it's rebuilt on make) 3) make

Future

In the currently proposed future system,

as an example the directory layout limited to handling a single texture looks like:

# v4
assets/  # note: everything under assets/ committed
  objects/
    gameplay_keep/
      gameplay_keep.c -> [extracted/VERSION/]objects/gameplay_keep/deku_stick.i8.inc.c
extracted/  # .gitignore'd
  VERSION/
    objects/
      gameplay_keep/
        deku_stick.i8.png  # written on make setup for reference, but otherwise unused
        deku_stick.i8.inc.c  # written on make setup (does *not* depend on the png)

Chronologically, what happens is

On make setup, textures are converted from N64 format to png and written both as .png and .inc.c: extracted/VERSION/objects/gameplay_keep/deku_stick.i8.png and extracted/VERSION/objects/gameplay_keep/deku_stick.i8.inc.c

assets/objects/gameplay_keep/gameplay_keep.c is committed to the repo, it contains #include "objects/gameplay_keep/deku_stick.i8.inc.c"

On make, compiling gameplay_keep.c has extracted/VERSION in the include path, which makes the .inc.c include resolve to extracted/VERSION/objects/gameplay_keep/deku_stick.i8.inc.c

In this scheme, extracted/ is to be considered read-only: making modifications should be done by adding files in assets/ and changing the includes from resolving to extracted/... to instead include added assets/ files. It is not expected for the png or anything in extracted/ to be modified, in particular editing the .png will have no effect on the corresponding .inc.c (both are written at once on make setup)

For example this allows the following workflow to change a vanilla texture:

1) Copy extracted/VERSION/objects/gameplay_keep/deku_stick.i8.png to assets/objects/gameplay_keep/deku_stick.i8.png 2) Edit assets/objects/gameplay_keep/deku_stick.i8.png 3) Change #include "objects/gameplay_keep/deku_stick.i8.inc.c" (which resolves to extracted/VERSION/objects/gameplay_keep/deku_stick.i8.inc.c) to #include "assets/objects/gameplay_keep/deku_stick.i8.inc.c" (which resolves to build/VERSION/assets/objects/gameplay_keep/deku_stick.i8.inc.c) 4) make

Discussion on "Future" choices

Why not generate the .inc.c from extracted/**/*.png on make rather than make setup

We could but since extracted/ is read-only it'd be more Makefile logic for no expected benefit. Note that if we were to use a catch-all %.png -> build/VERSION/%.inc.c Makefile rule including for pngs in extracted/, this would result in extracted/VERSION/objects/gameplay_keep/deku_stick.i8.png -> build/VERSION/extracted/VERSION/objects/gameplay_keep/deku_stick.i8.inc.c Then either build/VERSION/extracted/VERSION (which is ugly from having VERSION twice) should be on the include path for #include "objects/gameplay_keep/deku_stick.i8.inc.c" to work as expected, or we can't have that "catch-all %.png -> build/VERSION/%.inc.c Makefile rule"

Dragorn421 commented 3 months ago

1967 extracts like

extracted/VERSION/assets/objects/gameplay_keep/deku_stick.i8.png , which is different from current new assets "future layout v4" extracted/VERSION/objects/gameplay_keep/deku_stick.i8.png

Both use -Iextracted/VERSION so #1967 assets include like "assets/objects/..." and current new assets like "objects/..."

It would be fine to change new assets to also use extracted/VERSION/assets/ afaict

Then extracted/VERSION would contain folders baserom, text, assets It would not be possible to move (extracted/VERSION/)text inside (extracted/VERSION/)assets, because that would require changing #include "text/message_data.h" in assets/text/message_data.h to #include "assets/text/message_data.h", which would be the file recursively including itself (-I. comes before -Iextracted/VERSION in the include path, which we could change but it'd still be fragile and also the include path being in the current order is the better idea so custom assets in assets/ can override extracted/VERSION/assets/ if they have the same name)

Dragorn421 commented 3 months ago

re: https://github.com/zeldaret/oot/issues/1904#issuecomment-2184026145

I think it would be wise to just not have two files named the same (message_data.h) at all currently: assets/text/message_data.h, #includes extracted/VERSION/text/message_data.h maybe change to: assets/text/message_data.h, #includes extracted/VERSION/text/message_data_vanilla.h (idk) and then extracted/VERSION/text/message_data_vanilla.h can also move into assets, to extracted/VERSION/assets/text/message_data_vanilla.h

Dragorn421 commented 3 months ago

https://github.com/zeldaret/oot/pull/1973#pullrequestreview-2139884551 : to be changed by new assets : don't reference gkeep globally, unduplicate xmls