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

Create a SubjectProxy and separate CommandCause from CommandContext #2191

Merged
merged 1 commit into from Jul 27, 2020

Conversation

dualspiral
Copy link
Contributor

Basic premise is that CommandContext and CommandContext.Builder no longer implement CommandCause.

I basically shouldn't have done this anyway, but the idea was that a CommandContext could functionally act as a CommandCause as a convenience to them. However, in implementation, there is an assumption that a CommandSource is a CommandCause, which is true. The opposite isn't necessarily true, however, and I've bumped up places where I've made this bad assupmption.

What I have opted to do is stop CommandContext and its builder from implementing CommandCause, but to continue helping plugin devs, it now implements the new SubjectProxy interface and continues to have the sendMessage(Component) method. So, if you need a CommandCause you have to explicity get it, but if you wanted to do a permission check and/or send a message (two very common things), this can be done via the context.

I do want to merge this soon because I'm working on Selectors and that does rely on this, but I'm using the PR as a sanity check for style and such.

This actually doesn't change much, but it was actually a problem in our implementation. It makes sense to keep it separate, and all the standard tasks can still be done in the same way (the context is now a SubjectProxy and you can easily send a message from it)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: 8 (u) version: 1.16 (unsupported since Oct 17th 2023)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant