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.

Sign up for GitHub

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

Jump to bottom

Added ICopper interface to reduce duplications #6390

Merged
merged 5 commits into from
Jul 10, 2024
Merged

Added ICopper interface to reduce duplications #6390

merged 5 commits into from
Jul 10, 2024

Conversation

ipad54
Copy link
Contributor

@ipad54 ipad54 commented Jul 8, 2024 •
edited
Loading

Introduction

This PR implements ICopper interface to reduce duplications and make further additions of copper blocks (such as #6366) easier.

Relevant issues

#6366

Changes

API changes

Added ICopper interface

jasonw4331
jasonw4331 suggested changes Jul 8, 2024
View reviewed changes
Copy link
Contributor

@jasonw4331 jasonw4331 left a comment

Choose a reason for hiding this comment

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

A rename of the interface should be done to remove the aliases everywhere.

src/block/utils/Copper.php Outdated Show resolved Hide resolved
@ipad54 ipad54 changed the title Added Copper interface to reduce duplications Added ICopper interface to reduce duplications Jul 9, 2024
ShockedPlot7560
ShockedPlot7560 approved these changes Jul 10, 2024
View reviewed changes
Copy link
Member

@ShockedPlot7560 ShockedPlot7560 left a comment

Choose a reason for hiding this comment

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

Looks OK to me

@jasonw4331 jasonw4331 added Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Jul 10, 2024
IvanCraft623
IvanCraft623 approved these changes Jul 10, 2024
View reviewed changes
Copy link
Member

@IvanCraft623 IvanCraft623 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@IvanCraft623 IvanCraft623 merged commit accaa0a into pmmp:minor-next Jul 10, 2024
13 checks passed
Joshy3282 added a commit to Joshy3282/PocketMine-MP that referenced this pull request Oct 5, 2024
@Joshy3282
Implement ICopper interface for blocks with common properties (pmmp#6390
0bd7ae2 
)
@dktapps
Copy link
Member

dktapps commented Oct 20, 2024

I don't much like the name of this interface. Are there no better options? It's not yet released (still stuck on minor-next) so there's time to improve it.

@ipad54
Copy link
Contributor Author

ipad54 commented Oct 20, 2024

Are there no better options?

There are some other naming options I came up with, but I doubt they are better than ICopper:

  • CopperInterface
  • CopperBase
  • CopperBlock
  • Cupreous

@dktapps
Copy link
Member

dktapps commented Oct 20, 2024

Maybe CopperMaterial or CopperLike, I'm not sure.

@ipad54
Copy link
Contributor Author

ipad54 commented Oct 22, 2024

CopperMaterial sounds good to me. I've created a branch in my fork repo with rename (ipad54@87a5337). If it looks good to you, I can open a PR.

@dktapps
Copy link
Member

dktapps commented Oct 22, 2024

Just make the PR so it can be reviewed here

@ipad54 ipad54 mentioned this pull request Oct 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonw4331 jasonw4331 jasonw4331 requested changes

@nicholass003 nicholass003 nicholass003 approved these changes

@ShockedPlot7560 ShockedPlot7560 ShockedPlot7560 approved these changes

@NhanAZ NhanAZ NhanAZ approved these changes

@IvanCraft623 IvanCraft623 IvanCraft623 approved these changes

Assignees
No one assigned
Labels
Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ipad54 @dktapps @jasonw4331 @IvanCraft623 @NhanAZ @ShockedPlot7560 @nicholass003