Improve entity toString methods

[averen]

Pull Request Etiquette

• [x] I have checked the PRs for upcoming features/bug fixes.
• [x] I have read the contributing guidelines.

Changes

• [ ] Internal code
• [x] Library interface (affecting end-user code)
• [ ] Documentation
• [ ] Other: _____

Closes Issue: NaN

Description

Every channel previously overrode Object#toString individually to differentiate between the channel types. This PR moves this override to the AbstractChannelImpl interface and updates the format to ChannelType:Name(Id). This also has the side effect of adding this handling to StageChannel and VoiceChannel which used the default method.

[Chew]

StoreChannels are going to be removed soon anyway, you can probably use "SC", though I suppose that may delay the PR until they're removed?

[averen]

StoreChannels are going to be removed soon anyway, you can probably use "SC", though I suppose that may delay the PR until they're removed?

If they are in the process of being removed, I don't think it will be an issue to share the prefix in the meantime.

[DV8FromTheWorld]

Lets go with the actual class (TextChannel) instead of the type (TEXT). The type gets weird for threads and the class is more explicit and understood by users. Going with the class also means we can update other entities like Member and Guild to use the longer naming.

[DV8FromTheWorld]

I feel like we need to define a consistent format for these toString implementations. right now they are kinda.. all over the place. We need to define:

• Name
• what : defines
• what [...] means
• what (...) means

Basically, I'm wondering if we need a format we can follow. Something like: Name:[Type]:Name(Id / Other info)

So something like Message might become: Message(123515896521, User:DV8FromTheWorld(2321892513583), Length trimmed messa)

PermissionOverride: PermissionOverride[Member](1232185621822, 156218362189857)

I'm actually not sold on the above format, but I do think we should have a discussion about what our format should be since we're touching everything here.

[averen]

I agree that the implementations are kind of a mess. I tried my best to follow somewhat of a pattern while retaining the original meaning in my first pass.

• I think it would be best to use : for IDs or "internal string" separators wherever possible.
• We could then use (...) to hold primary values like parent objects or user-provided content.
• And finally, the square brackets [...] could be used for an enum or type, as well as secondary values.

I would also argue to avoid using commas, slashes, and spaces wherever possible unless the string is representing a collection.

While reusing your examples, I was thinking something like: ClassName:[Type]:Id:ChannelId(Username)[Other info]

Where message would become: Message:2321892513583(User:2321892513583(DV8FromTheWorld))[Length trimmed messa]

And PermissionOverride would be: PermissionOverride:[Member]:1232185621822:156218362189857 I'm not completely sold on the colons here between the brackets either.

Lastly, Member could look like: Member:1232185621822:156218362189857(effectiveName)

[DV8FromTheWorld]

Honestly, the strings start getting longer than what I'd like to see at that point. I'm not quite sure what the answer here is yet.

[averen]

To be fair, we haven't added any information that wasn't already available in the current toString implementations, we've only updated the formatting so far. If they are getting too long, maybe we will need to consolidate which information is included.

[DV8FromTheWorld]

Snowflakes

• Current Implementations: - "User(" + getId() + ')'; - "Guild:" + getName() + '(' + id + ')'; - "Member:" + getEffectiveName() + '[' + getUser() + "](" + getGuild() + ')'; - "Role:" + getName() + '(' + id + ')'; - "Emote:" + getName() + '(' + getIdLong() + ')'; - "Emoji:" + name + '(' + id + ')' - "ReactionEmote:" + getName() + '(' + getId() + ')' - "Widget:" + (isAvailable() ? getName() : "") + '(' + id + ')'; - "Webhook:" + getName() + '(' + id + ')';

• Possible Format: Class:name(id, internalInfo)

  • Changed Implementations
    • "User:' + getName() +"(" + getId() + ')';
    • "Member:" + getEffectiveName() + '(' + getUser() + ", " + getGuild() + ')';
      • This one technically violates the pattern as the (...) doesn't start with the id. It uses getUser to provide the Id. Is that a problem?

Channels

• Current Implementations: - "Category:" + getName() + '(' + getId() + ')'; - "TextChannel:" + getName() + '(' + getId() + ')'; - "ThreadChannel:" + getName() + '(' + getId() + ')'; - "NewsChannel:" + getName() + '(' + getId() + ')'; - "PermissionOverride:" + (isMemberOverride() ? "MEMBER" : "ROLE") + ':' + channel.getId() + '(' + getId() + ')'; - "PrivateChannel:" + getName() + '(' + getId() + ')'; - "StageChannel:" + getName() + '(' + getId() + ')'; - "StoreChannel:" + getName() + '(' + getId() + ')'; - "VoiceChannel:" + getName() + '(' + getId() + ')';

• Possible Format: Class[type]:name(id, internalInfo)

  • Changed Implementations
    • "PermissionOverride[" + (isMemberOverride() ? "Member" : "Role") + "](" getId() + ", " + channel + ')';

Messages

• Current Implementations:

  • String.format("Message:%s:%#s(%.20s)", getId(), author, this) : String.format("Message(%.20s)", this)
  • "SystemMessage[" + type + ']' + author + '(' + id + ')';

• Possible Format: Class[type](id, author, partialMessage)

  • Changed Implementations
    • String.format("Message(%s, %#s, \"%.20s ...\")", getId(), author, this)
    • String.format("DataMessage('%.20s ...')", this)
    • "SystemMessage[" + type + "](" + id + ", " + author + ')';

Events

• Current Implementations:

  • GenericGuildMemberUpdateEvent[" + getPropertyIdentifier() + "](" + getOldValue() + "->" + getNewValue() + ')'
  • "RoleUpdate[" + getPropertyIdentifier() + "](" + getOldValue() + "->" + getNewValue() + ')'

• Possible Format: Class[propIdentifer](oldVal -> newVal)

  • Changed Implementations
    • None from this PR. Maybe other events.

Initial Conclusion

So far, it looks like a pattern of Class[Type]:name(id, additionalInfo, ...) seems to work fairly well

[DV8FromTheWorld]

The Rest - I've not edited any of these yet.

I pulled them straight from the code in the PR. Just pasting in one place to simplify visualizing them

Constructs

• "ShardInfo " + getShardString();
• "Command:" + getName() + '(' + getId() + ')';
• "Choice:" + name + '(' + stringValue + ')';
• "Option[" + getType() + "](" + name + ')'; (Command.Option)
• "Option[" + getType() + "](" + getName() + '=' + getAsString() + ')'; (OptionMapping)
• "Subcommand(" + name + ')';
• "SubcommandGroup(" + name + ')';
• String.format("Activity[%s](%s)", name, url) : String.format("Activity(%s)", name)
• "VanityInvite(" + code + ')'
• "VoiceState:" + getGuild().getName() + '(' + getId() + ')';
• "RichPresence[" + name + "](" + applicationId + ')'
• "MessageReaction:" + messageId + '(' + emote + ')';

Utility

• "ConnectionRequest[" + stage + "](" + Long.toUnsignedString(guildId) + '#' + Long.toUnsignedString(channelId) + ')';
• "CompiledRoute[" + method + "](" + compiledRoute + ')';
• "Result(success=" + value + ')' : "Result(error=" + error + ')';

Unsure how to classify these yet

• "Widget.Member:" + getName() + '(' + id + ')';
• "Widget.VoiceChannel:" + getName() + '(' + id + ')';
• "Widget.VoiceState:" + widget.getName() + '(' + member.getEffectiveName() + ')';
• "ApplicationInfo(" + this.id + ')';
• "RoleTags(bot=" + getBotId() + ",integration=" + getIntegrationId() + ",boost=" + isBoost() + ')';
• "TeamMember:" + getTeamId() + '(' + user + ')';
• "Button:" + label + '(' + id + ')';

[MinnDevelopment]

What is the status on this?

[averen]

I haven't had the time to look into this too closely, but I will try to get this going again this week.