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

New assets system #1904

Open
Dragorn421 opened this issue Feb 28, 2024 · 5 comments
Open

New assets system #1904

Dragorn421 opened this issue Feb 28, 2024 · 5 comments

Comments

@Dragorn421
Copy link
Collaborator

Dragorn421 commented Feb 28, 2024

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 #1730 (comment)
v1->v2: cf discord discussion linked above
v1: assets/_extracted/, cf #1730 (comment)

current zeldaret/main layout

For reference.

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

-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 bf37ad1 and before 9def6f4):

-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 a6f646d and before bf37ad1
(didn't write it out, in-between v1 and v3: has text in extracted/ but not baserom files)

v1 (before a6f646d):

-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
Copy link
Collaborator

fig02 commented Feb 28, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

#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
Copy link
Collaborator Author

re: #1904 (comment)

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
Copy link
Collaborator Author

#1973 (review) : to be changed by new assets : don't reference gkeep globally, unduplicate xmls

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

No branches or pull requests

2 participants