-
Notifications
You must be signed in to change notification settings - Fork 52
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
Support for custom enum name #25
Comments
A stupid thing with enums is that it's all global, plugins could have
colliding enum names and Commando intends to be used with public plugins as
well
#blameMojang
…On Wed, Apr 29, 2020, 15:46 Jack M. Taylor ***@***.***> wrote:
Currently commando would set a random enum name only if the enumName is
not set or is equal to "" or if two enums with the same name exist.
Ideally it should throw an exception if two enums have same name. Reason
being that those random letters look really weird and can confuse the
users. Plus servers which have their own plugins can easily cater for the
enum mess. So having the ability to set enum names on our own would be
greatly appreciated.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#25>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEHQQFD6XSPJTVLJAYLCMO3RO7LPDANCNFSM4MTPPTSA>
.
|
Same issue exists in BDS, registering a new enum with the same name will override it. I'd just stick with the vanilla behavior, thus i support Jacks idea |
In that case there should be guidelines on how to format the enum names to
prevent collisions in general use cases. This would help with plugin
developers in preventing unintended errors just because another plugin used
the same enums name.
Something like `abc.argName` or practically, `f.rank` for an enum with the
contents [Loner, Member, Captain, Leader]
…On Wed, Apr 29, 2020, 21:21 XenialDan ***@***.***> wrote:
Same issue exists in BDS, registering a new enum with the same name will
override it. I'd just stick with the vanilla behavior, thus i support Jacks
idea
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#25 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEHQQFCSCUMFDG7ASCPGZ4DRPASW5ANCNFSM4MTPPTSA>
.
|
Or register enums with a plugin-name prefix maybe? Similar to commands in pmmp with fallbackPrefix |
Wouldn't the plugin name be too long sometimes?
…On Thu, Apr 30, 2020, 06:41 XenialDan ***@***.***> wrote:
Or register enums with a plugin-name prefix maybe? Similar to commands in
pmmp with fallbackPrefix
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#25 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEHQQFFL2ZFYIH4RJBT5BW3RPCUJHANCNFSM4MTPPTSA>
.
|
How about if we make it mandatory to supply fallback prefix and then only
use it when two enum names collide?
…On Thu, Apr 30, 2020, 9:50 AM marshall ***@***.***> wrote:
Wouldn't the plugin name be too long sometimes?
On Thu, Apr 30, 2020, 06:41 XenialDan ***@***.***> wrote:
> Or register enums with a plugin-name prefix maybe? Similar to commands in
> pmmp with fallbackPrefix
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#25 (comment)>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AEHQQFFL2ZFYIH4RJBT5BW3RPCUJHANCNFSM4MTPPTSA
>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#25 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AH3QIR2ZDRBH4HTRYI2B63DRPD7RHANCNFSM4MTPPTSA>
.
|
Just do what jack suggested ffs lol |
Maybe only display the prefix to the client if the enum collides or actually don't display it at all because probably it will have only one unique enum in one command at a time |
Would be a BC-Breaking change but that would work... Just need some way to track what enum name is used and what enum name isn't used across virion instances. Virions are a complicated mess and when injected properly as intended they have shaded namespaces. A singleton tracking which enum names are used wouldn't cut it in this situation. |
An issue is because of how Virions are injected, the namespaces are changing. Therefore using a singleton to track the enum names that have been used would be ineffective on shaded builds. |
|
Currently commando would set a random enum name. How about it only sets it if the
enumName
is not set or is equal to""
or if two enums with the same name exist. Ideally it should throw an exception if two enums have same name. Reason being that those random letters look really weird and can confuse the users. Plus servers which have their own plugins can easily cater for the enum mess. So having the ability to set enum names on our own would be greatly appreciated.The text was updated successfully, but these errors were encountered: