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

TrampleCrop Handler #868

Merged
merged 10 commits into from
Aug 31, 2024
Merged

TrampleCrop Handler #868

merged 10 commits into from
Aug 31, 2024

Conversation

xNatsuri
Copy link
Contributor

@xNatsuri xNatsuri commented May 2, 2024

This PR implements TrampleCrop handler to prevent players from trampling crops.

Copy link
Member

@Sandertv Sandertv left a comment

Choose a reason for hiding this comment

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

I think it might be more suitable to have a HandleCropTrample method in world.Handler. That way we don't have to duplicate this method for every entity (because ultimately, all entities can trample crops). That would mean calling this event in the EntityLand method of block.Farmland.

@xNatsuri
Copy link
Contributor Author

I was looking through issues when I saw this and I saw the discussion that you and Tal both thought it would be better to have it in a player/entity handler

@Sandertv
Copy link
Member

Good point, let me think about it a bit more.

Copy link
Member

@Sandertv Sandertv left a comment

Choose a reason for hiding this comment

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

I think it's fine to have this as a player handler. Got one more comment naming wise, but otherwise all is good once conflicts are resolved.

server/player/handler.go Outdated Show resolved Hide resolved
@xNatsuri
Copy link
Contributor Author

should be good to merge

@xNatsuri
Copy link
Contributor Author

added position of crop users might find it useful if they just want to cancel it in a certain area

Copy link
Member

@Sandertv Sandertv left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the PR!

@Sandertv Sandertv merged commit 06e5131 into df-mc:master Aug 31, 2024
1 check passed
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