Remove GraphNode's comment property and related functionality

[Geometror]

(this functionality will be reintroduced in a new and more flexible way)

Part of the graph editor refactoring process.
Comment nodes in the current form aren't really working well, especially with several of them on top of each other.
Improved, special Nodes for organizing the graph will be reintroduced at a later time.
(There are already some experiments, but a satisfying solution hasn't been found yet)

[YuriSizov]

@dsnopek Since this class has been marked as experimental since 4.0, and we do intend to make further changes to the public API, what's the best way to handle the GDExtension compatibility part? Same question for @raulsntos regarding C#, I guess.

We can discuss this after all 3 PRs are merged too (this one, #79308, and #79311 are merged). May be easier to figure out the scope of BC breakage at that point.

[YuriSizov]

Looks good to me. VShader comments are preserved after this, and most of the logic is maintained, just made inaccessible (so restoring it at a later date should be rather easy).

[YuriSizov]

Thanks! Merging now to unblock the remaining PRs. Changes for my question about compatibility can be done in a follow-up.

[raulsntos]

I think, since this API was marked as experimental, breaking compat here hopefully won't catch users off guard. So I think documenting the breakage like we did with the changes to the Navigation API for 4.1¹ is probably good enough.

Footnotes

1. https://docs.godotengine.org/en/latest/tutorials/migrating/upgrading_to_godot_4.1.html#navigation ↩

[aaronfranke]

Which pull request will be reintroducing comments?

[EnlightenedOne]

I was just prepping a defect for this, figuring out why between 4.1.2 and 4.2 we lost such a crucial feature. Im really shocked this hasn't made more noise and that it was intentional.

Look at what creators publishing shader tutorials are having to do since this was removed (source Minions Art, publicly visible):

Please can descriptions be added to nodes that might die? Nothing here for any new Godot user to indicate this is an experimental feature. Trying to migrate shaders to visual ones for greater compatibility it is unnerving to see nodes vanish:

I wouldn't mind if the replacement was available and migration was manual (thats why we backup doing migrations!) but 4.2 VisualShaders are now simply undocumentable.

[YuriSizov]

@EnlightenedOne The entire graph edit system was marked as an experimental feature. The comments in visual shaders are, however, preserved, just not visible. For them the feature is disabled, not removed. Once the implementation for frame/comment nodes is ready, all existing comments will be visible again.

[EnlightenedOne]

You are correct, I thought they had been wiped from the most recent tres file but they are there still. I still do not understand disabling it before a replacement is ready, looks zero sum from the outside especially if you intend to port the existing comments.

How does a laymen find out if part of Godot is experimental? The UI for Godot doesn't hint at it on the tab "Shader Editor" in any way, creating a new VisualShader doesn't have any note on it. The docs don't have a warning or marker on them https://docs.godotengine.org/en/stable/tutorials/shaders/visual_shaders.html I had kind of assumed that it was a WIP.

[YuriSizov]

@EnlightenedOne The graph edit system was going through a rework, which this is just one step of. Creating a robust solution for comment/frame nodes is not a straightforward task, and existing comment nodes had a few usability issues to prove it. So we decided that as part of the rework, which was already considered a breaking change, we will temporarily remove the feature, let the work continue, and then reintroduce comments in a future release.

You are correct, that this is not communicated well to the visual shader users. The thing is, the graph edit system is a widget, a node that you can use in your projects. It also powers graph tools in the editor, but you may not know that and thus you won't pay attention to the graph edit changes and won't consider them affecting you. That's an issue on our part. We can probably add a note to the documentation at least, but I guess not much else can be done now.

Comment nodes should hopefully make a return in 4.3. At least it's a priority to @Geometror to make it happen.

[EnlightenedOne]

Thank you for the quick updates, hopefully this wasn't seen as overtly hostile you are doing incredible work here. Still odd removing it in 4.2 without having the replacement in hand but I am glad its on the radar for 4.3.

[Geometror]

@EnlightenedOne It's now implemented for 4.3 (#88014), but previously created comment nodes unfortunately aren't restored since the new frames work fundamentally different.

[YuriSizov]

but previously created comment nodes unfortunately aren't restored since the new frames work fundamentally different.

This shouldn't be a problem for visual shaders, since their internal data doesn't depend on the GraphEdit implementation. Surely new nodes can be recreated from that data with old information and positional data?

[Geometror]

Well, yes, their internal data doesn't depend on the GraphEdit implementation, but restoring them wouldn't be really helpful IMO since the users would need to manually attach all nodes and the description was also removed (with the intention of having a real comment node like a draggable label in the future in case people miss this feature). Of course one could write some sophisticated code that checks which nodes are enclosed by the rect of the restored node and attaches them, but to be honest I don't think that is worth the time.
And we would need to bring back VisualShaderNodeComment as a stub because I don't think there is a way to convert serialized resources that aren't registered anymore.

[QbieShay]

Do you think you could check how much work it is to bring back comments? I think previously it was said that they would be forward compatible (so once new nodes are in, they'd reuse the previous ones) I know a bunch of people were upset with the removal and I believe it's worth to make an assessment