Add save integrity levels.

[darksylinc]

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]

The function ResourceFormatSaver::save performs a GDVIRTUAL_CALL():

Error ResourceFormatSaver::save(const Ref<Resource> &p_resource, const String &p_path, uint32_t p_flags, const int p_integrity_level) {
	Error err = ERR_METHOD_NOT_FOUND;
	GDVIRTUAL_CALL(_save, p_resource, p_path, p_flags, p_integrity_level, err);
	return err;
}

I don't know if this breaks the GD ABI and if so, how to implement a fallback function.

[RandomShaper]

It may be better to fuse the integrity flags with the mode flags to preserve compatibility.

[darksylinc]

It may be better to fuse the integrity flags with the mode flags to preserve compatibility.

Ahhh!!!! That's a lot more KISS. I like it. Way less effort and more backwards compatible. I'll incorporate that change.

[darksylinc]

Ahhh!!!! That's a lot more KISS. I like it. Way less effort and more backwards compatible. I'll incorporate that change.

Argh!

I spoke too soon.

The main problem is ResourceFormatSaver::save and similar virtual functions. But unfortunately they do not include access flags (nor they should). The value of making integrity levels part of access types is a lot less appealing now.

[bruvzg]

It may be better to fuse the integrity flags with the mode flags to preserve compatibility.

Not sure if it's better compatibility:

• custom FileAccess implementations might use (internal do exactly this) (mode_flags == WRITE) or similar checks.
• ModeFlags name imply that it is i/o mode of operation not generic flags, and it is defined as Enum, not BitField (renaming it and changing type is more breaking than adding new argument).

[darksylinc]

It may be better to fuse the integrity flags with the mode flags to preserve compatibility.

Not sure if it's better compatibility:

* custom `FileAccess` implementations might use (internal do exactly this) `(mode_flags == WRITE)` or similar checks.

* `ModeFlags` name imply that it is i/o mode of operation not generic flags, and it is defined as `Enum`, not `BitField` (renaming it and changing type is more breaking than adding new argument).

Gotcha. Yeah, I think I prefer the current solution of using a separate enum.

Slightly unrelated to that, I think I prefer that, instead of modifying ResourceSaver::save to add p_integrity_level to each function overload, to modify SaverFlags:

enum SaverFlags {
		FLAG_NONE = 0,
		FLAG_RELATIVE_PATHS = 1,
		FLAG_BUNDLE_RESOURCES = 2,
		FLAG_CHANGE_PATH = 4,
		FLAG_OMIT_EDITOR_PROPERTIES = 8,
		FLAG_SAVE_BIG_ENDIAN = 16,
		FLAG_COMPRESS = 32,
		FLAG_REPLACE_SUBRESOURCE_PATHS = 64,
		FLAG_SAVE_INTEGRITY_NONE = 128, // New flag!
		FLAG_SAVE_INTEGRITY_SAVE_SWAP = 256, // New flag!
		FLAG_SAVE_INTEGRITY_SAVE_SWAP_PLUS_SYNC = FLAG_SAVE_INTEGRITY_NONE | FLAG_SAVE_INTEGRITY_SAVE_SWAP, // New flag!
	};

Not setting any flag will use SAVE_INTEGRITY_DEFAULT. Else you can explicitly specify what save integrity you want. And this is a lot more backwards compatible.

[RandomShaper]

As much as I'm a fan of trying to overload the current enums, that looks too awkward: swap + sync = none + swap

[darksylinc]

I've got a question.

There is this validation error:

Validate extension JSON: Error: Field 'classes/ResourceFormatSaver/methods/_save/arguments': size changed value in new API, from 3 to 4.

That belongs to a GDVIRTUAL:

class ResourceFormatSaver : public RefCounted {
	// BEFORE:
	GDVIRTUAL3R(Error, _save, Ref<Resource>, String, uint32_t)
	// AFTER:
	GDVIRTUAL4R(Error, _save, Ref<Resource>, String, uint32_t, int)
};

Error ResourceFormatSaver::save(const Ref<Resource> &p_resource, const String &p_path, uint32_t p_flags, int p_integrity_level) {
	Error err = ERR_METHOD_NOT_FOUND;
	// BEFORE:
	GDVIRTUAL_CALL(_save, p_resource, p_path, p_flags, err);
	// AFTER:
	GDVIRTUAL_CALL(_save, p_resource, p_path, p_flags, p_integrity_level, err);
	return err;
}

void ResourceFormatSaver::_bind_methods() {
	// BEFORE:
	GDVIRTUAL_BIND(_save, "resource", "path", "flags");
	// AFTER:
	GDVIRTUAL_BIND(_save, "resource", "path", "flags", "integrity_level");
}

Is there a way to deal with this in a backwards compatible way? Or do we just drop the integrity_level argument?

[dsnopek]

Is there a way to deal with this in a backwards compatible way?

Unfortunately, there isn't a way to change the signature of virtual methods in a backwards compatible way.

One option is adding a new virtual method with the new signature, and then using GDVIRTUAL_IS_OVERRIDDEN() to call the new one if implemented, and then fall back on the old one if it isn't implemented?

I don't think we've actually done that anywhere yet - so far, I think we've always found some other way to work around it, so I don't have any examples to point to. However, we have a similar issue with GDExtension interface functions, and there we have the convention of adding an incrementing integer to each new version of the function. Not the most attractive API, but it works...

[darksylinc]

All checks passed. All questions cleared. This PR is ready for review.

[darksylinc]

I've rebased this PR to latest master. It's ready for review and merge.

Unlike last time, now we have the infrastructure for backwards compatibility with GD script virtual calls.