Implement a patch system.

[PoqXert]

This PR adds Export As Patch setting for exporting PCK/ZIP.

When enabled, godot compare exported files and files in packs specified in the Patches tab. And exports only new/changed files.

Adds --export-patch option.

Proposal: godotengine/godot-proposals#146

[fire]

Here was my notes when I looked at the updating the game problem. https://docs.google.com/document/d/1wU-q23wSeXnnXkJKdiFAScvChCA82dTcZe5y46_KxKs/edit?usp=sharing Hope it is useful for future work.

[PoqXert]

Note: If encrypted PCKs (exported with "Encrypt Index" option) are used in the "Patches" tab, then a custom build of the editor is required. Otherwise, Godot will not be able to load these PCKs to check the changes.

[unfa]

One question: is there a way to use this functionality from code? I'd love to be able to export differential PCKs from a custom editor interface. Anyway, this is what I was looking for!

[PoqXert]

One question: is there a way to use this functionality from code?

It can't be used from GDScript, if that's what you're mean.

[SanteriLoitomaa]

Hello! What's the status on this pr? I'm curious about when it would be merged because it would be nice to have for my project.

[Calinou]

Hello! What's the status on this pr? I'm curious about when it would be merged because it would be nice to have for my project.

4.2 is currently in feature freeze, and this PR needs to be rebased before it can be merged. It also needs a review and testing from users who need this feature, to make sure it works as expected.

[PoqXert]

2. There seems to be something odd going on when you provide previously exported *.zip files as prior patches, where GDScript files will end up being exported despite not having changed. I haven't tried C# scripts.

ZIP-packs do not store file hashes, so all files included in the archive are considered modified.

godot/core/io/file_access_zip.cpp

Lines 196 to 198 in 658e97c

	uint8_t md5[16] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
	PackedData::get_singleton()->add_path(p_path, fname, 1, 0, md5, this, p_replace_files, false);

[mihe]

ZIP-packs do not store file hashes, so all files included in the archive are considered modified.

Ah. That seems like a fairly big gotcha. I suppose it could be made clear in the documentation somewhere, but wouldn't it make more sense then to just disallow ZIP packs from being listed among the patches altogether, or at least until hashes can be added to them in some way (if it all possible)?

[akien-mga]

For the record, your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

[PoqXert]

Ah. That seems like a fairly big gotcha. I suppose it could be made clear in the documentation somewhere, but wouldn't it make more sense then to just disallow ZIP packs from being listed among the patches altogether, or at least until hashes can be added to them in some way (if it all possible)?

Just remove the *.zip filter. But there will be no warning if the user ignores the filter and selects the zip file.

4. I think encoding the enabled value of each patch as an asterisk into the path itself is somewhat questionable. It strikes me as something that might not age very well, and could trip up future contributors. In fact, there's already a bug related to this, where if you delete a patch from the list of patches the asterisk is not removed before being passed to the confirmation dialog. I believe asterisks are also technically valid path characters on Unix-like systems, so doing a blanket replace("*", "") might cause trouble. I'm not sure what would be the best alternative approach, but just having a separate enabled_patches entry might suffice.

I have an implementation with separate statuses. This seems to require discussion before adding these changes to this PR.

[mihe]

Just remove the *.zip filter. But there will be no warning if the user ignores the filter and selects the zip file.

Yeah, I personally think this is fine, although there should ideally be some accompanying documentation that explains why this is the case as well.

It is now however somewhat strange that you're allowed to export a ZIP pack as a patch, but not actually add it to the list of patches. There may perhaps be some argument for disallowing ZIP packs to even be exported as patches, although I'm not sure through what mechanism that would be done.

I have an implementation with separate statuses. This seems to require discussion before adding these changes to this PR.

I think this is a lot better than the asterisks. I would still perhaps opt for just a complementary array of paths rather than this ints-as-booleans status array though. In fact, seeing as how the vast majority of patches that will ever be added will stay enabled, flipping it around and storing it as something like a disabled_patches string array would mean that it would stay as an empty array for the most part, as well as (in my opinion) result in a more readable/editable export_presets.cfg.

Here's what that would look like: mihe/godot@patcher-with-disabled

The only gotcha with this is of course that a patch can't be listed multiple times in the list of patches, but that strikes me as potentially problematic either way.

EDIT: In case it's not clear from the diff alone, here's what the separate statuses would store as (with one patch disabled):

patches=PackedStringArray("my_game.pck", "my_first_patch.pck", "my_second_patch.pck")
patch_statuses=PackedInt32Array(1, 0, 1)

... versus my suggestion:

