Skip to content

Conversation

ImMorpheus
Copy link
Contributor

Fix #1526

See #1977

I've updated the PR (most of those events are gone).

@ImMorpheus ImMorpheus added system: event type: bug Something isn't working api: 8 (u) version: 1.16 (unsupported since Oct 17th 2023) labels Jul 22, 2020
@Zidane
Copy link
Member

Zidane commented Jul 26, 2020

@ImMorpheus Put these on the lifecycle file exclusions please.

@dualspiral
Copy link
Contributor

How is @NoFactoryMethod different from an exclude directive in the Gradle file? If there is no difference, I'd rather we just add things to the exclude directive, especially as we want to move the factory generation to Common, reducing the exposure of the generator to the API itself.

@Zidane
Copy link
Member

Zidane commented Jul 26, 2020

@dualspiral That is a good point, didn't even think about that.

@ImMorpheus
Copy link
Contributor Author

How is @NoFactoryMethod different from an exclude directive in the Gradle file? If there is no difference, I'd rather we just add things to the exclude directive, especially as we want to move the factory generation to Common, reducing the exposure of the generator to the API itself.

The exclude directive targets a java file. The annotation targets a type.
This means that

public interface CollideEvent extends Event, Cancellable {
/**
* Fired after an {@link Entity} or {@link BlockSnapshot} impact with each
* other.
*
* <p>Note: this should only fire once after the first impact.</p>
*/
@NoFactoryMethod
interface Impact extends CollideEvent {

can't be implemented using the exclude directive AFAIK.

I'll move the other ones since this doesn't apply to them.

Copy link
Member

@gabizou gabizou left a comment

Choose a reason for hiding this comment

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

LGTM

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) system: event type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants