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

Allow plugin name in is_plugin_enabled #61820

Merged

Conversation

pfertyk
Copy link
Contributor

@pfertyk pfertyk commented Jun 8, 2022

Fixes #61604.

@KoBeWi KoBeWi added this to the 4.0 milestone Jun 8, 2022
@KoBeWi KoBeWi added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 8, 2022
@akien-mga
Copy link
Member

akien-mga commented Jun 9, 2022

There's also set_plugin_enabled which likely has the same issue. And the documentation is still incorrect as it now mentions only one of the supported formats, but not both.

IMO, it would be better if only the plugin name was supported (i.e. like the docs), the fully qualified path is not a useful format for this API.

Which means that #43734 should be reworked to make sure that it doesn't regress for the case it implemented.

@pfertyk
Copy link
Contributor Author

pfertyk commented Jun 9, 2022

I've changed the other method as well ;) For now, both support path and name. Let me know if I should change it to name only, or change the docs :)

I'm traveling now, but next week I'll have more time to take a look at #43734 to see if I've missed something ;)

@pfertyk pfertyk force-pushed the issue-61604-is-plugin-enabled-by-name branch from e6fcb78 to 725eb62 Compare June 9, 2022 16:45
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

This is fine to fix the regression for now but I think this still needs more work, it's a super weird API that requires passing a fully qualified path to the plugin.cfg, and the docs are not reflecting this well.

@akien-mga akien-mga merged commit ac9cfd0 into godotengine:master Jun 16, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

This doesn't seem relevant for 3.x which already had this issue fixed in #46389. For some reason this was not forward ported to master. @KoBeWi you mentioned the bug affected 3.5, can you check again?

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 16, 2022
@KoBeWi
Copy link
Member

KoBeWi commented Jun 16, 2022

Indeed, the issue doesn't happen in 3.5. I incorrectly assumed that it's relevant for this version, because I forgot it was fixed .-.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

is_plugin_enabled() not working as stated in the docs
3 participants