Expose set_edited and is_edited on EditorInterface

[Naros]

Exposes several tools-based methods on Object.

The reason to expose these is simple. If an editor plugin provides a EditorResourcePreviewGenerator implementation to create thumbnails for custom resources, there isn't a way for the plugin to have the EditorResourcePreview regenerate thumbnails when the resource is modified. In the engine, this is handled by calling set_edited(true), which increments the edited version that is used by the hash algorithm to determine whether the cached thumbnail should be discarded and regenerated.

By exposing these three methods, addons can easily use the thumbnail generator framework and regenerate the thumbnails when the underlying resource is changed rather than requiring an editor restart.

[RedMser]

You may want to include information about when this is changed to true/false by the engine. For example the fact that it is set to false on save.

[Naros]

Updated with the suggestions, thanks.

[sanderfoobar]

Looks OK, its just adding existing methods to the ClassDB registration so shouldn't think too hard about merging this.

[Mickeon]

Starting thoughts, I'm not particularly fond of this. Mainly, these methods feel too "technical", "mechanical" to be needing to be exposed. The additional documentation does not help. Yes, these are used by the engine, but what can the user... use this for?
The above questioning is why we decided to "unexpose" Resource's setup_local_to_scene (#67082). The method is literally just an internal method exposed for little to no reason: The user could implement it on their own, and documenting its internal purpose was difficult. So we gotta be a bit careful to expose these willy-nilly.

A quick skim over this has me believe that it would be a better fit to expose this in Resource, instead.

And furthermore, these are purely editor-exclusive, and very situational. Too much to be in Object, personally.
This has me think there's got to be a better way to accomplish what one may need this for. Namely, exposing these methods through another Editor-exclusive class.

[Mickeon]

Suggested change

	Set whether the object has been edited based on the [param edited] value.
	If [param edited] is [code]true[/code], the object is marked as edited.

[Mickeon]

Suggested change

	Returns whether the object has been marked as edited using [method set_edited].
	Returns [code]true[/code] if the object has been marked as edited through [method set_edited].

[Mickeon]

Suggested change

	Returns the object's current edited version. This version is incremented each time the object's [method set_edited] is called.
	Returns the object's current edited version. This value is incremented each time the object's [method set_edited] is called.

[Naros]

Starting thoughts, I'm not particularly fond of this. Mainly, these methods feel too "technical", "mechanical" to be needing to be exposed. The additional documentation does not help. Yes, these are used by the engine, but what can the user... use this for?

Hi, @Mickeon. There are several reasons for exposing these methods.

set_edited:

It's common practice in when serializing sub-resources during a save operation to set the resource edited state to false. Without this method, any custom ResourceFormatSaver has no way to reset this state, and the engine will continue to view the resource as modified. This creates a divide in how C++ modules and GDExtension work for plug-ins, meaning that GDExtensions that have custom resource loader/saver are incapable of handling this use case properly.

is_edited:

This one is chosen to be exposed mainly because if GDExtension needs a way to set the state to true or false, it may need a way in the future to query the edited state of the object. So this is purely for convenience.

get_edited_version:

This was being exposed primarily to support hash_edited_version_for_preview; however, in hindsight, maybe it would make more sense to change this and expose hash_edited_version_for_preview instead so that again resources exposed in GDExtension that may want to disable previews, like TileSet, can do so.

A quick skim over this has me believe that it would be a better fit to expose this in Resource, instead.

And furthermore, these are purely editor-exclusive, and very situational. Too much to be in Object, personally. This has me think there's got to be a better way to accomplish what one may need this for. Namely, exposing these methods through another Editor-exclusive class.

For our purposes, exposing this on the Resource class would definitely work, given that's our prime use case with our custom format loader/saver for our plug-in's data; however, I wonder if placing these on EditorInterface would also be better for other non-resource types, such as ConfigFile?

void EditorInterface::set_object_edited(Object* p_object, bool p_edited);
bool EditorInterface::is_object_edited(Object* p_object) const;

As for the get_edited_version or hash_edited_version_for_preview, I think maybe the best play is to expose the latter on the Resource class with a GDVIRTUAL call that can be implemented by custom resources.

Whats your thoughts on that approach @Mickeon ?

[Mickeon]

Sorry for not having much else to add. I am mostly looking at this from a general user perspective. Whether this is actually implemented should not be up to me.

I wonder if placing these on EditorInterface would also be better for other non-resource types, such as ConfigFile?

As I hinted at, delegating this to another class may be a better choice. So, personally, yes.

[Naros]

@AThousandShips so I decided on the following, in an attempt to get this into 4.4.

1. I removed exposing the get_edited_version from this PR. That is more directed toward rendering of thumbnails and I'd like to look at that work from a separate PR as I believe there may be other changes to align GDE and C++ modules in this area.
2. I removed exposing the set_edited and is_edited on Object and provided delegators from EditorInterface instead.

Let me know if you think anything else is needed here.