Lachee / discord-rpc-csharp

C# custom implementation for Discord Rich Presence. Not deprecated and still available!
MIT License
560 stars 93 forks source link

[BUG] Presence doesn't show for others when there's a button with a 32 symbols long label #182

Open maximmax42 opened 2 years ago

maximmax42 commented 2 years ago

Describe the bug Title, basically.

To Reproduce

  1. Make any presence with a button that has a label label that's 32 symbols long, e.g. 12345678901234567890123456789012.
  2. Whole presence only appears for you, until you change a label to be 31 symbols long at max.

Expected behavior Presence should appear for everyone.

Desktop (please complete the following information):

Almighty-Shogun commented 2 years ago

Can't imagine a button label needing 32+ characters.. 😅. But I added it on my "Try to fix" list for my PR https://github.com/Lachee/discord-rpc-csharp/pull/177

maximmax42 commented 2 years ago

32 is the maximum library can allow anyway. I just don't know if 31 is the actual maximum, or there's a bug in the library.

Lachee commented 2 years ago

Unicode Shenanigans most likely. Discord's limit is 32 bytes, and if Unicode characters are used then multiple bytes maybe necessary to encode characters.

Almighty-Shogun commented 2 years ago

I just checked the code. 32 characters is indeed the maximum this library allows you to use for button labels. You can see it here

/// <summary>
/// Text shown on the button
/// <para>Max 32 bytes.</para>
/// </summary>
[JsonProperty("label")]
public string Label
{
    get { return _label; }
    set
    {
    if (!BaseRichPresence.ValidateString(value, out _label, 32, Encoding.UTF8))
        throw new StringOutOfRangeException(32);
    }
}
private string _label;

I think this is just a standard that is being used. But before I am gonna make changes like increasing the max characters I need to discuss it with @Lachee

maximmax42 commented 2 years ago

Unicode Shenanigans most likely. Discord's limit is 32 bytes, and if Unicode characters are used then multiple bytes maybe necessary to encode characters.

I'm certain that "12345678901234567890123456789012" is exactly 32 bytes, since it's not even Unicode, just ASCII, and it still doesn't work properly. Plus if it was more than 32, surely ValidateString would catch that?

Almighty-Shogun commented 2 years ago

12345678901234567890123456789012 is indeed 32 bytes. About the ValidateString method. It call within a method WithinLength that checks the bytes. The method returns true/false depending on the bytes. From what I can tell it should return true when the bytes are equal or lower then 32.

public static bool WithinLength(this string str, int bytes, Encoding encoding)
{
    return encoding.GetByteCount(str) <= bytes;
}
Almighty-Shogun commented 2 years ago

32 characters is the maximum limit. This is a thing with Discord. You can find it back in the Discord documentation

maximmax42 commented 2 years ago

32 characters is the maximum limit. This is a thing with Discord. You can find it back in the Discord documentation

That still doesn't answer the question what exactly causes whole presence to not show for everyone except the user if there's at least 1 button with 32 symbols long label. Is it discord being weird or is it the library screwing up somehow?

Almighty-Shogun commented 2 years ago

32 characters is the maximum limit. This is a thing with Discord. You can find it back in the Discord documentation

That still doesn't answer the question what exactly causes whole presence to not show for everyone except the user if there's at least 1 button with 32 symbols long label. Is it discord being weird or is it the library screwing up somehow?

I honestly can't give a answer on that without it being tested. It could be Discord that is doing some weird stuff, but it might also be the package. I'll will get this tested this week and will come back to this asap.

maximmax42 commented 2 years ago

I just now realized that I needed to check if the bug happens when using a different library (like discord-rich-presence for node.js or something) before opening this bug report, maybe it was a discord bug all along and I didn't need to open this. But now I can only test it tomorrow.

Almighty-Shogun commented 2 years ago

I just now realized that I needed to check if the bug happens when using a different library (like discord-rich-presence for node.js or something) before opening this bug report, maybe it was a discord bug all along and I didn't need to open this. But now I can only test it tomorrow.

When you have it tested, please comment back and mention me so I will get notified. I honestly think it is an issue with Discord, but I could be wrong there.

maximmax42 commented 2 years ago

@Almighty-Shogun Okay, yeah, false alarm, this is not library's fault, it's Discord's fault. Might be viable to either warn people about passing 32 byte long labels or just change the maximum allowed size to 31.

Almighty-Shogun commented 2 years ago

@Almighty-Shogun Okay, yeah, false alarm, this is not library's fault, it's Discord's fault. Might be viable to either warn people about passing 32 byte long labels or just change the maximum allowed size to 31.

Yeah I thought is was Discord fault. But it is weird that they say in their documentation you can use 32 characters, but then by doing it, it gives problems. I will check later on if there is a way to use 32 characters on a button label.