discord-php / DiscordPHP

An API to interact with the popular messaging app Discord
MIT License
985 stars 236 forks source link

MessageBuilder::_setFlags() is internal, but is needed for message flags #1244

Closed jdwx closed 1 month ago

jdwx commented 1 month ago

Environment

Describe the bug The method MessageBuilder::_setFlags() is flagged as "@internal" with the message, "You cannot set flags except for when sending webhooks or interaction. Use the APIs given." This is not correct. Setting flags is needed for other purposes, including sending "@silent" messages. (See https://discord.com/developers/docs/resources/channel#message-object-message-flags )

To Reproduce Steps to reproduce the behavior:

                $msg = ( new MessageBuilder() )->setContent( 'The bot is now online!' )->_setFlags( 1 << 12 );
                $channel?->sendMessage( $msg )->done();
    // This will correctly send the message as "@silent."
});

Expected behavior The interface should be modified to expose this functionality, either by promoting this method from _setFlags() to setFlags() and removing @internal, or (if I have understood the comment's reference to using the API) by exposing the ability to set SUPPRESS_NOTIFICATIONS via a separate public method, which would need to binary-OR the flags value.

jdwx commented 1 month ago

It looks like most of this has already been corrected on the master branch, this may just be a request to backport that into a released version, which would be a very small change.

valzargaming commented 1 month ago

v7.3.5 is currently broken in a couple critical ways. At this time, we're currently recommending upgrading to 10.0.0-RC7 as the changes that need to be made are many and not necessarily trivial.

jdwx commented 1 month ago

7.3.5 has worked pretty well for me, but I'll take your word for it and switch over!

Do you want a pull request to add methods for individual flags? E.g., setSuppressNotifications() and setSuppressEmbeds().

Based on the research I did on this, it looks like those might be the only two (and maybe EPHEMERAL?) that are meaningfully set on a new message. But I'm far from sure about that; I'm no expert, and have only tested and confirmed SUPPRESS_NOTIFICATIONS.

jdwx commented 1 month ago

Hmm, had to back down to 7.3.5 because messages had empty strings for content in 10.0.0-RC7. I'm sure there's a reason; I'll have to look into that when I have more time.

Log1x commented 1 month ago

RC7 is definitely pretty stable and is used by Laracord. As far as v7 goes, streaming a file over the voice client, removing roles, and a few other things are broken that I can recall.