Fix API hash related crash in EditorSettings

[AThousandShips]

Crash due to manipulating API after getting the hash, which happens with --verbose

Instead of registering EditorSettings as abstract, and then registering as complete when creating an instance, we now register it as complete from the start.

This behavior is equivalent to how it worked prior to #78615, where the built-in documentation doesn't show the default values of EditorSettings, the abstract state of EditorSettings is and has always been false, and it is and has been possible to instance it from scripts (required for the loading of the settings)

I don't think the loss of the built-in documentation for these settings is a major loss, EditorSettings itself remains in the documentation, and the results of --doctool remains the same

In summary, I think this is a good balanced solution to the issue, without having to mess with the specific registration location of EditorSettings. Unless there is some reason for it being abstract that I have missed (other than the documentation generation)

• Fixes: Assert in ClassDB::set_current_api in latest master when launched w/ --verbose #80062

[RandomShaper]

Seems to fix the issue rationally. However, I'd still like more eyes looking into this.

[YuriSizov]

I don't think the loss of the built-in documentation for these settings is a major loss, EditorSettings itself remains in the documentation, and the results of --doctool remains the same

I don't understand from this description what we are losing here. Just the default values? We don't have them documented anyway...

and it is and has been possible to instance it from scripts (required for the loading of the settings)

EditorSettings should not be instanced from scripts. It should be accessed through the EditorInterface instance.

[AThousandShips]

I don't understand from this description what we are losing here.

They weren't documented before, in current master have values documented only in built-in, so not anything lost IMO

I think we shouldn't generate anything with --doctool because of the above reasons

EditorSettings should not be instanced from scripts.

Agreed, but this is possible in the current master, don't think we can block it without also losing the ability to load it, think this needs a larger rework

The current implementation stores the settings by serialisation as .res, which requires the type to be concrete, a rework would be extensive I'd say, though it'd be useful to serialise it custom

[YuriSizov]

They weren't documented before, in current master have values documented only in built-in, so not anything lost IMO

Oh, you mean that in the built-in help we get the default values added despite them not being present in the XML docs? Well, that could be a problem if we want to show default values in on-hover tooltips. But we don't right now, so I agree that this is fine.

Agreed, but this is possible in the current master, don't think we can block it without also losing the ability to load it, think this needs a larger rework

We don't need to block it. But we shouldn't consider this as a valid use case either. Though currently the docs for EditorSettings are missing a mention that you should access them through the interface, I think. Worth adding, as we do it in some other classes.

[AThousandShips]

Though currently the docs for EditorSettings are missing a mention that you should access them through the interface, I think.

They already do fortunately:

Note: This class shouldn't be instantiated directly. Instead, access the singleton using EditorInterface.get_editor_settings.

Thank you for checking these things, important questions

As far as I know creating a new EditorSettings instance on your own doesn't have any sideeffects (though it does seem to affect some index counting that might break things, can be worth investigating, but this PR has no effect on that either way)

[akien-mga]

Makes sense. It's somewhat cleaner than the previous code registering EditorSettings twice.

[akien-mga]

Thanks!

[AThousandShips]

Thank you!