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

Add anyblock support (eg: leaves and more). Block list configurable in the config file #34

Merged
merged 2 commits into from
Jan 9, 2022

Conversation

Oxina
Copy link
Contributor

@Oxina Oxina commented Jan 8, 2022

Hi,

This plugin is very usefull but in my case and for some other user (#17), we would like to have leaves support.

More than adding only leaves, this commit add the ability to define a list of blocks (the same way the exclusion works) in the config that will be processed by the resolver.

The code modifications are quite small and are implemented for all supported version of minecraft of the plugin.

I hope it can be added in the main plugin.

Have a nice day.
Best regards,
Oxina

@Lauriichan
Copy link
Member

In general this is a good idea for a change but I don't think that using a list with just a new WoodType would be the way to go implementing something like this. Instead a rework of the Wood Type system would be better in my opinion so that users can define their own type in a separate config.
@PierreSchwang What do you think?

@Oxina
Copy link
Contributor Author

Oxina commented Jan 8, 2022

I agree with you but, it would have required a lot more modification of the code.
I was trying to be as simple as possible so you can easily make change.
Not touching the WoodType would require to change the BlockBreakListener.

@PierreSchwang
Copy link
Collaborator

In general this is a good idea for a change but I don't think that using a list with just a new WoodType would be the way to go implementing something like this. Instead a rework of the Wood Type system would be better in my opinion so that users can define their own type in a separate config. @PierreSchwang What do you think?

I don't have a problem with the current approach. Should work just fine, and as there is a current recode in progress that is enough.

@Lauriichan Lauriichan self-requested a review January 8, 2022 23:43
Copy link
Member

@Lauriichan Lauriichan left a comment

Choose a reason for hiding this comment

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

It seems to be right.
Did you test it already?

EDIT: config safety is missing, we can't use items as falling blocks and I don't want to know what happens if we try to

@Lauriichan Lauriichan merged commit 047b198 into SourceWriters:legacy Jan 9, 2022
@Lauriichan
Copy link
Member

Thanks for your contribution @Oxina :)

@PierreSchwang
Copy link
Collaborator

Not a big fan of the static imports tho - But I guess that's okay as we don't have a style guide nevertheless.

@Oxina
Copy link
Contributor Author

Oxina commented Jan 9, 2022

I agree with you PierreSchwang. It should have been a normal import. Static imports can be confusing.

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.

3 participants