Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix C# main screen plugin example and document C# EditorInterface singleton #9399

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

easterNday
Copy link
Contributor

@easterNday easterNday commented May 19, 2024

Correct the syntax errors in the plugin development section, specifically by modifying EditorInterface to EditorInterface.Singleton in the C# portion. It is imperative to utilise the singleton pattern here to ensure the plugin successfully compiles.

Correct the syntax errors in the plugin development section, specifically by modifying 'EditorInterface' to 'EditorInterface.Singleton' in the C# portion. It is imperative to utilise the singleton pattern here to ensure the plugin successfully compiles.
@AThousandShips
Copy link
Member

Are you sure this is needed? See here

@AThousandShips AThousandShips requested a review from a team May 19, 2024 14:52
@AThousandShips AThousandShips added topic:dotnet area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:plugin labels May 19, 2024
@easterNday
Copy link
Contributor Author

easterNday commented May 19, 2024

The EditorInterface isn't a static class, and in the course of developing plugins for Godot's main screen, it's essential to utilise the singleton instance of EditorInterface; I fancy this to be a prerequisite. Indeed, attempts to proceed without incorporating this singleton would meet with compilation obstacles, rendering the process unfeasible.
image
image

@paulloz
Copy link
Member

paulloz commented May 19, 2024

Are you sure this is needed?

Yes, it is totally needed 🙂

@AThousandShips
Copy link
Member

Then this singleton is an exception, should probably be added to the documentation

Provide explanations for Singleton exceptions of `EditorInterface`
@easterNday
Copy link
Contributor Author

I'm not entirely sure if this fully meets your requirements, but I have added a specific exception explanation to that section.

@paulloz
Copy link
Member

paulloz commented May 19, 2024

I think if we want to make a table listing everywhere .Singleton is necessary, we should do it exhaustively (could be done in its own PR). IMHO, an incomplete list is as bad as the current status quo.
FWIW, examples in the class reference are already up-to-date. E.g.

return EditorInterface.Singleton.GetEditorTheme().GetIcon("Node", "EditorIcons");

@easterNday
Copy link
Contributor Author

I think if we want to make a table listing everywhere .Singleton is necessary, we should do it exhaustively (could be done in its own PR). IMHO, an incomplete list is as bad as the current status quo.

You're right, I will follow your advice and complete this list accordingly.

@raulsntos
Copy link
Member

EditorInterface is the only one:

https://github.com/godotengine/godot/blob/daa81bbb7d1c6d75d1711595604178ee62a5801d/modules/mono/editor/bindings_generator.cpp#L4801-L4803

This is because historically this class wasn't registered as a singleton, so when it became one in godotengine/godot#75694 we couldn't make it static without breaking compat.

All the other singletons are static classes, although they also have a Singleton property. In this case using the static class or the instance makes no difference.

@tetrapod00 tetrapod00 changed the title 🐞 fix(plugins): fix syntax errors Fix C# main screen plugin example and document C# EditorInterface singleton Dec 10, 2024
Copy link
Contributor

@tetrapod00 tetrapod00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current state was also verbally approved by raulsntos in the RC: https://chat.godotengine.org/channel/dotnet?msg=mJnHFdxJM75PhnRWN

Since the author turned off edits by maintainers, I am merging the current acceptable state and will make a followup PR for the wording adjustments.

@tetrapod00 tetrapod00 merged commit f967e9e into godotengine:master Dec 10, 2024
1 check passed
@tetrapod00
Copy link
Contributor

Thank you, and congrats on your first merged contribution to the Godot documentation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation bug cherrypick:4.3 topic:dotnet topic:plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugins docs on Dotnet C# EditorInterface to get singleton is wrong and misleading
5 participants