Update to 1.12

[ghost]

Hey

On latest version (Pre version released from Spigot) I'm getting http://paste.yu8.me/?5c7a326696ea001f#oIaBcdd04kVOAQjlamMFTVcgPN3YWizSz0c767iewNk=

Just giving you a heads up before 1.12 is released

[Jikoo]

Thanks for the heads up, though I probably won't officially update OpenInv to support a prerelease. Might push out a build with temporary support here, but based on how the Spigot team has been handling NMS revisions (not bumping when changes are made) I don't want to have to create too much of a mess in the official versions.

[Jikoo]

All right, here's OpenInv v3.1.3-SNAPSHOT with 1.12-pre2 support, just rename the file. I have not tested it and don't really intend to until an official 1.12 release.

Edit: We'll consider this the official "Hey, update to 1.12" issue, so I'll leave it open. If I'm silent or you happen to test it on an official release before I get to it, feel free to poke me to make a release.

[TNTUP]

Hi @Jikoo the plugin works fine in 1.12-pre5 (Guessing Spigot considers this as 1.12)

[Jikoo]

Yeah, the NMS version is 1_12_R1, same as it will end up being on official release.

Spigot has, since midway through 1.8, opted not to bother changing NMS revisions (which for a pre-release, I'm fine with) - 1.11.2 really should have been 1_11_R2, but instead authors got stuck with the complications that the Bukkit team was seeking to avoid by introducing versioned packages in the first place. It's such a pain to track down, especially when a method is renamed and another with the same signature takes its place.

For that reason, I don't want to officially push out a release without an official 1.12 version. If Mojang does end up making additional changes I don't want to have to create a mess for versions almost no one will be using in another month. Speaking of supporting things no one uses, I should probably consider dropping support for versions < 1.7.10 in official builds, but that's an issue for another ticket.

[smmmadden]

Confirmed that the snapshot build 3.1.3 is working with Spigot Pre5 builds. Previously, the 3.1.2 version not only failed, it forced a server shutdown. I was able to qualify this be removing each failing plugin until I removed and re-added it back in to see it disable OpenInv, then shutdown happens.

If you didn't make any changes to fail more gracefully, I'd recommend adding a catch for it. :-)

[Jikoo]

Previously, the 3.1.2 version not only failed, it forced a server shutdown.

Could you provide logs? OpenInv should be disabling itself if no matching NMS version is found.

[smmmadden]

2017-05-24-1.log.gz

What happens after it gets to the last line is press any key to continue.... The error in the OpenInv doesn't look bad, actually looks better than other exceptions, but somehow it did in fact stop the spigot server.

[Jikoo]

Odd, based on that log I'd blame ChestShop. If you care to and can replicate it without other plugins I'll worry about it, otherwise we'll shrug and move on. All OpenInv does is disable itself.

[smmmadden]

I thought so too at first since ChestShop was last in the list, but without chestshop even included the results are the same with OpenInv 3.1.2. All the plugins that were failing I removed each and tried one at a time to narrow it down to only this (unfortunately). :-)

[ghost]

ChestShop is the one crashing/not letting you start the server. I had this problem myself, and worked fine when I removed it.

[smmmadden]

I disagree. I tested this 3 times with and without chestshop plugin and with and without openinv. It was only when OpenInv 3.1.2 was added, that the server shut down. At this point its mute since it works in 3.1.3. I was only recommending that there is a catch to prevent this scenario (replicated by me or anyone).

[Jikoo]

I can't reproduce killing the server using 1.11.2 and a build of OpenInv that only supports 1.10. It's likely either your hardware, the server software itself, another plugin, or just that OpenInv needed to be re-downloaded. OpenInv appears to be working as intending, the server loads fine for me. Whichever issue it is, there is nothing to catch, because it's (unfortunately for you, fortunately for me) not an issue with OpenInv's code. If you do try to replicate the crash, please do so with OpenInv as the only plugin, not a mix of all of your other plugins sans one.

[TNTUP]

I don't use ChestShop, never experienced an OpenINV server crash (I'm on QuickShop haha) XD

[smmmadden]

Guys, you're missing the point. The likelyhood of you actually testing using the same combinations of plugins, builds, settings and O/S is very low. I provided the full log, showing clearly OpenInv 3.1.2 on Spigot 1.12 build (not 1.10 or 1.11.2, but 1.12 pre5 latest build) running on Windows 10 Pro x64 and Java 1.8.0_131, the server crashes when OpenInv is included. It has nothing to do with ChestShop even though it was loaded and didn't crash or have any indication of a problem during startup. I too am not using ChestShop as I now use QuickShop as well, but simply testing the validity of the plugins during startup of new Spigot builds. So the point again is adding a catch (ya know, Java "catch" condition) that makes sure if the plugin fails, it doesn't cause a cascading shutdown of the server it runs on. That's all this request was for. Sometimes requests are very simple and don't need a lot of digging into. Rare, yes, but it happens. :-)

[Kakifrucht]

You are not supposed to "catch" seemingly random exceptions just for the sake of it. The issue must be reproduced and fixed properly.

[Jikoo]

So the point again is adding a catch (ya know, Java "catch" condition) that makes sure if the plugin fails, it doesn't cause a cascading shutdown of the server it runs on.

I think you missed my point last time - OpenInv does catch the exceptions related to not having a valid NMS adapter, which is why its logs are so pretty when it disables itself. Your logs show that there is no exception thrown, the thread just dies. It also does not die during any OpenInv code execution. As this cannot be reproduced using only OpenInv on a release version, my conclusion is that it's a server problem, be it the server software, hardware, or the system the server is running on.

[smmmadden]

just close the request

[Jikoo]

just close the request

The issue is open because 1.12 is not officially released. When it is, if modifications need to be made, I will make them and drop any support for the prereleases. I'm not closing this issue until 1.12 comes out, nor am I planning on tagging a release until then.

[smmmadden]

that works for me too - just didn't want to waste more time on the topic :-)

[Jikoo]

1.12 is officially released, so I'll be checking over the implementation tonight when I get out of work. Assuming no complications, I should be able to make a release within 13 hours or so.

[TNTUP]

@Jikoo No rush haha, anyways 1.12 is quite too "premature" to be in a production state (my server/community) so no rush for me but I guess for others who are eager to switch to 1.12 will be nice XD

[SXRWahrheit]

Looking forward to the release!

[Jikoo]

3.2.0