patches=PackedStringArray("my_game.pck", "my_first_patch.pck", "my_second_patch.pck")
disabled_patches=PackedStringArray("my_first_patch.pck")

[SanteriLoitomaa]

Does the hash of GDScript files change when an export is made? When I export a main .pck, set it as a patch, and then export a patch as a .zip file and look inside there's a .gdc file inside even though I made no changes to the code.

Specifically I'm using the Compatibility renderer and the script in the .zip seems to be the script used by my main scene. When I switched to Forward+ there was another script in the .zip that I made to test the only main scene theory.

[PoqXert]

Does the hash of GDScript files change when an export is made?

Yes when GDScript Export Mode set not Text. Sometimes the hashes are the same, so unchanged scripts may or may not be added to the patch.

[SanteriLoitomaa]

Does the hash of GDScript files change when an export is made?

Yes when GDScript Export Mode set not Text.

Ah I see, that makes sense. Hopefully this gets merged soon as they are starting to tease the feature freeze and I don't really want to wait for another 6 months.

[mihe]

as they are starting to tease the feature freeze

The feature freeze is already in effect.

[SanteriLoitomaa]

as they are starting to tease the feature freeze

The feature freeze is already in effect.

Oh, so it is. Guess I missed that. Is there any chance they'll still merge this anyway as it's been on the list for quite some time now?

[akien-mga]

No, but this is on the roadmap for 4.4, and should be merged early in that dev cycle to get ample testing.

We're already aware of potential follow-up changes that would be needed to really have a solid system for patches and DLCs, covering all the different use cases (adding, removing, replacing files, etc.).

In the meantime of course, this PR is up-to-date with the master branch, and will likely be reasonably up-to-date with 4.3.stable once it's released. So any user who depends on this for their game could likely backport it to their own fork and start using it in production (and report any issue they find so we make sure the 4.4 solution covers all needs).

[mihe]

In the interest of hopefully getting this PR merged shortly, I've gone ahead and rebased it against latest master (which is 4788f54 as of writing this), since there seemed to be some conflicts. You can find my rebased branch here: master...mihe:godot:patcher-4.4

There have apparently been some changes to EditorExportPlatform and surrounding files recently, which complicated the rebase a little bit. The changes in question (#90782) adds support for adding custom export platforms through scripts/extensions. I've tried marrying the two as best I could, which includes adding the new patch stuff to this new script/extension interface.

I also threw in some other minor changes that I personally feel should be made, especially with the new patch methods now being exposed as part of the stable API. In total the changes (compared to PoqXert/godot@5b17ac4) are as follows:

1. I renamed _save_patch_file and export_patch to _save_pack_patch_file and export_pack_patch respectively, to lessen the ambiguity of what type of patch archive they concern.
2. Since there's now already _save_pack and _save_zip methods in EditorExportPlatform, which are meant to be what gets bound to the script bindings, I moved the save function parameter to save_pack and save_zip instead. They're now optional as well, and will default to EditorExportPlatform::_save_pack_file and EditorExportPlatform::_save_zip_file respectively.
3. The call to save_pack in EditorExportPlatformPC had to be changed as a result of moving the save function parameter to save_zip.
4. I added _save_pack_patch, _save_zip_patch, save_pack_patch, save_zip_patch, export_pack_patch and export_zip_patch, for better parity with the non-patch versions. These are similarly exposed in the script/extension bindings.

I also stumbled upon a bug, where GDScript files exported as binary tokens (compressed or otherwise) would often be exported as part of any patch PCK despite the files not having changed. This turned out to be because of random memory being written as part of the .gdc files, which I reported in #96854 and fixed in #96855.

Anyway, feel free to grab the rebased branch as a whole or cherrypick the pieces you feel are appropriate. So long as this gets rebased and merged in some fashion we can work out the kinks afterwards I guess.

Given that the issue of the enabled/disabled statuses seems to have been resolved I don't really have any more feedback to give here, and would be happy to give this PR my approval once it's been rebased (whatever that's worth).

[mihe]

@PoqXert Any chance you'll be able to do the rebase within the next few days?

[PoqXert]

@PoqXert Any chance you'll be able to do the rebase within the next few days?

I can't do this until next weekend. If you want to continue working on this feature earlier, you can open a new PR and close this one.

[mihe]

If you want to continue working on this feature earlier, you can open a new PR and close this one.

Great, thank you! I'll go ahead and do exactly that. Expect to see a superseding PR shortly.

I'll be sure to add the appropriate co-author attribution and all that stuff of course.

[mihe]

See #97118.

[Calinou]

• Superseded by Add ability to export patch packs #97118.

Thanks a lot for the contribution nonetheless!