Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDScript: Reintroduce binary tokenization on export #87634

Merged
merged 2 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions editor/export/editor_export.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ void EditorExport::_save() {
config->set_value(section, "encryption_exclude_filters", preset->get_enc_ex_filter());
config->set_value(section, "encrypt_pck", preset->get_enc_pck());
config->set_value(section, "encrypt_directory", preset->get_enc_directory());
config->set_value(section, "script_export_mode", preset->get_script_export_mode());
credentials->set_value(section, "script_encryption_key", preset->get_script_encryption_key());

String option_section = "preset." + itos(i) + ".options";
Expand Down Expand Up @@ -269,6 +270,7 @@ void EditorExport::load_config() {
preset->set_include_filter(config->get_value(section, "include_filter"));
preset->set_exclude_filter(config->get_value(section, "exclude_filter"));
preset->set_export_path(config->get_value(section, "export_path", ""));
preset->set_script_export_mode(config->get_value(section, "script_export_mode", EditorExportPreset::MODE_SCRIPT_BINARY_TOKENS_COMPRESSED));

if (config->has_section_key(section, "encrypt_pck")) {
preset->set_enc_pck(config->get_value(section, "encrypt_pck"));
Expand Down
9 changes: 9 additions & 0 deletions editor/export/editor_export_preset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,15 @@ String EditorExportPreset::get_script_encryption_key() const {
return script_key;
}

void EditorExportPreset::set_script_export_mode(int p_mode) {
script_mode = p_mode;
EditorExport::singleton->save_presets();
}

int EditorExportPreset::get_script_export_mode() const {
return script_mode;
}

Variant EditorExportPreset::get_or_env(const StringName &p_name, const String &p_env_var, bool *r_valid) const {
const String from_env = OS::get_singleton()->get_environment(p_env_var);
if (!from_env.is_empty()) {
Expand Down
10 changes: 10 additions & 0 deletions editor/export/editor_export_preset.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ class EditorExportPreset : public RefCounted {
MODE_FILE_REMOVE,
};

enum ScriptExportMode {
MODE_SCRIPT_TEXT,
MODE_SCRIPT_BINARY_TOKENS,
MODE_SCRIPT_BINARY_TOKENS_COMPRESSED,
};

private:
Ref<EditorExportPlatform> platform;
ExportFilter export_filter = EXPORT_ALL_RESOURCES;
Expand Down Expand Up @@ -84,6 +90,7 @@ class EditorExportPreset : public RefCounted {
bool enc_directory = false;

String script_key;
int script_mode = MODE_SCRIPT_BINARY_TOKENS_COMPRESSED;

protected:
bool _set(const StringName &p_name, const Variant &p_value);
Expand Down Expand Up @@ -152,6 +159,9 @@ class EditorExportPreset : public RefCounted {
void set_script_encryption_key(const String &p_key);
String get_script_encryption_key() const;

void set_script_export_mode(int p_mode);
int get_script_export_mode() const;

Variant get_or_env(const StringName &p_name, const String &p_env_var, bool *r_valid = nullptr) const;

// Return the preset's version number, or fall back to the
Expand Down
32 changes: 31 additions & 1 deletion editor/export/project_export.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,9 @@ void ProjectExportDialog::_edit_preset(int p_index) {
script_key_error->hide();
}

int script_export_mode = current->get_script_export_mode();
script_mode->select(script_export_mode);

updating = false;
}

Expand Down Expand Up @@ -582,6 +585,19 @@ bool ProjectExportDialog::_validate_script_encryption_key(const String &p_key) {
return is_valid;
}

void ProjectExportDialog::_script_export_mode_changed(int p_mode) {
if (updating) {
return;
}

Ref<EditorExportPreset> current = get_current_preset();
ERR_FAIL_COND(current.is_null());

current->set_script_export_mode(p_mode);

_update_current_preset();
}

void ProjectExportDialog::_duplicate_preset() {
Ref<EditorExportPreset> current = get_current_preset();
if (current.is_null()) {
Expand Down Expand Up @@ -1328,7 +1344,7 @@ ProjectExportDialog::ProjectExportDialog() {
feature_vb->add_margin_child(TTR("Feature List:"), custom_feature_display, true);
sections->add_child(feature_vb);

// Script export parameters.
// Encryption export parameters.

VBoxContainer *sec_vb = memnew(VBoxContainer);
sec_vb->set_name(TTR("Encryption"));
Expand Down Expand Up @@ -1373,6 +1389,20 @@ ProjectExportDialog::ProjectExportDialog() {
sec_more_info->connect("pressed", callable_mp(this, &ProjectExportDialog::_open_key_help_link));
sec_vb->add_child(sec_more_info);

// Script export parameters.

VBoxContainer *script_vb = memnew(VBoxContainer);
script_vb->set_name(TTR("Scripts"));

script_mode = memnew(OptionButton);
script_vb->add_margin_child(TTR("GDScript Export Mode:"), script_mode);
script_mode->add_item(TTR("Text (easier debugging)"), (int)EditorExportPreset::MODE_SCRIPT_TEXT);
script_mode->add_item(TTR("Binary tokens (faster loading)"), (int)EditorExportPreset::MODE_SCRIPT_BINARY_TOKENS);
script_mode->add_item(TTR("Compressed binary tokens (smaller files)"), (int)EditorExportPreset::MODE_SCRIPT_BINARY_TOKENS_COMPRESSED);
script_mode->connect("item_selected", callable_mp(this, &ProjectExportDialog::_script_export_mode_changed));

sections->add_child(script_vb);
Comment on lines +1392 to +1404
Copy link
Member

@akien-mga akien-mga Feb 8, 2024

Choose a reason for hiding this comment

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

Technically this is GDScript specific code spilling into the main editor, which wouldn't be relevant if the GDScript module is disabled. So it's slightly breaking encapsulation.

But it's not a first, and I'm not sure it's worth adding a complex abstraction for this so it can be handled in the module for each script language individually. Your call if you think it's worth looking into.

For now I think this doesn't need to prevent merging.

Copy link
Member Author

Choose a reason for hiding this comment

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

I though about this but I'm not sure of what would be the best solution. This is the same way it is handled in 3.x.


sections->connect("tab_changed", callable_mp(this, &ProjectExportDialog::_tab_changed));

// Disable by default.
Expand Down
4 changes: 4 additions & 0 deletions editor/export/project_export.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ class ProjectExportDialog : public ConfirmationDialog {
LineEdit *enc_in_filters = nullptr;
LineEdit *enc_ex_filters = nullptr;

OptionButton *script_mode = nullptr;

void _open_export_template_manager();

void _export_pck_zip();
Expand All @@ -183,6 +185,8 @@ class ProjectExportDialog : public ConfirmationDialog {
void _script_encryption_key_changed(const String &p_key);
bool _validate_script_encryption_key(const String &p_key);

void _script_export_mode_changed(int p_mode);

void _open_key_help_link();

void _tab_changed(int);
Expand Down
24 changes: 22 additions & 2 deletions modules/gdscript/gdscript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "gdscript_compiler.h"
#include "gdscript_parser.h"
#include "gdscript_rpc_callable.h"
#include "gdscript_tokenizer_buffer.h"
#include "gdscript_warning.h"

#ifdef TOOLS_ENABLED
Expand Down Expand Up @@ -740,7 +741,12 @@ Error GDScript::reload(bool p_keep_state) {

valid = false;
GDScriptParser parser;
Error err = parser.parse(source, path, false);
Error err;
if (!binary_tokens.is_empty()) {
err = parser.parse_binary(binary_tokens, path);
} else {
err = parser.parse(source, path, false);
}
if (err) {
if (EngineDebugger::is_active()) {
GDScriptLanguage::get_singleton()->debug_break_parse(_get_debug_path(), parser.get_errors().front()->get().line, "Parser Error: " + parser.get_errors().front()->get().message);
Expand Down Expand Up @@ -1050,6 +1056,19 @@ Error GDScript::load_source_code(const String &p_path) {
return OK;
}

void GDScript::set_binary_tokens_source(const Vector<uint8_t> &p_binary_tokens) {
binary_tokens = p_binary_tokens;
}

const Vector<uint8_t> &GDScript::get_binary_tokens_source() const {
return binary_tokens;
}

Vector<uint8_t> GDScript::get_as_binary_tokens() const {
GDScriptTokenizerBuffer tokenizer;
return tokenizer.parse_code_string(source, GDScriptTokenizerBuffer::COMPRESS_NONE);
}

const HashMap<StringName, GDScriptFunction *> &GDScript::debug_get_member_functions() const {
return member_functions;
}
Expand Down Expand Up @@ -2805,6 +2824,7 @@ Ref<Resource> ResourceFormatLoaderGDScript::load(const String &p_path, const Str

void ResourceFormatLoaderGDScript::get_recognized_extensions(List<String> *p_extensions) const {
p_extensions->push_back("gd");
p_extensions->push_back("gdc");
}

bool ResourceFormatLoaderGDScript::handles_type(const String &p_type) const {
Expand All @@ -2813,7 +2833,7 @@ bool ResourceFormatLoaderGDScript::handles_type(const String &p_type) const {

String ResourceFormatLoaderGDScript::get_resource_type(const String &p_path) const {
String el = p_path.get_extension().to_lower();
if (el == "gd") {
if (el == "gd" || el == "gdc") {
return "GDScript";
}
return "";
Expand Down
5 changes: 5 additions & 0 deletions modules/gdscript/gdscript.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ class GDScript : public Script {
bool clearing = false;
//exported members
String source;
Vector<uint8_t> binary_tokens;
String path;
bool path_valid = false; // False if using default path.
StringName local_name; // Inner class identifier or `class_name`.
Expand Down Expand Up @@ -296,6 +297,10 @@ class GDScript : public Script {
String get_script_path() const;
Error load_source_code(const String &p_path);

void set_binary_tokens_source(const Vector<uint8_t> &p_binary_tokens);
const Vector<uint8_t> &get_binary_tokens_source() const;
Vector<uint8_t> get_as_binary_tokens() const;

bool get_property_default_value(const StringName &p_property, Variant &r_value) const override;

virtual void get_script_method_list(List<MethodInfo> *p_list) const override;
Expand Down
55 changes: 47 additions & 8 deletions modules/gdscript/gdscript_cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@ Error GDScriptParserRef::raise_status(Status p_new_status) {

while (p_new_status > status) {
switch (status) {
case EMPTY:
case EMPTY: {
status = PARSED;
result = parser->parse(GDScriptCache::get_source_code(path), path, false);
break;
String remapped_path = ResourceLoader::path_remap(path);
if (remapped_path.get_extension().to_lower() == "gdc") {
result = parser->parse_binary(GDScriptCache::get_binary_tokens(remapped_path), path);
} else {
result = parser->parse(GDScriptCache::get_source_code(remapped_path), path, false);
}
} break;
case PARSED: {
status = INHERITANCE_SOLVED;
Error inheritance_result = get_analyzer()->resolve_inheritance();
Expand Down Expand Up @@ -205,7 +210,8 @@ Ref<GDScriptParserRef> GDScriptCache::get_parser(const String &p_path, GDScriptP
return ref;
}
} else {
if (!FileAccess::exists(p_path)) {
String remapped_path = ResourceLoader::path_remap(p_path);
if (!FileAccess::exists(remapped_path)) {
r_error = ERR_FILE_NOT_FOUND;
return ref;
}
Expand Down Expand Up @@ -239,6 +245,20 @@ String GDScriptCache::get_source_code(const String &p_path) {
return source;
}

Vector<uint8_t> GDScriptCache::get_binary_tokens(const String &p_path) {
Vector<uint8_t> buffer;
Error err = OK;
Ref<FileAccess> f = FileAccess::open(p_path, FileAccess::READ, &err);
ERR_FAIL_COND_V_MSG(err != OK, buffer, "Failed to open binary GDScript file '" + p_path + "'.");

uint64_t len = f->get_length();
buffer.resize(len);
uint64_t read = f->get_buffer(buffer.ptrw(), buffer.size());
ERR_FAIL_COND_V_MSG(read != len, Vector<uint8_t>(), "Failed to read binary GDScript file '" + p_path + "'.");

return buffer;
}

Ref<GDScript> GDScriptCache::get_shallow_script(const String &p_path, Error &r_error, const String &p_owner) {
MutexLock lock(singleton->mutex);
if (!p_owner.is_empty()) {
Expand All @@ -251,10 +271,20 @@ Ref<GDScript> GDScriptCache::get_shallow_script(const String &p_path, Error &r_e
return singleton->shallow_gdscript_cache[p_path];
}

String remapped_path = ResourceLoader::path_remap(p_path);

Ref<GDScript> script;
script.instantiate();
script->set_path(p_path, true);
r_error = script->load_source_code(p_path);
if (remapped_path.get_extension().to_lower() == "gdc") {
Vector<uint8_t> buffer = get_binary_tokens(remapped_path);
if (buffer.is_empty()) {
r_error = ERR_FILE_CANT_READ;
}
script->set_binary_tokens_source(buffer);
} else {
r_error = script->load_source_code(remapped_path);
}

if (r_error) {
return Ref<GDScript>(); // Returns null and does not cache when the script fails to load.
Expand Down Expand Up @@ -294,9 +324,18 @@ Ref<GDScript> GDScriptCache::get_full_script(const String &p_path, Error &r_erro
}

if (p_update_from_disk) {
r_error = script->load_source_code(p_path);
if (r_error) {
return script;
if (p_path.get_extension().to_lower() == "gdc") {
Vector<uint8_t> buffer = get_binary_tokens(p_path);
if (buffer.is_empty()) {
r_error = ERR_FILE_CANT_READ;
return script;
}
script->set_binary_tokens_source(buffer);
} else {
r_error = script->load_source_code(p_path);
if (r_error) {
return script;
}
}
}

Expand Down
1 change: 1 addition & 0 deletions modules/gdscript/gdscript_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class GDScriptCache {
static void remove_script(const String &p_path);
static Ref<GDScriptParserRef> get_parser(const String &p_path, GDScriptParserRef::Status status, Error &r_error, const String &p_owner = String());
static String get_source_code(const String &p_path);
static Vector<uint8_t> get_binary_tokens(const String &p_path);
static Ref<GDScript> get_shallow_script(const String &p_path, Error &r_error, const String &p_owner = String());
static Ref<GDScript> get_full_script(const String &p_path, Error &r_error, const String &p_owner = String(), bool p_update_from_disk = false);
static Ref<GDScript> get_cached_script(const String &p_path);
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ bool GDScriptLanguage::supports_documentation() const {
}

int GDScriptLanguage::find_function(const String &p_function, const String &p_code) const {
GDScriptTokenizer tokenizer;
GDScriptTokenizerText tokenizer;
tokenizer.set_source_code(p_code);
int indent = 0;
GDScriptTokenizer::Token current = tokenizer.scan();
Expand Down
Loading
Loading