CortexPE / Commando

A Command Framework virion for PocketMine-MP
GNU Lesser General Public License v3.0
70 stars 53 forks source link

Support for custom enum name #25

Open Ifera opened 4 years ago

Ifera commented 4 years ago

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.

CortexPE commented 4 years ago

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 notifications@github.com 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 https://github.com/CortexPE/Commando/issues/25, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEHQQFD6XSPJTVLJAYLCMO3RO7LPDANCNFSM4MTPPTSA .

inxomnyaa commented 4 years ago

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

CortexPE commented 4 years ago

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 notifications@github.com 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 https://github.com/CortexPE/Commando/issues/25#issuecomment-621199179, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEHQQFCSCUMFDG7ASCPGZ4DRPASW5ANCNFSM4MTPPTSA .

inxomnyaa commented 4 years ago

Or register enums with a plugin-name prefix maybe? Similar to commands in pmmp with fallbackPrefix

CortexPE commented 4 years ago

Wouldn't the plugin name be too long sometimes?

On Thu, Apr 30, 2020, 06:41 XenialDan notifications@github.com 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 https://github.com/CortexPE/Commando/issues/25#issuecomment-621505394, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEHQQFFL2ZFYIH4RJBT5BW3RPCUJHANCNFSM4MTPPTSA .

Ifera commented 4 years ago

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 notifications@github.com wrote:

Wouldn't the plugin name be too long sometimes?

On Thu, Apr 30, 2020, 06:41 XenialDan notifications@github.com 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 https://github.com/CortexPE/Commando/issues/25#issuecomment-621505394, 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 https://github.com/CortexPE/Commando/issues/25#issuecomment-621610779, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH3QIR2ZDRBH4HTRYI2B63DRPD7RHANCNFSM4MTPPTSA .

lukeeey commented 4 years ago

Just do what jack suggested ffs lol

inxomnyaa commented 4 years ago

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

CortexPE commented 4 years ago

How about if we make it mandatory to supply fallback prefix and then only use it when two enum names collide?

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.

CortexPE commented 4 years ago

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.

CortexPE commented 4 years ago

sAvE iT iNtO a FiLe InStEaD aNd ThEn ChEcK tHaT