Addon docs: imported assets need a Godot-exported pack #18

Open
hiccup444 wants to merge 1 commit from hiccup444/da-game-hiccup:addons-docs-imported-assets into main
First-time contributor

Summary

docs/addons.md told addon authors to replace the player arms, or any model, texture or sound, by putting the raw file at the same res:// path inside a .zip. Step 2 of "Making an addon" said to zip that tree. That override never takes effect, in the editor or in an exported build:

  • load() of an imported asset follows the base game's .import file to the imported copy under res://.godot/imported/.
  • Neither of those comes from the zip, so the raw .glb or .png in the pack is never opened and the override silently does nothing.

Adding fails the same way. A raw model, texture or sound at a new path in a zip has no .import file, so load() cannot find it, and a scene that uses it loads with that resource missing. The README repeated the claim that the base arms are "overridable by any pack".

addon_check could not catch this. It only overrode files read with FileAccess (a LICENSE.txt and a level JSON) and never called load() on an imported resource.

Both adding and replacing work with a pack exported by Godot. The export carries the .import file and the imported data. The imported file name is derived from the source path, so a replacement shadows the base one.

Changes

  • docs/addons.md
    • "How loading works" now says imported assets are the exception: a raw .glb, .png or .wav in a zip is ignored at a base path and cannot be loaded at a new one.
    • It also covers exported games:
      • With Godot's default export settings, an exported game loads base scenes, resources and scripts through .remap files, so raw replacements of those are ignored there too.
      • When the game runs from the project the raw file does win, which hides the problem until export.
      • Scenes, resources and scripts at new paths load normally.
    • "What you can do with that":
      • The arms row now points to an exported pack.
      • "Add a level" says a zip is enough unless the level brings its own models, textures or sounds, and sends those to the exported pack.
    • "Making an addon" now has two routes:
      • a plain zip, for data and for new scenes and scripts. It says a zip cannot carry models, textures or sounds, not even at new paths;
      • a pack exported by Godot, for adding or replacing imported assets, or for replacing base scenes and scripts in an exported game. The arms stay as the example here.
    • The export steps say to work in a copy of the game project:
      • Every export also carries project-wide files (settings, the script class list, the UID cache) that shadow the game's own.
      • After mounting an addon exported from an unrelated empty project, ProjectSettings.get_global_class_list() went from 125 entries to 0.
      • Registered classes kept working, but working in a copy keeps those files matching.
  • src/core/addon_loader.gd: the header comment, which says a file in a pack replaces the base file at the same path, now adds that load() reaches imported assets (and, in an exported game, scenes and scripts) through their .import or .remap file, so those need a pack exported by Godot.
  • README.md: the base arms are now "replaceable by a pack exported from Godot (a raw .glb in a zip is ignored)" instead of "overridable by any pack". The other README drift is tracked separately.
  • tests/addon_check.gd: a new case that goes through load(). It uses a small imported texture already in the repo (addons/godot-xr-tools/images/teleport_arrow.png) and a texture at a path the base game does not have. See Tests.

This PR only makes the docs honest. The real fix for zip-tier assets is the AssetLoader proposed on #3: key resolution plus runtime loaders (GLTFDocument for glTF, Image to ImageTexture for images, WAV from bytes). Until it lands, zip packs can only add or override data the game reads as files, like level JSON, plus new scenes and scripts. When it lands, ArmsView should resolve its model through it instead of calling load(MODEL_PATH) directly. Otherwise the arms stay unmoddable by zip even after #3.

Tests

New checks in addon_check:

  • base texture loads
  • raw asset zip opens, raw asset zip mounts
  • raw texture readable as a file: FileAccess returns the zip's PNG at the base path, so the pack really is mounted.
  • raw texture in a zip does not change load(): load() still returns the base texture.
  • raw texture at a new path is not loadable: the same zip carries the PNG at a new path. FileAccess sees it, ResourceLoader.exists() does not.
  • override texture saves, override .import writes, pck opens, pck writes, imported asset pck mounts
  • pck with .import and data replaces load(): the test builds a pck at runtime with PCKPacker. It holds an .import file at the texture's path, remapped to an ImageTexture saved with ResourceSaver. Mounted through AddonLoader.mount_pack(), it makes load() return the override.
  • pck with .import and data adds a texture at a new path: the same pck also carries that .import file at the new path, and load() then returns the texture there.
  • throwaway files removed: every pack and temp file is deleted and the test folder is removed. The check fails if any of those removals fails. Nothing is written to user://addons.

The new loads use CACHE_MODE_IGNORE, so a copy cached before mounting cannot hide an override. Mutation runs behaved as expected:

  • Leaving either .import file out of the pck fails the matching positive check.
  • Leaving a temp file behind fails the cleanup check.

Also checked by hand on 4.6.3. These are not part of the suite, since they need --export-pack and a second project:

  • Replacing, with an addon exported from a separate project.
    • The project held a different teleport_arrow.png and a different player_arms.glb.
    • Its --export-pack output carried teleport_arrow.png-f1bd44b6f478277692b3fa29171b62d3.ctex and player_arms.glb-cdbc2179235e3e4bc80946aa62fb407a.scn, the same names as the base.
    • Mounted in the game, it replaced both. The same export as a .zip worked the same way, and no export templates were needed.
  • Adding, with an addon exported from a separate project.
    • The project held a new texture, a scene using it and a level descriptor, all at new paths.
    • Mounted in the game, load() returned the texture, the scene came with it, and the level registry found the descriptor.
    • As a raw zip, load() reported "Resource file not found" for a new .png, .glb and .wav, and a new scene using the .png loaded with its texture missing.
  • Game exported as a pack and run with --main-pack.
    • A raw .png and .glb in a zip were readable as files, but load() kept returning the base texture and arms. The exported addon pack replaced both.
    • Raw .tscn and .gd replacements at base paths were ignored too, because the base .remap files point to the converted copies.
    • New .tscn, .tres and .gd files at new paths loaded, and a shadowed level descriptor JSON was read.

Full suites (smoke_menu, arms_check, addon_check, save_check, rig_motion_check, pause_check, input_check): all PASS. The local run was on Windows with --headless --xr-mode off. Every suite's error lines match main (the known harness noise only). CI covers Linux.

Corrections to the review

  • The container is not what matters. A .zip exported by Godot works exactly like a .pck. A hand-made zip that carries the .import file and the data it points to also works. What a raw zip lacks is the import data, not the .pck format, so the docs say "a pack exported by Godot" rather than "a .pck".
  • Adding is affected too, not only replacing. The review limited the problem to replacing an imported asset. A raw model, texture or sound at a new path in a zip cannot be loaded either, so new imported assets also need an exported pack.
  • The problem is wider than imported assets. With default export settings, an exported game also converts base scenes, resources and scripts and reaches them through .remap files. Shadowing those with raw files in a zip only works when running from the project. The docs now say so.

Overlap with open PRs

Sibling PRs from the same review: #15, #16, #17 and #19. Their source changes merge cleanly with each other. Several of them add checks at the same spot in tests/pause_check.gd (and two in tests/save_check.gd), so whichever merges second may need a small rebase that keeps both blocks.

## Summary `docs/addons.md` told addon authors to replace the player arms, or any model, texture or sound, by putting the raw file at the same `res://` path inside a `.zip`. Step 2 of "Making an addon" said to zip that tree. That override never takes effect, in the editor or in an exported build: - `load()` of an imported asset follows the base game's `.import` file to the imported copy under `res://.godot/imported/`. - Neither of those comes from the zip, so the raw `.glb` or `.png` in the pack is never opened and the override silently does nothing. Adding fails the same way. A raw model, texture or sound at a new path in a zip has no `.import` file, so `load()` cannot find it, and a scene that uses it loads with that resource missing. The README repeated the claim that the base arms are "overridable by any pack". `addon_check` could not catch this. It only overrode files read with `FileAccess` (a `LICENSE.txt` and a level JSON) and never called `load()` on an imported resource. Both adding and replacing work with a pack exported by Godot. The export carries the `.import` file and the imported data. The imported file name is derived from the source path, so a replacement shadows the base one. ## Changes - `docs/addons.md` - "How loading works" now says imported assets are the exception: a raw `.glb`, `.png` or `.wav` in a zip is ignored at a base path and cannot be loaded at a new one. - It also covers exported games: - With Godot's default export settings, an exported game loads base scenes, resources and scripts through `.remap` files, so raw replacements of those are ignored there too. - When the game runs from the project the raw file does win, which hides the problem until export. - Scenes, resources and scripts at new paths load normally. - "What you can do with that": - The arms row now points to an exported pack. - "Add a level" says a zip is enough unless the level brings its own models, textures or sounds, and sends those to the exported pack. - "Making an addon" now has two routes: - a plain zip, for data and for new scenes and scripts. It says a zip cannot carry models, textures or sounds, not even at new paths; - a pack exported by Godot, for adding or replacing imported assets, or for replacing base scenes and scripts in an exported game. The arms stay as the example here. - The export steps say to work in a copy of the game project: - Every export also carries project-wide files (settings, the script class list, the UID cache) that shadow the game's own. - After mounting an addon exported from an unrelated empty project, `ProjectSettings.get_global_class_list()` went from 125 entries to 0. - Registered classes kept working, but working in a copy keeps those files matching. - `src/core/addon_loader.gd`: the header comment, which says a file in a pack replaces the base file at the same path, now adds that `load()` reaches imported assets (and, in an exported game, scenes and scripts) through their `.import` or `.remap` file, so those need a pack exported by Godot. - `README.md`: the base arms are now "replaceable by a pack exported from Godot (a raw `.glb` in a zip is ignored)" instead of "overridable by any pack". The other README drift is tracked separately. - `tests/addon_check.gd`: a new case that goes through `load()`. It uses a small imported texture already in the repo (`addons/godot-xr-tools/images/teleport_arrow.png`) and a texture at a path the base game does not have. See Tests. This PR only makes the docs honest. The real fix for zip-tier assets is the AssetLoader proposed on #3: key resolution plus runtime loaders (`GLTFDocument` for glTF, `Image` to `ImageTexture` for images, WAV from bytes). Until it lands, zip packs can only add or override data the game reads as files, like level JSON, plus new scenes and scripts. When it lands, `ArmsView` should resolve its model through it instead of calling `load(MODEL_PATH)` directly. Otherwise the arms stay unmoddable by zip even after #3. ## Tests New checks in `addon_check`: - `base texture loads` - `raw asset zip opens`, `raw asset zip mounts` - `raw texture readable as a file`: `FileAccess` returns the zip's PNG at the base path, so the pack really is mounted. - `raw texture in a zip does not change load()`: `load()` still returns the base texture. - `raw texture at a new path is not loadable`: the same zip carries the PNG at a new path. `FileAccess` sees it, `ResourceLoader.exists()` does not. - `override texture saves`, `override .import writes`, `pck opens`, `pck writes`, `imported asset pck mounts` - `pck with .import and data replaces load()`: the test builds a pck at runtime with `PCKPacker`. It holds an `.import` file at the texture's path, remapped to an `ImageTexture` saved with `ResourceSaver`. Mounted through `AddonLoader.mount_pack()`, it makes `load()` return the override. - `pck with .import and data adds a texture at a new path`: the same pck also carries that `.import` file at the new path, and `load()` then returns the texture there. - `throwaway files removed`: every pack and temp file is deleted and the test folder is removed. The check fails if any of those removals fails. Nothing is written to `user://addons`. The new loads use `CACHE_MODE_IGNORE`, so a copy cached before mounting cannot hide an override. Mutation runs behaved as expected: - Leaving either `.import` file out of the pck fails the matching positive check. - Leaving a temp file behind fails the cleanup check. Also checked by hand on 4.6.3. These are not part of the suite, since they need `--export-pack` and a second project: - **Replacing, with an addon exported from a separate project.** - The project held a different `teleport_arrow.png` and a different `player_arms.glb`. - Its `--export-pack` output carried `teleport_arrow.png-f1bd44b6f478277692b3fa29171b62d3.ctex` and `player_arms.glb-cdbc2179235e3e4bc80946aa62fb407a.scn`, the same names as the base. - Mounted in the game, it replaced both. The same export as a `.zip` worked the same way, and no export templates were needed. - **Adding, with an addon exported from a separate project.** - The project held a new texture, a scene using it and a level descriptor, all at new paths. - Mounted in the game, `load()` returned the texture, the scene came with it, and the level registry found the descriptor. - As a raw zip, `load()` reported "Resource file not found" for a new `.png`, `.glb` and `.wav`, and a new scene using the `.png` loaded with its texture missing. - **Game exported as a pack and run with `--main-pack`.** - A raw `.png` and `.glb` in a zip were readable as files, but `load()` kept returning the base texture and arms. The exported addon pack replaced both. - Raw `.tscn` and `.gd` replacements at base paths were ignored too, because the base `.remap` files point to the converted copies. - New `.tscn`, `.tres` and `.gd` files at new paths loaded, and a shadowed level descriptor JSON was read. Full suites (smoke_menu, arms_check, addon_check, save_check, rig_motion_check, pause_check, input_check): all PASS. The local run was on Windows with `--headless --xr-mode off`. Every suite's error lines match `main` (the known harness noise only). CI covers Linux. ## Corrections to the review - **The container is not what matters.** A `.zip` exported by Godot works exactly like a `.pck`. A hand-made zip that carries the `.import` file and the data it points to also works. What a raw zip lacks is the import data, not the `.pck` format, so the docs say "a pack exported by Godot" rather than "a `.pck`". - **Adding is affected too, not only replacing.** The review limited the problem to replacing an imported asset. A raw model, texture or sound at a new path in a zip cannot be loaded either, so new imported assets also need an exported pack. - **The problem is wider than imported assets.** With default export settings, an exported game also converts base scenes, resources and scripts and reaches them through `.remap` files. Shadowing those with raw files in a zip only works when running from the project. The docs now say so. ## Overlap with open PRs Sibling PRs from the same review: #15, #16, #17 and #19. Their source changes merge cleanly with each other. Several of them add checks at the same spot in `tests/pause_check.gd` (and two in `tests/save_check.gd`), so whichever merges second may need a small rebase that keeps both blocks.
addons: say imported assets need a Godot-exported pack
Some checks failed
tests / test (pull_request) Failing after 57s
c9eab62cbb
docs/addons.md told addon authors to replace the arms, or any model,
texture or sound, by putting the raw file at the same res:// path in
a zip. load() never opens that file: it follows the base game's
.import file to the imported copy, so the override silently does
nothing, and a raw model, texture or sound at a new path cannot be
loaded at all. Base scenes, resources and scripts are ignored the
same way in an exported game, where they load through .remap files.

The docs now keep plain zips for data and for new scenes and scripts,
and send new or replaced imported assets, and replaced converted
files, through a pack exported by Godot. The README no longer says
any pack can replace the arms, and
the AddonLoader header comment points at the same rule.

addon_check only covered files read with FileAccess. It now mounts
raw textures in a zip and checks that load() is unchanged at a base
path and finds nothing at a new one, then mounts a pck carrying
.import files and the data they point to and checks that load()
returns the override at both paths.
Some checks failed
tests / test (pull_request) Failing after 57s
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u addons-docs-imported-assets:hiccup444-addons-docs-imported-assets
git switch hiccup444-addons-docs-imported-assets

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff hiccup444-addons-docs-imported-assets
git switch hiccup444-addons-docs-imported-assets
git rebase main
git switch main
git merge --ff-only hiccup444-addons-docs-imported-assets
git switch hiccup444-addons-docs-imported-assets
git rebase main
git switch main
git merge --no-ff hiccup444-addons-docs-imported-assets
git switch main
git merge --squash hiccup444-addons-docs-imported-assets
git switch main
git merge --ff-only hiccup444-addons-docs-imported-assets
git switch main
git merge hiccup444-addons-docs-imported-assets
git push origin main
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
dead-air/engine!18
No description provided.