Add ability to export patch packs #97118
Conversation
mihe
force-pushed
the
patch-exports
branch
2 times, most recently
from
f1a30ac to
204ebc4
Compare
|
I've tested this a bit using a modified version of https://github.com/gdquest-demos/godot-4-new-features/ which can be found here: https://github.com/akien-mga/godot-4-new-features/tree/test-patches-2d-3d This demo is a good test case for patches/DLCs as it has a main UI launcher (base pck) used to start standalone demos which are all in their own folders. So I tweaked the project to have a first export preset to export a base pck with common stuff, and then two more export presets to export the
That seems to work fairly well! I didn't actually test stacking patches that depend on previous patches, which I believe is what the actual "Patches" tab in the export preset should be used for. If someone wants to try it out by further tweaking this demo, that should be doable I suppose? Overall the feature seems functional, but the UX isn't great IMO. Having to create one export preset per patch makes sense, though it might not be very intuitive to users. But then having to know that to make a patch you can use "Export PCK" and enable "Export as Patch" there (with its state remembered across the session, but not specifically for that preset), is a bit weird. It might be better to have a dedicated system to add "patch presets" of some form under a platform export preset, and only get patch-relevant options in a dedicated configuration dialog (e.g. no need to configure stuff that relates to the executable in a patch as we only care about the pck contents, the "export as patch" would be implicit or an option in the sectioned inspector as opposed to the file dialog, etc.). Such presets could possibly even be reused for multiple platforms, because (as far as I can imagine at least) I would expect patches to be mostly about platform-agnostic content (scenes, resources), and at most what would change is the VRAM compressed texture type to export. With such a structure, it would also become possible to implement a system to export the project + all its patches/DLCs in one go. But all this is for future work, I think it makes sense to merge the actual feature for now, even with subpar UX, and rework it in the future. To be honest, the whole export dialog UX is bad IMO and needs a redesign. The patch/DLC workflow should be taken into account when doing that work. One thing that could maybe still be done in this PR, if there's consensus, would be to move the "is this a patch" option to the export preset itself, and then have it appear in the UI as " (Patch)" like we currently have for the "(Runnable)" suffix. Mockup: I haven't tested the command line yet, but one remark: semicolon separated strings seems a bit unconventional, and might conflict with some shells where a semicolon is actually used to separate commands. Users would need to remember to quote their list for the semicolon not to be handled by the shell. I'd suggest using comma separated strings instead, like we already do for the |
|
The feature works correctly. I made a simple test project: Some problems I found:
My suggestions would be:
EDIT: |
I must admit this was not exactly how I envisioned this to be used myself, but I've also been more focused on the patch aspect, rather than the DLC aspect. I figured most projects (or at least the simpler ones) would be fine with using their regular export presets and you would simply add any new patch pack to the list of packs in the "Patches" tab as you released them. But I guess that would only really make sense for patches though, since for something like DLCs you would likely do exactly what you did with the 2D/3D demo separation, since they'd almost always be exporting different sets of scenes/resources. They would also likely not depend on eachother. But yeah, managing multiple presets might perhaps end up being the more common way of interacting with patch/DLC exports, and I agree that it's not very intuitive/ergonomic with having all the regular executable-related options in there. It's not obvious which ones (if any) you might need to keep in sync in order for the exported patch pack to be correct.
I'd be fine with this so long as we also disable the "Export All..." and "Export Project..." buttons when selecting such presets, to drive home that it only makes sense for packs. This would however mean that you can't utilize the flow I mentioned above of sharing the export preset between the base PCK and the patches, which seems mildly unfortunate.
Done.
This strikes me as more of a general problem with exports. I can't spot any precedence for export errors showing a dialog. They all seems to go through the dedicated
Done. This is bordering on bikeshedding I guess, but the "Patches" tab itself sort of served as the title for the Also, I see PCK files being referred to as "packs" in some places in Godot and "packages" in some places. Which is the correct one?
Yeah, I think the enabled/disabled state was something just taken from the screenshots in the (now ancient) original proposal. I'm not entirely sure what the motivation was for this to be added in the first place, but it seems niche enough that I have no objections to removing, so I've done just that. It can always be added back if there's a great need for it I guess.
Done.
Done. To sum up, these are the unresolved questions:
|
I'm not sure how useful is that workflow. Steam for example is able to automatically calculate the difference from last uploaded game package and make the user only download what's changed. Not sure about other stores though. Regardless, if you follow the incremental patch approach, you will eventually end up with a build that has e.g. 50+ patches, which kind of kills the purpose of PCK files, which are supposed to be singular for less fragmentation.
Depends if you want to toggle it per export or not. For DLC, the DLC preset will always be a patch. For incremental patches, you might sometimes want to export everything.
IMO the label makes sense. Tab name describes content, while the label describes the specific element (though it's the only one).
"Packs", if you say it's already used in the engine. I used "packages" in contrast to "patches", as I find it more intuitive. |
I for one am waiting for this exact feature as in my project I cannot use an external store and have to code the patching system myself. You are correct though that making 50+ patches is not really intuitive. A manual cleanup might be required from time to time. I'd use this to make one "base patch" with the main game assets and then a separate "fix patch" with only changed files which would be updated until it becomes too big. When that happens the "base patch" would be updated and the circle would continue or maybe just make the "fix patch" into a DLC kind of thing and start a new fix one. The main point being that the original binary would only need to be updated once the Godot version itself is changed. |
mihe
force-pushed
the
patch-exports
branch
2 times, most recently
from
acab811 to
2fcd89f
Compare
I don't personally have any strong feelings one way or the other, so I'll leave it as-is for now.
Fair enough.
I've gone ahead and changed the label to "Base Packs" and the button to "Add Pack". Unless we consider the suggestion of the preset toggle to be a blocking issue it would seem to me that there are no unresolved issues remaining. Let me know if there's anything I can do to help move this forward. |
editor/export/project_export.cpp
Outdated
| Control *panel = custom_feature_display->get_parent_control(); | ||
| if (panel) { | ||
| panel->add_theme_style_override(SceneStringName(panel), get_theme_stylebox("bg", "Tree")); | ||
| } |
There was a problem hiding this comment.
I'm not sure to be honest. It looks unrelated to this PR, so I went ahead and removed it. Maybe @PoqXert can shed some light on why this was in there.
|
I think the feature is fine in the current state. It works, though I didn't test with various options like encryption etc. I think "Export As Patch" should be enabled by default. Also the option could be hidden when there are no packs assigned, as it has no effect then. |
I can see the reasoning behind this, but I personally feel like the UX of this would be even more confusing than what we have currently. I'd be curious to see some more input on this. EDIT: Oh, and thank you for catching all the issues @KoBeWi, I was clearly in too much of a hurry with some of these later changes. |
And what about making it enabled by default? It's easy to imagine someone adding packs, but failing to notice the checkbox and get confused. |
I think it should be enabled by default as there is no harm for it being enabled for people who don't use the tab. |
Co-authored-by: Poq Xert <poqxert@poqxert.ru>
|
I went ahead and made "Export As Patch" enabled by default. |
|
Many thanks to @PoqXert for allowing me to supersede the original PR, despite having stuck with the original one for over two years now. Hopefully we can get some more feedback now and dial in this feature further. |
Closes godotengine/godot-proposals#146.
Supersedes #64712.
This is largely the same as #64712, but rebased to take into account the export extension API added in #90782, as well as some other minor additions, and as such @PoqXert deserves much of the credit here.
This PR adds the following to the public interface of Godot:
--export-patch [preset] [path]command-line option, which lets you export patches from command-line, using the list of patches defined in the export preset.--patches [paths]command-line option, which overrides the patches defined in the export preset.EditorExportPresetnow exposes the following methods:get_patches.EditorExportPlatformnow exposes the following methods:export_pack_patch,export_zip_patch,save_pack_patchandsave_zip_patch.EditorExportPlatformExtensionnow exposes the following virtual methods:_export_pack_patchand_export_zip_patch.