Spigot 1.13, Java 1.8

[Jikoo]

• Fixed double chests/other containers
  • All multiblock structures (double chests, beds, etc.) will attempt to heal both blocks at once. If a single block fails, the whole heal fails.
  • Future Bisected additions will be considered multiblocks by default, but may need to be excluded the way stairs and slabs are to properly heal block-by-block.
• Added support for most new blocks
  • Missing coral and some other things - still have yet to revisit the concept of NeighborBlocks entirely, which should handle cases in a more generic fashion.
• Fixed a lot of potential future issues associated with redundant explosion replacement entries for "dependent" multiblocks - mostly ignoring more duplicate entries in CreeperExplosion
• Changed configs to use Material names. This WILL break existing configs using IDs. Not sure how well configs work at all, have yet to test. Not a priority for me, there are a dozen claim/anti-grief plugins out there.

Currently not ready to be pulled, input welcome.

TODO:

• [x] Split up PR into multiple commits
• [ ] Revisit Neighbor blocks
• [x] Fix formatting
• [ ] Configurations
  • [ ] Test, make sure materials work properly
  • [x] Investigate converter
• [ ] Remove local dependencies

I also did not bump the version, figured that'd be up to you to decide.

[nitnelave]

Wow, thanks for the PR! I'll try to have a look this weekend :)

In general, though, it's much easier to get PRs accepted if you decrease the scope: 5 small PRs get in faster than one huuuuge PR, and issues/bugs can be identified and addressed more easily.

That is especially true if you decide to make breaking changes ;)

[nitnelave]

I had a very quick look, and it would really help if you did at least the following splits:

• Formatting corrections (trailing whitespace, line wrapping, etc...)
• Dependency changes (removing the lib/ folder, changing the pom.xml file)
• Ideally, each main feature as a separate change (multiblock, new blocks, redundancy, ...)
• Breaking changes: we have to consider each of them separately, and separately from the rest of the changes that are just clear improvements.

If you need help with that, I'll try my best to help!

[Jikoo]

• Formatting corrections (trailing whitespace, line wrapping, etc...)

Definitely on my list, the trailing whitespace removal was a mistake I thought I'd corrected. I also need to go back and redo the formatting to match your style.

• Dependency changes (removing the lib/ folder, changing the pom.xml file)

If that's how you want to go, can do. Didn't want to mess with the setup you have.

• Ideally, each main feature as a separate change (multiblock, new blocks, redundancy, ...)

Most multiblocks are tied to BlockData, new in 1.13. I can definitely move some of the simplifications to other methods into a separate commit.It should also be possible to split off the chest/doublechest section of it if you want a point where you can release a last pre-1.13 build that should fix chests backwards to around 1.8/1.9 (I forget when Inventory#getLocation was added).

• Breaking changes: we have to consider each of them separately, and separately from the rest of the changes that are just clear improvements.

I believe the only breaking change is to config values that accept material IDs, and that could be fixed with a converter, I just don't really want to write one. A quick search hasn't yielded anything that fits the need, but I'm sure someone's put one out there somewhere. Beyond that, the 1.13 update really should be done without backwards compatibility, depending on the Spigot's compatibility layer can only do so much. I imagine it'll get worse in future updates. Minecraft and Spigot have been Java 8 since 1.12, there just wasn't a good reason to break backwards compatibility with older servers when it wasn't required.

[nitnelave]

So, what we can do is the following:

• A series of patches addressing as much as possible in the old environment. That means Java 6, no BlockData, no breaking changes.
• A bump in major version, with Java 8 conversion, BlockData, Material ID changes, dropping Factions support (we'll have to see about that), etc.

For the configuration, I'd really like a silent converter to new style (i.e. if the old field is present, convert it to the new field and drop the old). It doesn't even have to be efficient, you can iterate through all the materials for each value until you find the matching one, it's just a one-time conversion process anyway.

[Jikoo]

Okay, I fiddled things around a bit, currently should be offering a much nicer diff for pre-1.13 changes.

• Deleted PlayerHeads, it's unused. Is Lockette used at all? I saw mention of it, but no actual usage.
• Updated some block types to use the new Multiblock. Didn't touch pistons or beds, I don't remember them being problematic.
• Reduced redundant code (or lack thereof) in CreeperExplosion's methods for checking exploded blocks, which was the source of the duplication issues for some multiblocks
• Changed plugin.yml to just use the maven version, why update more places than you have to :p
• Did not remove block types that were not necessary due to how BlockStates work - I assume they used to be required, and I don't want to bother if there's a chance it'll break backwards compatibility early

[Jikoo]

Okay, we're about where I was when I opened the one massive PR. TODO list is updated.

Found a bug with advanced option drop-destroyed-blocks: false that appears to have been present since 2013 - drops are always done for whatever block happens to exist in the spot being healed, even if the block is not being overwritten. I'll open a separate PR for that, it should be fixed before 1.13 and I don't want to have to cherry pick each commit again.

Edit: Yeah, the bug is pretty bad, tons of dupes. My fix mitigates some of it, but container-related dupes are even worse. I'll debug tomorrow.

[nitnelave]

Thanks for your work! But again, small PRs make happy maintainers :D

[Jikoo]

But again, small PRs make happy maintainers :D

1.13 and Java 8 is never going to be small. I've broken it up a little in separate commits, but that's about the best I can do.

I'm going to open a separate pull for pre-1.13 due to how interdependent the bug fixes are with my changes to how certain blockstates are handled, but that's it. I've already spent way too long reconsidering the original diff when splitting it up, I'm not doing it all again over even more content. Completely understand if you don't want to read it all, it's a lot. At least this way it's out there if someone else wants to take a crack at it.

[nitnelave]

But again, small PRs make happy maintainers :D

1.13 and Java 8 is never going to be small. I've broken it up a little in separate commits, but that's about the best I can do.

A small PR doesn't necessarily mean few lines changed (although that usually helps), but it can also mean single-purpose. In more mature projects, it's common to have a PR reference a single bug, and address that single bug. Bigger features are implemented in several PR; for instance, you could introduce the new data structures and populate them, implement the algorithm with tests, and then integrate with the main data flow: the first 2 PRs do not affect the control flow of the program and are easily tested on their own, and the last one is usually trivial.

I'm going to open a separate pull for pre-1.13 due to how interdependent the bug fixes are with my changes to how certain blockstates are handled, but that's it. I've already spent way too long reconsidering the original diff when splitting it up, I'm not doing it all again over even more content. Completely understand if you don't want to read it all, it's a lot. At least this way it's out there if someone else wants to take a crack at it.

However, I understand that this is a minecraft plugin, not an operating system :D We're all volunteers here, I'm just trying to help you improve. I'll try to have a look at the PR when I get the time. Thanks again for your efforts :)

[hafarooki]

MC itself actually requires Java 8 now, only bukkit/spigot API (not server) doesn't.

[Jikoo]

Thanks for your comments! I've been over all of them and marked duplicates as resolved for readability. Any issues now should be acknowledged or explained, let me know how you want me to proceed.

[Jikoo]

Actually, were you not aware that Java 8 is required for MC now? In that case, I apologize for my previous attitude, which was getting to be brusque at best. We've been looking at this through completely different eyes for a while. To me, this PR really was for only 1 upgrade which turned into 2 bugfixes and an upgrade (still need to backport the drop fixes, will get to that). You've been seeing it as 2 separate MAJOR changes along with some minor stuff this whole time.

[nitnelave]

No, I wasn't aware, I haven't really touched any plugin for at least 5 years now. I'd still like to keep Java 6 for 1.12, and then we can update in the next PR.

[hafarooki]

I think an external IM chat platform, such as Slack or Discord, might be more convenient for this :stuck_out_tongue:

[Jikoo]

I'd still like to keep Java 6 for 1.12, and then we can update in the next PR.

I was planning on keeping Java 6 till 1.13, that was the idea behind 8dca5d7 preceding the other commits. I'll open a separate PR for that, we can discuss it, and then I'll open another PR cherry picking relevant sections of f3b468b in.

When that's out of the way we can get back into this mess, which hopefully I'll have somewhat addressed.

I think an external IM chat platform, such as Slack or Discord, might be more convenient for this 😛

I work 10+ hours a day, my limited availability probably actually make this easier. No matter how you slice it I end up with a lot of reading to which I get to compose a bulky reply. Here at least we can discuss individual diffs as separate threads, IM would make responding to something specific hours later a nightmare for readability.

[hafarooki]

I'm not sure I see the purpose in not updating to Java 6. It's just a change in version number in the build script.

[nitnelave]

@Miclebrick The point is to separate the changes to make sure you don't break anything. By moving in small steps, you can make sure you move safely. Ideally, we would have plenty of tests to make sure that you don't break anything, but that would require a major reorganization in the plugin :)

[hafarooki]

I didn't mean in this PR, I just meant if you're fixing bugs in 1.12 anyway, might as well take advantage of Java 8. Not in this PR, in separate PRs you were talking about making for 1.12 bugfixes.

[nitnelave]

Yes, I'm all for updating to Java 8 if it's required by Minecraft. I just want to give the latest bugfixes for those who might still be in Java 6 and don't want to update, and then do the migration.

[SaganWolfric]

I would like to know when a beta version will be released... I, as well as many others, am in dire need of having Creeper Heal installed. It might be interesting to set up a Jenkins server.

[Jikoo]

I would like to know when a beta version will be released... I, as well as many others, am in dire need of having Creeper Heal installed. It might be interesting to set up a Jenkins server.

I created a temporary release on my fork about a week ago after a couple requests. Use at your own risk, still needs a couple more fixes. It will for sure fix issues with double chests and the age-old dupe bug with certain advanced config options, but likely does not properly heal entities (whoops). As for an official release timeframe, this particular PR is backburnered until I can get through all of the other changes pre-1.13. Unfortunately, my free time has been drastically reduced recently, so progress has been particularly slow.

[nitnelave]

I would like to know when a beta version will be released... I, as well as many others, am in dire need of having Creeper Heal installed. It might be interesting to set up a Jenkins server.

Hi! We're still working on this PR, but keep in mind that we're doing that in our free time while juggling work and family life on the side. We'll release a new version when it's ready, but if you're really pressed for time, you can always grab the latest commit from the PR and build the plugin yourself.

[VMBindraban]

Maybe a radical idea but is it an idea to make separate versions for 1.13 and 1.12 (and below)? I have seen this for work for multiple plugins.

[Jikoo]

Not a fan of sacrificing modern potential for ancient versions. I'm very much aware of how hypocritical that sounds coming from the guy who reintroduced backwards compatibility to OpenInv, but it's not worth the hassle of making sure everything works properly on outdated versions.

Considering that no one else has stepped up to fix CH's multiple issues over the years they've been present for, you're probably going to be hard pressed to find anyone willing to maintain multiple versions of it.

Edit: In case you're not aware, the plan is to fix all the bugs I'm aware of prior to the 1.13 update so that any users interested in running outdated versions are free to do so.

[nitnelave]

Okay, since the other PR has been accepted, we can start working on this one again :)

[Jikoo]

we can start working on this one again

I wish ;-; Need to cherry pick f3b468b to 1.11, then fix armor stands not regenerating with armor.

[Jikoo]

I'm sorry, but I am not going to rework this pull into a satisfactory state. I'm only just getting back to CreeperHeal for a couple bugfixes on 1.15, and I do not have the motivation to delve into finalizing everything on a version so old - it's been nearly 2 years since 1.13's release.