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

[Enhancement] Now allow for custom implementations of EventWaiter #59

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Shengaero
Copy link
Collaborator

@Shengaero Shengaero commented Mar 26, 2018

Pull Request

Pull Request Checklist

Please follow the following steps before opening this PR.

PRs that do not complete the checklist will be subject to denial for
missing information.

Pull Request Information

Check and fill in the blanks for all that apply:

  • My PR fixes a bug, error, or other issue with the library's codebase.
  • My PR is for the commons module of the JDA-Utilities library.
  • My PR creates a new module for the JDA-Utilities library: ______.

Description

Due to the recent EventWaiter requests (among them, asynchronous dispatch of EventListener#onEvent, concurrent-modification support, and others I am probably missing), there will now be a new interface as part of the commons module: IEventWaiter

This will allow for developers to have even the tiniest specifications they might desire at their disposal when dealing with the menus in the menu module, and other general specifics.

An example implementation in kotlin can be found here.

Please note that if you currently use EventWaiter this PR is 100% backwards compatible, and you should have no codebase changes required upon the release of next version.

@Shengaero Shengaero added Enhancement Modification of existing code-base to enhance quality and/or functionality. Module: Commons Related to the "commons" module. Version: 2.2 labels Mar 26, 2018
@Shengaero Shengaero added this to the Version 2.2 milestone Mar 26, 2018
@Shengaero Shengaero self-assigned this Mar 26, 2018
@Shengaero
Copy link
Collaborator Author

This newest commit should also address #60
For anyone who still encounters issues, you should consider implementing IEventWaiter

@schnapster
Copy link

Any help needed to move this PR along? Suffering from #60

@Shengaero
Copy link
Collaborator Author

@napstr if you absolutely need this PR right now, you can check it out via jitpack.

*/
public EventWaiter(boolean threadSafe)
{
this(Executors.newSingleThreadScheduledExecutor(), true, true);

Choose a reason for hiding this comment

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

Not using the local threadSafe for calling the overloaded constructor.

Suggested change
this(Executors.newSingleThreadScheduledExecutor(), true, true);
this(Executors.newSingleThreadScheduledExecutor(), true, threadSafe);

@Andre601
Copy link
Contributor

Just wanted to point out that the linked Gist is invalid (doesn't exist)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Modification of existing code-base to enhance quality and/or functionality. Module: Commons Related to the "commons" module. Version: 2.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants