Replace node definition string-based lookups with DefinitionIdentifier instances

[adamgerhant]

Improves the definition reference system by referencing nodes with DefinitionIdentifier instead of strings. This means protonodes use their identifier to reference their definition, instead of an arbitrary string.

Sets the foundation for an improved migration system and linking nodes to their definition when serializing.

For example:
instead of

resolve_document_node_type("Star")

you would do

resolve_document_node_type(&DefinitionIdentifier::ProtoNode(graphene_std::vector::generator_nodes::star::IDENTIFIER))

Another example

resolve_document_node_type("Merge")

to

resolve_document_node_type(&DefinitionIdentifer::Network("Merge".to_string()))

This PR also moves

pub reference: Option<String>,

from struct DocumentNodePersistentMetadata to its descendant struct NodeNetworkPersistentMetadata (but only when it's a network node, not a proto node).

[adamgerhant]

Removed since the output name of a protonode should not be a single random output type, as this can change and should be based on the current graph state.

[adamgerhant]

not possible to include this migration since it relies on changing reference to not run again, which is no longer possible.

[Keavon]

Marking as a draft while a few issues with crashes on document opening are happening due to some weirdness with migrations. @adamgerhant has the files to reproduce this with in our chat.

Also, please help by fixing the tests that are failing, thanks!

[TrueDoctor]

This branch should only be taken if the node is not registered by the node macro right? I don't think we have any nodes which are not registered by the node macro, so this should never be taken. Also if we do take it, the name would be weird, FlattenPath instead of Flatten Path

[adamgerhant]

Yes, this branch is for nodes with #[node_macro::node(skip_impl)] like the monitor node. It could have a space added before each capital letter

[adamgerhant]

Would it be possible for the node macro to add metadata to NODE_METADATA for nodes with #[node_macro::node(skip_impl)] so its purpose is to only not add to the registry?

[TrueDoctor]

That is exactly how the node macro behaves, the metadata is generated regardless of whether the skip_impl is used. Again the only way I see for this branch being taken is if a node is not registered using the node macro

[Keavon]

I was confident that was true, but then we looked into it and it seems (maybe due to a logic bug?) that it's surprisingly not actually so. Can you please double-check @adamgerhant's conclusion about this, and maybe suggest how it ought to be fixed if what I'm conveying is indeed accurate?

[TrueDoctor]

I mean you could have trusted my comment or double-checked by just looking at the code in codegen.rs but by design the skip_impl attribute only skips the addition of implementation and specifically not the metadata registration / implementation + codgen of the node so you add your own implementations in the node registry.
So adams first suggestion is not relevant since this entire branch is dead code. The only nodes I am aware of which are not generated with the macro are ones which would never be accessible to the user.
Regarding the second suggestion, that is how this feature has worked since I have implemented it