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

feat(state): clone User on GuildMemberUpdate #1320

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryanblenis
Copy link

@ryanblenis ryanblenis commented Jan 30, 2023

Properly reference previous user when user data is changed and GuildMemberUpdate is fired with TrackMembers enabled

Minimal implementation for obtaining before/after snapshot of user changes. See additional information and items for consideration in #1318 if a different implementation path is preferred.

Currently the User changed data is lost as the BeforeUpdate Member copies the pointer to the associated user, leaving both the current Member and BeforeUpdate Member to reference the now updated user properties.

Thank you.

…emberUpdate is fired with TrackMembers enabled
@FedorLap2006 FedorLap2006 changed the title Properly reference previous user when user data is changed and GuildM… feat(state): clone User on GuildMemberUpdate Mar 9, 2023
Copy link
Collaborator

@FedorLap2006 FedorLap2006 left a comment

Choose a reason for hiding this comment

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

Sorry for a late response. I also probably was too quick to approve this. But anyway.

After investigating this myself, I haven't exactly found where the copy is happening into the User.

*m = *member

In memberAdd (linked above) we only replace the member object. And since we're in OnInterface - we should receive a new User. Thus only the pointer should be replaced, and in the copy it would stay as it was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants