Separate Assets that Differ Between Versions

[hohle]

Building different versions of the game without cleaning caused build errors due to assets being written to the same files. These conflicting and differing assets have now been moved to gen/$version.

sotn-assets's spritesheet and rawgfx handlers now treats the name field as a relative path like other handlers. The created variable names are based on the last component of that path.

Different version builds must still be done serially. This does not make it possible to build different game versions in parallel. build.ninja, several generated C files, and assets that do not differ between versions are still overwritten during each build. Even if they don't conflict, reading a file as it is being written by another build may cause transitive failures.

[sozud]

@Xeeynamo knows the most about how assets are handled so a review would be helpful

[sozud]

So did you confirm that sound is working on this? I'd expect it to be broken without changes. For example

sotn-decomp/src/pc/sotn.c

Line 319 in 5dedceb

ReadToArray("assets/dra/vb_0.bin", D_8013B6A0, LEN(D_8013B6A0));

[Xeeynamo]

I've been trying, very unsuccessfully, to make a macro for those gen/ files to do something very similar to what you're doing. I am glad you found a way to make it happen. I have some questions and points to make.

1. Did you consider to modify the asset_path from each splat+asset configuration instead of prepending the version ID to the path name?
2. I noticed there are some assets that are not using the version path (example: headergfx, D_800C4CEC] or - [0x18688, subweaponsdef, subweapons, RicSubweapons, RicBlueprints]). What is the logic of modifying the path for certain assets and not others?
3. I noticed you covered DRA. Do you envision to migrate the assets of all overlays to have split asset directories?
4. My long-term plan is to have the Ninja script generator to build US+PSP+HD concurrently. build_all would be gone and build should be able to build everything without the need of specifying separate env variables.
5. I understand that picci is kinda broken at the moment. I am happy to get it back in a separate PR.

[hohle]

• Did you consider to modify the asset_path from each splat+asset configuration instead of prepending the version ID to the path name?

I don't recall if I considered that. it would solve the same issue, but I liked that most assets could still be shared and it's obvious which specific assets differ between versions. There may be something I'm not thinking about, however. If there were lots of conflicting assets separating at the splat config level might be preferable.

• I noticed there are some assets that are not using the version path (example: headergfx, D_800C4CEC] or - [0x18688, subweaponsdef, subweapons, RicSubweapons, RicBlueprints]). What is the logic of modifying the path for certain assets and not others?
• I noticed you covered DRA. Do you envision to migrate the assets of all overlays to have split asset directories?

Only the assets that differ in content between versions had paths modified. All other assets are identical or currently named differently, so it doesn't matter which version it comes from.

One disadvantage to this approach with current tooling is that it's reactive. When one version clobbers an asset a completely different version fails. sotn-assets could help identify these in the future (e.g. if an asset file exists and is different from what's about to be generated, emit an error).

• My long-term plan is to have the Ninja script generator to build US+PSP+HD concurrently. build_all would be gone and build should be able to build everything without the need of specifying separate env variables.

I think that plan can be a short term plan ;-)

$> time make
⋮ 
[1/3] check config/check.hd.sha
✅ build/hd/cen.bin
✅ build/hd/f_cen.bin
✅ build/hd/dra.bin
✅ build/hd/ric.bin
✅ build/hd/wrp.bin
✅ build/hd/f_wrp.bin
✅ build/hd/tt_000.bin
[2/3] check config/check.pspeu.sha
✅ build/pspeu/dra.bin
✅ build/pspeu/maria.bin
✅ build/pspeu/ric.bin
✅ build/pspeu/cat.bin
✅ build/pspeu/chi.bin
✅ build/pspeu/dai.bin
✅ build/pspeu/lib.bin
✅ build/pspeu/no2.bin
✅ build/pspeu/no3.bin
✅ build/pspeu/no4.bin
✅ build/pspeu/np3.bin
✅ build/pspeu/st0.bin
✅ build/pspeu/wrp.bin
✅ build/pspeu/tt_000.bin
[3/3] check config/check.us.sha
✅ build/us/main.exe
✅ build/us/DRA.BIN
✅ build/us/RIC.BIN
✅ build/us/CAT.BIN
✅ build/us/F_CAT.BIN
✅ build/us/CEN.BIN
✅ build/us/F_CEN.BIN
✅ build/us/CHI.BIN
✅ build/us/F_CHI.BIN
✅ build/us/DAI.BIN
✅ build/us/F_DAI.BIN
✅ build/us/DRE.BIN
✅ build/us/F_DRE.BIN
✅ build/us/LIB.BIN
✅ build/us/F_LIB.BIN
✅ build/us/MAD.BIN
✅ build/us/F_MAD.BIN
✅ build/us/NO0.BIN
✅ build/us/F_NO0.BIN
✅ build/us/NO1.BIN
✅ build/us/F_NO1.BIN
✅ build/us/NO2.BIN
✅ build/us/F_NO2.BIN
✅ build/us/NO3.BIN
✅ build/us/F_NO3.BIN
✅ build/us/NO4.BIN
✅ build/us/F_NO4.BIN
✅ build/us/NP3.BIN
✅ build/us/F_NP3.BIN
✅ build/us/NZ0.BIN
✅ build/us/F_NZ0.BIN
✅ build/us/SEL.BIN
✅ build/us/ST0.BIN
✅ build/us/F_ST0.BIN
✅ build/us/TOP.BIN
✅ build/us/F_TOP.BIN
✅ build/us/WRP.BIN
✅ build/us/F_WRP.BIN
✅ build/us/RTOP.BIN
✅ build/us/F_RTOP.BIN
✅ build/us/RWRP.BIN
✅ build/us/F_RWRP.BIN
✅ build/us/BO4.BIN
✅ build/us/F_BO4.BIN
✅ build/us/MAR.BIN
✅ build/us/BO6.BIN
✅ build/us/F_BO6.BIN
✅ build/us/RBO3.BIN
✅ build/us/F_RBO3.BIN
✅ build/us/RBO5.BIN
✅ build/us/F_RBO5.BIN
✅ build/us/TT_000.BIN
✅ build/us/TT_001.BIN
✅ build/us/TT_002.BIN
✅ build/us/TT_003.BIN
✅ build/us/TT_004.BIN
✅ build/us/WEAPON0.BIN

real    0m0.744s
user    0m0.751s
sys     0m0.047s

What's not included in this PR to make this possible is fixing a race condition for assets shared between versions, but the work to resolve that is straight forward (making asset writes atomic). It wasn't necessary for this PR, but can be followed up quickly.

I haven't looked into making picci parallel with the others.

• I understand that picci is kinda broken at the moment. I am happy to get it back in a separate PR.

Removed.

[hohle]

@Xeeynamo Here's another branch built on top of this change with us, hd, and pspeu builds all being done together in a single ninja build - hohle/sotn-decomp@fe866f8...hohle:sotn-decomp:atomic-assets I haven't removed VERSION from the Makefile, but the default target builds all three in parallel.

On my localhost, master takes about 36.5s for a clean us build, 14.5s for pspeu, and around 10s for hd.

For the parallel build there's around a 10% speedup:

$> time make
⋮
[2976/2976] create dynamic splat build/pspeu/config/splat.pspeu.stnp3.yaml.dyn_syms

real    0m54.736s
user    9m39.913s
sys     0m57.574s

But even better, with header dependencies for hd and us, something like touch srt/st/e_collect.h; make takes 2s and rebuilds everything necessary for both versions. (I'm trying to get dep file generation support into mwccgap as well, but that may be stalled).

[Xeeynamo]

@Xeeynamo Here's another branch built on top of this change with us, hd, and pspeu builds all being done together in a single ninja build - hohle/[email protected]:sotn-decomp:atomic-assets I haven't removed VERSION from the Makefile, but the default target builds all three in parallel.

On my localhost, master takes about 36.5s for a clean us build, 14.5s for pspeu, and around 10s for hd.

For the parallel build there's around a 10% speedup:

$> time make
⋮
[2976/2976] create dynamic splat build/pspeu/config/splat.pspeu.stnp3.yaml.dyn_syms

real    0m54.736s
user    9m39.913s
sys     0m57.574s

But even better, with header dependencies for hd and us, something like touch srt/st/e_collect.h; make takes 2s and rebuilds everything necessary for both versions. (I'm trying to get dep file generation support into mwccgap as well, but that may be stalled).

This is great! One solution I've been thinking to deal with the asset tool is to just synchronize all the dependencies to it. The asset tool is extremely fast, so needing to run it thrice before start building the C files should be unnoticeable. Just to keep it simple.

Anyway, for the changes you proposed, I am happy to have them merged. Great stuff @hohle !