Skip to content

Conversation

@gogojokid
Copy link
Contributor

@gogojokid gogojokid commented Aug 19, 2023

4.1.1.stable

https://docs.godotengine.org/en/latest/community/contributing/content_guidelines.html

In the ".gdextension" code block, the [Icon] tag does not work in 4.1.1.stable version, and it does not match the naming conventions of other tags. I test it, [icons] tag is worked well. In addition, I added a blank lines between the tag and the content to make it more consistent with other ".gdextension" code block formatting.

Bugsquad edit: Fixes #7885

@AThousandShips AThousandShips added the area:manual Issues and PRs related to the Manual/Tutorials section of the documentation label Aug 20, 2023
@AThousandShips AThousandShips changed the title Custom editor icon section's command wrong or out of data. Custom editor icon section's command wrong or out of date Aug 20, 2023
Copy link
Contributor

@capnm capnm left a comment

Choose a reason for hiding this comment

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

Maybe change the name of the icon, it's not obvious that it doesn't have to be the same, e.g. "res://icon_gdexample.svg".

@AThousandShips
Copy link
Member

The standard recommended procedure is to have icons stored in such a directory, see all the modules in the engine

Copy link
Contributor

@paddy-exe paddy-exe left a comment

Choose a reason for hiding this comment

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

Looks good. Had a look at Yuri's implementation PR details and [icons] is correct. Thanks for the correction!

@capnm

This comment was marked as outdated.

@AThousandShips
Copy link
Member

Can you please update your commit message, it says "out of data"

@YuriSizov
Copy link
Contributor

Commit message looks correct to me 👀

@AThousandShips
Copy link
Member

"Custom editor icon section's command wrong or out of data." not "date"

@YuriSizov
Copy link
Contributor

Ah, I thought you were seeing "out of data" as the entire commit message for some reason. My bad.

@AThousandShips
Copy link
Member

The message still says "out of data"

@gogojokid
Copy link
Contributor Author

The message still says "out of data"

I am sorry, I spent some time looking at Godot's PR process and didn't notice it, it will change soon.

In the "*.gdextension" code block, the [Icon] tag does not work in 4.1.1.stable version, and it does not match the naming conventions of other tags. I test it, [icons] tag is worked well. In addition, I added a blank lines between the tag and the content to make it more consistent with other "*.gdextension" code block formatting.

Update with capnm's suggestion. Thank you all for your attention and reply!
@capnm

This comment was marked as outdated.

@AThousandShips
Copy link
Member

I think this is outside the scope of this PR, which is fixing a simple out of date description, needs to be discussed and looked into

@capnm
Copy link
Contributor

capnm commented Sep 3, 2023

I'm now 'out of data' 😄 and hope that someone someday hits the commit button.

@gogojokid
Copy link
Contributor Author

I think we should use "res://" here to reduce confusion, because on this page and elsewhere in the documentation, "res://" is always explicitly used for relative paths. Since it is a relative path, it means that the user can specify it freely, and technically can also use the path on the upper layer of the addon without any restrictions. For example, if you only make GDExtensions for your own use, these extensions can all read icons from the recommended global path "res://icons" for sharing. But if publishing them, it would be better to encapsulate the icons in the "res://addon/your_extension" directory.

To make this clearer, some more explanation could be added, and an example added, or stated in the Plugins section, since there seems to be no other mention of addons in this section. However, this PR is indeed just a simple fixing (sorry I didn’t use “Fixes” to start the title because I was not familiar with the PR process before). I agree with AtousandShips, we can merge it as soon as possible so that new users will no longer get the wrong information. Perhaps capnm can submit an Issue or PR as a discussion of Documentation Enhancement?

@mhilbrunner mhilbrunner merged commit 8e69f6b into godotengine:master Sep 11, 2023
@mhilbrunner
Copy link
Member

mhilbrunner commented Sep 11, 2023

Merged. Thanks and congrats on your first merged contribution to Godot's docs!

@mhilbrunner
Copy link
Member

Cherry-picked to 4.1

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A problem about editor icons

7 participants