Skip to content

Conversation

@passivestar
Copy link
Contributor

Closes godotengine/godot-proposals#12701

Excludes metadata of excluded files from pck. Generates the UID cache for exported files instead of force-exporting the entire uid_cache.bin and filters the contents of global_script_class_cache.cfg

Example project: exportfilter.zip

Export config:


Occurences of "unreleased" in pck:

Master: 4 PR: 0
a b

@passivestar passivestar requested a review from a team as a code owner October 5, 2025 16:30
@KoBeWi KoBeWi added this to the 4.x milestone Oct 5, 2025
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks fine overall. Might need some more testing, but I verified that it works as expected.

I'm not sure if searching the list of all files multiple times is a good idea, but I didn't test if it visibly impacts export time.

}

Vector<String> forced_export = get_forced_export_files(p_preset);
for (int i = 0; i < forced_export.size(); i++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be changed to a range loop.

Comment on lines +1701 to +1711
// Same binary format as in ResourceUID::save_to_cache():
Ref<StreamPeerBuffer> buffer;
buffer.instantiate();
buffer->put_u32(valid_entries.size());

for (const Pair<ResourceUID::ID, String> &entry : valid_entries) {
buffer->put_u64(uint64_t(entry.first));
CharString cs = entry.second.utf8();
buffer->put_u32(cs.length());
buffer->put_data((const uint8_t *)cs.ptr(), cs.length());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if this code wasn't duplicated.

Comment on lines +1717 to +1721
Ref<ConfigFile> cf;
cf.instantiate();
if (cf->load(p_cache_path) != OK) {
return Vector<uint8_t>();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be fetched from ScriptServer, although it's more code duplication or refactoring, so I think it's fine for now.

for (const Variant &item : original_list) {
const Dictionary class_dict = item;
ERR_CONTINUE(!class_dict.has("path"));
if (p_paths.has(class_dict["path"])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that p_paths can be super big (it contains all files in the project), and now we'll have 3 methods that search it multiple times. It can be done much more efficiently, by iterating the path list and adding to filtered array if the corresponding cache has the entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to load extension list and global class cache in advance and create all three filtered cache files in a single loop over the paths?

To think about it, do we even need to put paths to those cache files into the intermediate "forced export" array? I can remove them from get_forced_export_files and check directly in the main export_project_files method, right?

Comment on lines +1735 to +1736
if (filtered_list.size() == original_list.size()) {
return FileAccess::get_file_as_bytes(p_cache_path);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if reading the file again is better than encoding it from memory. You already have the data, comparing it with the original is unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't export metadata of excluded files in game builds

2 participants