MockBukkit
MockBukkit copied to clipboard
Add BlockDataMock support for TrapDoor
Description
Add BlockDataMock
support for TrapDoor
. Also:
- moved
BlockData
String keys into consts classBlockDataKey
to reduce duplication - refactored
BlockDataMock.mock()
since existing implementation would get rather large if moreTag
-based subclasses are added
Checklist
The following items should be checked before the pull request can be merged.
- [x] Unit tests added.
- [ ] Code follows existing code format.
Info on creating a pull request
- Make sure that unit tests are added which test the relevant changes.
- Make sure that the changes follow the existing code format.
For maintainer
When a PR is approved, the maintainer should label this PR with release/*
.
- If the PR fixes a bug in MockBukkit, update the patch version. (if the current version is
0.4.1
, the new version should be0.4.2
) - If the PR adds a new feature to MockBukkit, update minor version. (if the current version is
0.4.1
, the new version should be0.5.0
)
Note that a PR that fixes an UnimplementedOperationException
should be considered a new version and not a bugfix.
Please fix the Codesmells and try to deduplicate the Code if possible
Please fix the Codesmells and try to deduplicate the Code if possible
Can do
I've been thinking about how to deduplicate the code and I'm not sure there's a way of doing so without rewriting BlockDataMock
. Here's why I think that:
Sonar is complaining about how getHalf()
/setHalf()
/getFacing()/setFacing()
/getFaces()
are duplicated in TrapDoorMock
. If we wanted to put the BlockFace
methods in a separate class, we could have an abstract class DirectionalMock
that extends BlockDataMock
and implements Directional
(though getFaces()
will have to differ for different subclasses). StairsMock
, for example, could then extend DirectionalMock
and that would reduce the duplication of those methods. However, when we also try dedupe the Half
methods, we'd need something like BisectedMock
, and we'd run into a multiple inheritance problem.
We could rewrite BlockDataMock
to be a subinterface of BlockData
and then allow for "mixing in" of Directional
/Bisected
/Waterlogged
/etc methods, but I don't think I'm comfortable (nor do I think y'all would be) doing that having just started looking at the source code last week.
What are your thoughts? Is the duplication something you'd like for me to take a deeper look into?
(As for the code smells, I don't think there should be a problem fixing those.)
It's something that would be nice to have done, but not a huge priority. If you're interested in it you can take a shot at it, but it's not required to keep adding new block data.
I addressed the smells. Also, I added jacocoTestReport
to the test
task, as I don't think it was running on test
automatically.
It's something that would be nice to have done, but not a huge priority. If you're interested in it you can take a shot at it, but it's not required to keep adding new block data.
Let me sit and think on that. I might be interested in trying my hand at it, but as of right now I don't have any ideas.
Kudos, SonarCloud Quality Gate passed!
0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells
No Coverage information
1.7% Duplication
LGTM. Thank you!
And thank you as well! This project wouldn't be possible without you two and I really appreciate your thoroughness and patience 😀