Skip to content
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

fix: make Message#author lazy #204

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 33 additions & 35 deletions lib/discordrb/data/message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ class Message
alias_method :text, :content
alias_method :to_s, :content

# @return [Member, User] the user that sent this message. (Will be a {Member} most of the time, it should only be a
# {User} for old messages when the author has left the server since then)
attr_reader :author
alias_method :user, :author
alias_method :writer, :author

# @return [Channel] the channel in which this message was sent.
attr_reader :channel

Expand Down Expand Up @@ -88,34 +82,7 @@ def initialize(data, bot)

@server = @channel.server

@author = if data['author']
if data['author']['discriminator'] == ZERO_DISCRIM
# This is a webhook user! It would be pointless to try to resolve a member here, so we just create
# a User and return that instead.
Discordrb::LOGGER.debug("Webhook user: #{data['author']['id']}")
User.new(data['author'], @bot)
elsif @channel.private?
# Turn the message user into a recipient - we can't use the channel recipient
# directly because the bot may also send messages to the channel
Recipient.new(bot.user(data['author']['id'].to_i), @channel, bot)
else
member = @channel.server.member(data['author']['id'].to_i)

if member
member.update_data(data['member']) if data['member']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a very quick check, data['member'] doesn't seem to exist anymore so I ignored the code relating to it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data['member'] is present when messages are received via a gateway event

else
Discordrb::LOGGER.debug("Member with ID #{data['author']['id']} not cached (possibly left the server).")
member = if data['member']
member_data = data['author'].merge(data['member'])
Member.new(member_data, @server, bot)
else
@bot.ensure_user(data['author'])
end
end

member
end
end
@author_data = data['author']

@webhook_id = data['webhook_id'].to_i if data['webhook_id']

Expand Down Expand Up @@ -352,7 +319,7 @@ def delete_all_reactions

# The inspect method is overwritten to give more useful output
def inspect
"<Message content=\"#{@content}\" id=#{@id} timestamp=#{@timestamp} author=#{@author} channel=#{@channel}>"
"<Message content=\"#{@content}\" id=#{@id} timestamp=#{@timestamp} author=#{author} channel=#{@channel}>"
end

# @return [String] a URL that a user can use to navigate to this message in the client
Expand Down Expand Up @@ -390,5 +357,36 @@ def buttons

results.flatten.compact
end

# The user who sent this message message. Will be a {Member} most of the time, a {User} in the case
# that the user has left the server, or a {Recipient} if the message was sent in a private channel.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a type doc here

def author
author_id = @author_data['id'].to_i
return unless author_id
swarley marked this conversation as resolved.
Show resolved Hide resolved

if @author_data['discriminator'] == ZERO_DISCRIM
# This is a webhook user! It would be pointless to try to resolve a member here, so we just create
# a User and return that instead.
Discordrb::LOGGER.debug("Webhook user: #{author_id}")
User.new(data['author'], @bot)
elsif @channel.private?
# Turn the message user into a recipient - we can't use the channel recipient
# directly because the bot may also send messages to the channel
biqqles marked this conversation as resolved.
Show resolved Hide resolved
Recipient.new(@bot.user(author_id), @channel, @bot)
else
author = @channel.server.member(author_id) || @bot.user(author_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, we should use ensure_member/ensure_user

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

member is cached by default but made the other changes

case author
when Member
author.update_data(@author_data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we're handling this lazily now we should not update at this point in the lifecycle. There is no guarantee that our data is more up to date than the cache

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to ensure that we're updating/creating the member during the MessageCreate event processing though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what ensure_user is for?

when User
Discordrb::LOGGER.debug("Member with ID #{author_id} not available (probably left the server).")
swarley marked this conversation as resolved.
Show resolved Hide resolved
end
# @bot.ensure_user(@author_data) # unsure of point of this
author
end
end

alias_method :user, :author
alias_method :writer, :author
end
end