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

[DRAFT] GDExtension: Add compatibility system for virtual methods #100674

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Changes from 1 commit
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
Next Next commit
Add save integrity levels.
This PR adds an argument to most save functions to specify what level of
integrity should Godot attempt to guarantee when writing a file.

This in preparation for PR #98361; which will introduce a full sync
barrier when performing save operations to avoid data loss.

This PR puts such expensive sync() behind the enum
SAVE_INTEGRITY_SAVE_SWAP_PLUS_SYNC; which should be used sparingly for
important information.

This PR also sets most cache files to be saved as either
SAVE_INTEGRITY_SAVE_SWAP (old default behavior) or SAVE_INTEGRITY_NONE;
because Godot can easily generate thousands of these file saving
operations. Performing thousands of full syncs() would be too expensive,
specially when these files can be regenerated if they're corrupted or
lost.

The PR is incomplete as it does not yet expose integrity levels to the
user, which is relevant for certain file saving operations, such as when
game save files that store the player's progress (those should be done
with SAVE_INTEGRITY_SAVE_SWAP_PLUS_SYNC).
darksylinc committed Dec 19, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 6f79f9339df2e352201eae012770734244547c8f
6 changes: 3 additions & 3 deletions core/config/project_settings.cpp
Original file line number Diff line number Diff line change
@@ -884,7 +884,7 @@ Error ProjectSettings::save() {

Error ProjectSettings::_save_settings_binary(const String &p_file, const RBMap<String, List<String>> &p_props, const CustomMap &p_custom, const String &p_custom_features) {
Error err;
Ref<FileAccess> file = FileAccess::open(p_file, FileAccess::WRITE, &err);
Ref<FileAccess> file = FileAccess::open(p_file, FileAccess::WRITE, &err, FileAccess::SAVE_INTEGRITY_SAVE_SWAP_PLUS_SYNC);
ERR_FAIL_COND_V_MSG(err != OK, err, vformat("Couldn't save project.binary at '%s'.", p_file));

uint8_t hdr[4] = { 'E', 'C', 'F', 'G' };
@@ -953,7 +953,7 @@ Error ProjectSettings::_save_settings_binary(const String &p_file, const RBMap<S

Error ProjectSettings::_save_settings_text(const String &p_file, const RBMap<String, List<String>> &p_props, const CustomMap &p_custom, const String &p_custom_features) {
Error err;
Ref<FileAccess> file = FileAccess::open(p_file, FileAccess::WRITE, &err);
Ref<FileAccess> file = FileAccess::open(p_file, FileAccess::WRITE, &err, FileAccess::SAVE_INTEGRITY_SAVE_SWAP_PLUS_SYNC);

ERR_FAIL_COND_V_MSG(err != OK, err, vformat("Couldn't save project.godot - %s.", p_file));

@@ -1261,7 +1261,7 @@ void ProjectSettings::store_global_class_list(const Array &p_classes) {
Ref<ConfigFile> cf;
cf.instantiate();
cf->set_value("", "list", p_classes);
cf->save(get_global_class_list_path());
cf->save(get_global_class_list_path(), FileAccess::SAVE_INTEGRITY_NONE);

global_class_list = p_classes;
}
12 changes: 11 additions & 1 deletion core/core_bind.cpp
Original file line number Diff line number Diff line change
@@ -238,8 +238,15 @@ void OS::close_midi_inputs() {
::OS::get_singleton()->close_midi_inputs();
}

#ifndef DISABLE_DEPRECATED
void OS::set_use_file_access_save_and_swap(bool p_enable) {
FileAccess::set_backup_save(p_enable);
WARN_DEPRECATED_MSG("Use set_default_save_integrity_level() instead.");
FileAccess::set_default_save_integrity_level(p_enable ? FileAccess::SAVE_INTEGRITY_SAVE_SWAP : FileAccess::SAVE_INTEGRITY_NONE);
}
#endif

void OS::set_default_save_integrity_level(FileAccess::SaveIntegrityLevel p_integrity_level) {
FileAccess::set_default_save_integrity_level(p_integrity_level);
}

void OS::set_low_processor_usage_mode(bool p_enabled) {
@@ -722,7 +729,10 @@ void OS::_bind_methods() {
ClassDB::bind_method(D_METHOD("is_keycode_unicode", "code"), &OS::is_keycode_unicode);
ClassDB::bind_method(D_METHOD("find_keycode_from_string", "string"), &OS::find_keycode_from_string);

#ifndef DISABLE_DEPRECATED
ClassDB::bind_method(D_METHOD("set_use_file_access_save_and_swap", "enabled"), &OS::set_use_file_access_save_and_swap);
#endif
ClassDB::bind_method(D_METHOD("set_default_save_integrity_level", "integrity_level"), &OS::set_default_save_integrity_level);

ClassDB::bind_method(D_METHOD("set_thread_name", "name"), &OS::set_thread_name);
ClassDB::bind_method(D_METHOD("get_thread_caller_id"), &OS::get_thread_caller_id);
3 changes: 3 additions & 0 deletions core/core_bind.h
Original file line number Diff line number Diff line change
@@ -227,7 +227,10 @@ class OS : public Object {
bool is_keycode_unicode(char32_t p_unicode) const;
Key find_keycode_from_string(const String &p_code) const;

#ifndef DISABLE_DEPRECATED
void set_use_file_access_save_and_swap(bool p_enable);
#endif
void set_default_save_integrity_level(FileAccess::SaveIntegrityLevel p_integrity_level);

uint64_t get_static_memory_usage() const;
uint64_t get_static_memory_peak_usage() const;
2 changes: 1 addition & 1 deletion core/crypto/crypto.cpp
Original file line number Diff line number Diff line change
@@ -228,7 +228,7 @@ String ResourceFormatLoaderCrypto::get_resource_type(const String &p_path) const
return "";
}

Error ResourceFormatSaverCrypto::save(const Ref<Resource> &p_resource, const String &p_path, uint32_t p_flags) {
Error ResourceFormatSaverCrypto::save(const Ref<Resource> &p_resource, const String &p_path, uint32_t p_flags, FileAccess::SaveIntegrityLevel p_integrity_level) {
Error err;
Ref<X509Certificate> cert = p_resource;
Ref<CryptoKey> key = p_resource;
2 changes: 1 addition & 1 deletion core/crypto/crypto.h
Original file line number Diff line number Diff line change
@@ -163,7 +163,7 @@ class ResourceFormatLoaderCrypto : public ResourceFormatLoader {

class ResourceFormatSaverCrypto : public ResourceFormatSaver {
public:
virtual Error save(const Ref<Resource> &p_resource, const String &p_path, uint32_t p_flags = 0) override;
virtual Error save(const Ref<Resource> &p_resource, const String &p_path, uint32_t p_flags = 0, FileAccess::SaveIntegrityLevel p_integrity_level = FileAccess::SAVE_INTEGRITY_DEFAULT) override;
virtual void get_recognized_extensions(const Ref<Resource> &p_resource, List<String> *p_extensions) const override;
virtual bool recognize(const Ref<Resource> &p_resource) const override;
};
41 changes: 41 additions & 0 deletions core/io/config_file.compat.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**************************************************************************/
/* config_file.compat.inc */
/**************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/**************************************************************************/
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/**************************************************************************/

#ifndef DISABLE_DEPRECATED

Error ConfigFile::_save_compat_100447(const String &p_path) {
return save(p_path);
}

void ConfigFile::_bind_compatibility_methods() {
ClassDB::bind_compatibility_method(D_METHOD("save", "path"), &ConfigFile::_save_compat_100447);
}

#endif
7 changes: 4 additions & 3 deletions core/io/config_file.cpp
Original file line number Diff line number Diff line change
@@ -29,6 +29,7 @@
/**************************************************************************/

#include "config_file.h"
#include "config_file.compat.inc"

#include "core/io/file_access_encrypted.h"
#include "core/os/keyboard.h"
@@ -153,9 +154,9 @@ String ConfigFile::encode_to_text() const {
return sb.as_string();
}

Error ConfigFile::save(const String &p_path) {
Error ConfigFile::save(const String &p_path, FileAccess::SaveIntegrityLevel p_integrity_level) {
Error err;
Ref<FileAccess> file = FileAccess::open(p_path, FileAccess::WRITE, &err);
Ref<FileAccess> file = FileAccess::open(p_path, FileAccess::WRITE, &err, p_integrity_level);

if (err) {
return err;
@@ -334,7 +335,7 @@ void ConfigFile::_bind_methods() {

ClassDB::bind_method(D_METHOD("load", "path"), &ConfigFile::load);
ClassDB::bind_method(D_METHOD("parse", "data"), &ConfigFile::parse);
ClassDB::bind_method(D_METHOD("save", "path"), &ConfigFile::save);
ClassDB::bind_method(D_METHOD("save", "path", "integrity_level"), &ConfigFile::save, DEFVAL(FileAccess::SAVE_INTEGRITY_DEFAULT));

ClassDB::bind_method(D_METHOD("encode_to_text"), &ConfigFile::encode_to_text);

8 changes: 7 additions & 1 deletion core/io/config_file.h
Original file line number Diff line number Diff line change
@@ -51,6 +51,12 @@ class ConfigFile : public RefCounted {
protected:
static void _bind_methods();

#ifndef DISABLE_DEPRECATED
Error _save_compat_100447(const String &p_path);

static void _bind_compatibility_methods();
#endif

public:
void set_value(const String &p_section, const String &p_key, const Variant &p_value);
Variant get_value(const String &p_section, const String &p_key, const Variant &p_default = Variant()) const;
@@ -64,7 +70,7 @@ class ConfigFile : public RefCounted {
void erase_section(const String &p_section);
void erase_section_key(const String &p_section, const String &p_key);

Error save(const String &p_path);
Error save(const String &p_path, FileAccess::SaveIntegrityLevel p_integrity_level = FileAccess::SAVE_INTEGRITY_DEFAULT);
Error load(const String &p_path);
Error parse(const String &p_data);

5 changes: 5 additions & 0 deletions core/io/file_access.compat.inc
Original file line number Diff line number Diff line change
@@ -30,6 +30,10 @@

#ifndef DISABLE_DEPRECATED

Ref<FileAccess> FileAccess::_open_bind_compat_100447(const String &p_path, ModeFlags p_mode_flags) {
return _open(p_path, p_mode_flags);
}

Ref<FileAccess> FileAccess::_open_encrypted_bind_compat_98918(const String &p_path, ModeFlags p_mode_flags, const Vector<uint8_t> &p_key) {
return open_encrypted(p_path, p_mode_flags, p_key, Vector<uint8_t>());
}
@@ -91,6 +95,7 @@ void FileAccess::store_pascal_string_bind_compat_78289(const String &p_string) {
}

void FileAccess::_bind_compatibility_methods() {
ClassDB::bind_compatibility_static_method("FileAccess", D_METHOD("open", "path", "flags"), &FileAccess::_open_bind_compat_100447);
ClassDB::bind_compatibility_static_method("FileAccess", D_METHOD("open_encrypted", "path", "mode_flags", "key"), &FileAccess::_open_encrypted_bind_compat_98918);

ClassDB::bind_compatibility_method(D_METHOD("store_8", "value"), &FileAccess::store_8_bind_compat_78289);
39 changes: 29 additions & 10 deletions core/io/file_access.cpp
Original file line number Diff line number Diff line change
@@ -44,7 +44,7 @@ FileAccess::CreateFunc FileAccess::create_func[ACCESS_MAX] = {};

FileAccess::FileCloseFailNotify FileAccess::close_fail_notify = nullptr;

bool FileAccess::backup_save = false;
FileAccess::SaveIntegrityLevel FileAccess::default_save_integrity_level = FileAccess::SAVE_INTEGRITY_NONE;
thread_local Error FileAccess::last_file_open_error = OK;

Ref<FileAccess> FileAccess::create(AccessType p_access) {
@@ -118,7 +118,7 @@ Ref<FileAccess> FileAccess::create_temp(int p_mode_flags, const String &p_prefix
{
// Create file first with WRITE mode.
// Otherwise, it would fail to open with a READ mode.
Ref<FileAccess> ret = FileAccess::open(path, FileAccess::ModeFlags::WRITE, &err);
Ref<FileAccess> ret = FileAccess::open(path, FileAccess::ModeFlags::WRITE, &err, SAVE_INTEGRITY_NONE);
if (err != OK) {
*r_error = err;
ERR_FAIL_V_MSG(Ref<FileAccess>(), vformat(R"(%s: could not create "%s".)", ERROR_COMMON_PREFIX, path));
@@ -158,11 +158,14 @@ void FileAccess::_delete_temp() {
DirAccess::remove_absolute(_temp_path);
}

Error FileAccess::reopen(const String &p_path, int p_mode_flags) {
return open_internal(p_path, p_mode_flags);
Error FileAccess::reopen(const String &p_path, int p_mode_flags, SaveIntegrityLevel p_integrity_level) {
if (p_integrity_level == SAVE_INTEGRITY_DEFAULT) {
p_integrity_level = default_save_integrity_level;
}
return open_internal(p_path, p_mode_flags, p_integrity_level);
}

Ref<FileAccess> FileAccess::open(const String &p_path, int p_mode_flags, Error *r_error) {
Ref<FileAccess> FileAccess::open(const String &p_path, int p_mode_flags, Error *r_error, SaveIntegrityLevel p_integrity_level) {
//try packed data first

Ref<FileAccess> ret;
@@ -176,8 +179,12 @@ Ref<FileAccess> FileAccess::open(const String &p_path, int p_mode_flags, Error *
}
}

if (p_integrity_level == SAVE_INTEGRITY_DEFAULT) {
p_integrity_level = default_save_integrity_level;
}

ret = create_for_path(p_path);
Error err = ret->open_internal(p_path, p_mode_flags);
Error err = ret->open_internal(p_path, p_mode_flags, p_integrity_level);

if (r_error) {
*r_error = err;
@@ -189,9 +196,9 @@ Ref<FileAccess> FileAccess::open(const String &p_path, int p_mode_flags, Error *
return ret;
}

Ref<FileAccess> FileAccess::_open(const String &p_path, ModeFlags p_mode_flags) {
Ref<FileAccess> FileAccess::_open(const String &p_path, ModeFlags p_mode_flags, SaveIntegrityLevel p_integrity_level) {
Error err = OK;
Ref<FileAccess> fa = open(p_path, p_mode_flags, &err);
Ref<FileAccess> fa = open(p_path, p_mode_flags, &err, p_integrity_level);
last_file_open_error = err;
if (err) {
return Ref<FileAccess>();
@@ -235,7 +242,7 @@ Ref<FileAccess> FileAccess::open_compressed(const String &p_path, ModeFlags p_mo
Ref<FileAccessCompressed> fac;
fac.instantiate();
fac->configure("GCPF", (Compression::Mode)p_compress_mode);
Error err = fac->open_internal(p_path, p_mode_flags);
Error err = fac->open_internal(p_path, p_mode_flags, SAVE_INTEGRITY_NONE);
last_file_open_error = err;
if (err) {
return Ref<FileAccess>();
@@ -821,6 +828,12 @@ String FileAccess::get_file_as_string(const String &p_path, Error *r_error) {
return ret;
}

void FileAccess::set_default_save_integrity_level(SaveIntegrityLevel p_integrity_level) {
ERR_FAIL_COND_MSG(p_integrity_level == SAVE_INTEGRITY_DEFAULT, "SAVE_INTEGRITY_DEFAULT is not a valid input setting.");
ERR_FAIL_INDEX(p_integrity_level, SAVE_INTEGRITY_MAX);
default_save_integrity_level = p_integrity_level;
}

String FileAccess::get_md5(const String &p_file) {
Ref<FileAccess> f = FileAccess::open(p_file, READ);
if (f.is_null()) {
@@ -903,7 +916,7 @@ String FileAccess::get_sha256(const String &p_file) {
}

void FileAccess::_bind_methods() {
ClassDB::bind_static_method("FileAccess", D_METHOD("open", "path", "flags"), &FileAccess::_open);
ClassDB::bind_static_method("FileAccess", D_METHOD("open", "path", "flags", "integrity_level"), &FileAccess::_open, DEFVAL(FileAccess::SAVE_INTEGRITY_DEFAULT));
ClassDB::bind_static_method("FileAccess", D_METHOD("open_encrypted", "path", "mode_flags", "key", "iv"), &FileAccess::open_encrypted, DEFVAL(Vector<uint8_t>()));
ClassDB::bind_static_method("FileAccess", D_METHOD("open_encrypted_with_pass", "path", "mode_flags", "pass"), &FileAccess::open_encrypted_pass);
ClassDB::bind_static_method("FileAccess", D_METHOD("open_compressed", "path", "mode_flags", "compression_mode"), &FileAccess::open_compressed, DEFVAL(0));
@@ -985,6 +998,12 @@ void FileAccess::_bind_methods() {
BIND_ENUM_CONSTANT(COMPRESSION_GZIP);
BIND_ENUM_CONSTANT(COMPRESSION_BROTLI);

BIND_ENUM_CONSTANT(SAVE_INTEGRITY_DEFAULT);
BIND_ENUM_CONSTANT(SAVE_INTEGRITY_NONE);
BIND_ENUM_CONSTANT(SAVE_INTEGRITY_SAVE_SWAP);
BIND_ENUM_CONSTANT(SAVE_INTEGRITY_SAVE_SWAP_PLUS_SYNC);
BIND_ENUM_CONSTANT(SAVE_INTEGRITY_MAX);

BIND_BITFIELD_FLAG(UNIX_READ_OWNER);
BIND_BITFIELD_FLAG(UNIX_WRITE_OWNER);
BIND_BITFIELD_FLAG(UNIX_EXECUTE_OWNER);
29 changes: 21 additions & 8 deletions core/io/file_access.h
Original file line number Diff line number Diff line change
@@ -84,6 +84,16 @@ class FileAccess : public RefCounted {
COMPRESSION_BROTLI = Compression::MODE_BROTLI,
};

// These enum values only make sense when p_mode_flags == WRITE and the underlying FileAccess
// type supports it (e.g. regular files support it, but zip or encrypted files do not).
enum SaveIntegrityLevel {
SAVE_INTEGRITY_DEFAULT,
SAVE_INTEGRITY_NONE,
SAVE_INTEGRITY_SAVE_SWAP,
SAVE_INTEGRITY_SAVE_SWAP_PLUS_SYNC,
SAVE_INTEGRITY_MAX,
};

typedef void (*FileCloseFailNotify)(const String &);

typedef Ref<FileAccess> (*CreateFunc)();
@@ -103,13 +113,15 @@ class FileAccess : public RefCounted {

AccessType get_access_type() const;
virtual String fix_path(const String &p_path) const;
virtual Error open_internal(const String &p_path, int p_mode_flags) = 0; ///< open a file
virtual Error open_internal(const String &p_path, int p_mode_flags, SaveIntegrityLevel p_integrity_level) = 0; ///< open a file
virtual uint64_t _get_modified_time(const String &p_file) = 0;
virtual void _set_access_type(AccessType p_access);

static FileCloseFailNotify close_fail_notify;

#ifndef DISABLE_DEPRECATED
static Ref<FileAccess> _open_bind_compat_100447(const String &p_path, ModeFlags p_mode_flags);

static Ref<FileAccess> _open_encrypted_bind_compat_98918(const String &p_path, ModeFlags p_mode_flags, const Vector<uint8_t> &p_key);

void store_8_bind_compat_78289(uint8_t p_dest);
@@ -131,7 +143,7 @@ class FileAccess : public RefCounted {
#endif

private:
static bool backup_save;
static SaveIntegrityLevel default_save_integrity_level;
thread_local static Error last_file_open_error;

AccessType _access_type = ACCESS_FILESYSTEM;
@@ -141,7 +153,7 @@ class FileAccess : public RefCounted {
return memnew(T);
}

static Ref<FileAccess> _open(const String &p_path, ModeFlags p_mode_flags);
static Ref<FileAccess> _open(const String &p_path, ModeFlags p_mode_flags, SaveIntegrityLevel p_integrity_level = FileAccess::SAVE_INTEGRITY_DEFAULT);

bool _is_temp_file = false;
bool _temp_keep_after_use = false;
@@ -224,11 +236,11 @@ class FileAccess : public RefCounted {

virtual bool file_exists(const String &p_name) = 0; ///< return true if a file exists

virtual Error reopen(const String &p_path, int p_mode_flags); ///< does not change the AccessType
virtual Error reopen(const String &p_path, int p_mode_flags, SaveIntegrityLevel p_integrity_level = SAVE_INTEGRITY_DEFAULT); ///< does not change the AccessType

static Ref<FileAccess> create(AccessType p_access); /// Create a file access (for the current platform) this is the only portable way of accessing files.
static Ref<FileAccess> create_for_path(const String &p_path);
static Ref<FileAccess> open(const String &p_path, int p_mode_flags, Error *r_error = nullptr); /// Create a file access (for the current platform) this is the only portable way of accessing files.
static Ref<FileAccess> open(const String &p_path, int p_mode_flags, Error *r_error = nullptr, SaveIntegrityLevel p_integrity_level = SAVE_INTEGRITY_DEFAULT); /// Create a file access (for the current platform) this is the only portable way of accessing files.
static Ref<FileAccess> create_temp(int p_mode_flags, const String &p_prefix = "", const String &p_extension = "", bool p_keep = false, Error *r_error = nullptr);

static Ref<FileAccess> open_encrypted(const String &p_path, ModeFlags p_mode_flags, const Vector<uint8_t> &p_key, const Vector<uint8_t> &p_iv = Vector<uint8_t>());
@@ -247,8 +259,8 @@ class FileAccess : public RefCounted {
static bool get_read_only_attribute(const String &p_file);
static Error set_read_only_attribute(const String &p_file, bool p_ro);

static void set_backup_save(bool p_enable) { backup_save = p_enable; }
static bool is_backup_save_enabled() { return backup_save; }
static void set_default_save_integrity_level(SaveIntegrityLevel p_integrity_level);
static SaveIntegrityLevel get_default_save_integrity_level() { return default_save_integrity_level; }

static String get_md5(const String &p_file);
static String get_sha256(const String &p_file);
@@ -267,11 +279,12 @@ class FileAccess : public RefCounted {

public:
FileAccess() {}
virtual ~FileAccess();
virtual ~FileAccess() override;
};

VARIANT_ENUM_CAST(FileAccess::CompressionMode);
VARIANT_ENUM_CAST(FileAccess::ModeFlags);
VARIANT_ENUM_CAST(FileAccess::SaveIntegrityLevel);
VARIANT_BITFIELD_CAST(FileAccess::UnixPermissionFlags);

#endif // FILE_ACCESS_H
Loading