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

Inner class name conflict with other plugins #4

Closed
lihop opened this issue Jun 26, 2021 · 1 comment · Fixed by #5
Closed

Inner class name conflict with other plugins #4

lihop opened this issue Jun 26, 2021 · 1 comment · Fixed by #5
Labels
bug Something isn't working

Comments

@lihop
Copy link
Contributor

lihop commented Jun 26, 2021

After installing and enabling the ThreadPool plugin with the following plug.gd:

extends "res://addons/gd-plug/plug.gd"

func _plugging():
	plug("zmarcos/godothreadpool")

Future invocations of the plug.gd script fail with the error messages:

ERROR: get_global_class_path: Condition "!global_classes.has(p_class)" is true. Returned: String()
   At: core/script_language.cpp:239.
SCRIPT ERROR: GDScript::reload: Parse Error: Can't override name of the unique global class "ThreadPool". It already exists at: 
   At: res://addons/gd-plug/plug.gd:832.
ERROR: reload: Method failed. Returning: ERR_PARSE_ERROR
   At: modules/gdscript/gdscript.cpp:583.
SCRIPT ERROR: GDScript::reload: Parse Error: Script isn't fully loaded (cyclic preload?): res://addons/gd-plug/plug.gd
   At: res://plug.gd:1.
ERROR: reload: Method failed. Returning: ERR_PARSE_ERROR
   At: modules/gdscript/gdscript.cpp:583.

This is because the ThreadPool plugin declares the unique global class here:

class_name ThreadPool

which conflicts with gd-plug's inner class:

class ThreadPool extends Reference:

I can also see the potential for conflict with gd-plug's other inner classes. Logger with some logging related plugin, and (maybe less likely) GitExecutable with some git related plugin.

More widely, this is related to github proposal #1566 Implement namespaces to avoid collisions in third-party add-ons. A suggested workaround there is to add a prefix to class names.

Some ideas (assuming these inner classes are for private use only and not part of the pubilc gd-plug API):

  • Add a plugin-specific prefix like "GDPlug" to the class names:
    ThreadPool -> GDPlugThreadPool
    Logger -> GDPlugLogger
    GitExecutable -> GDPlugGitExecutable
    This is a little ugly but least-likely to conflict with other plugins.
  • Add an underscore prefix:
    ThreadPool -> _ThreadPool
    Logger -> _Logger
    GitExecutable -> _GitExecutable
    This marks the inner classes as private and keeps the pretty names. It shouldn't be too likely that these conflict with other plugins, but who knows. Perhaps another plugin developer might declare class_name _Logger or class_name _ThreadPool for whatever reason.
@imjp94
Copy link
Owner

imjp94 commented Jun 26, 2021

Now there's another reason for me to avoid class_name...

Personally, I prefer the approach of adding underscore prefix.
As it make sense even if the issue(Implement namespaces to avoid collisions in third-party add-ons #1566) is fixed in the future.
And as mentioned that this approach doesn't fully mitigate the conflict, but I think the chance is so low that it is neglectable.

Thanks for the bug report!

@imjp94 imjp94 added the bug Something isn't working label Jun 26, 2021
lihop added a commit to lihop/gd-plug that referenced this issue Jun 27, 2021
Fixes class name conflict with the [ThreadPool
plugin](https://github.com/zmarcos/godothreadpool) and reduces the
likelyhood of class name conflicts with other plugins.

The underscore prefix also indicates that these classes are "private"
(intended for internal use by gd-plug) and not part of the public
gd-plug API.

Closes imjp94#4
lihop added a commit to lihop/gd-plug that referenced this issue Jun 27, 2021
Fixes class name conflict with the [ThreadPool
plugin](https://github.com/zmarcos/godothreadpool) and reduces the
likelyhood of class name conflicts with other plugins.

The underscore prefix also indicates that these classes are "private"
(intended for internal use by gd-plug) and not part of the public
gd-plug API.

Closes imjp94#4
@imjp94 imjp94 closed this as completed in #5 Jun 29, 2021
imjp94 pushed a commit that referenced this issue Jun 29, 2021
Fixes class name conflict with the [ThreadPool
plugin](https://github.com/zmarcos/godothreadpool) and reduces the
likelyhood of class name conflicts with other plugins.

The underscore prefix also indicates that these classes are "private"
(intended for internal use by gd-plug) and not part of the public
gd-plug API.

Closes #4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants