Bukkit

@mwkaicz

Yup. Got it, merged it, looks good. I did some basic tests and it no longer lets me move my ship in such a way that it would result in a chest on my ship merging with a chest not on my ship.

Updated beta for others to test. Same link as before.

In other news, I'm 90% done with my torpedo tutorial, which I have turned into an overall combat tutorial.

@AJCStriker

I made request to Loraxe repository few minutes ago. I tested all possible combinations of translation and rotation what I know. And in all cases I can't make a triple chest anymore. And I added detection of double trapped chests to detection task. So you can test it all too, but from my point of view I'm ready.

Found a bug with torpedo's/collisionExplosion. Previously the explosion would be centered in a corner of a colliding block, resulting in the torpedo being much less effective when firing from certain directions. Now it originates in the center of the block. While this makes the explosions more consistent, it also reduces their effect and server admins will need to increase the collisionExplosion value to compensate.

For the default Torpedo, a value of 6.0 seems about right.

Code pushed, beta build where it always is...

@mwkaicz

To your latest post: I think that sounds like an excellent solution, combined with the other protections in case anything slips through. I don't know how it would slip through, but better to be careful than crash someones server. I'll sit tight and test other things until the pull request comes through.

@AJCStriker

Hopefully the above will address these concerns, so that when it happens it is handled better than it currently is.

Ironically, when setting up the test I found a bug with WorldEdit with chests that caused inventory loss. So, at least we know chests cause problems for others too :P

@AJCStriker

Right, and there are protections against even that since the map updater checks for double chests and throws a custom exception to prevent server crashes or inventory loss.

I'm not saying its perfect. The exception handling could be more graceful as currently it prevents anyone else from moving until its resolved and the exception keeps getting thrown because the map updater data doesn't get cleared.

Honestly, I wasn't worried about how gracefully it handled that particular eventuality because I didn't consider it would happen very often. However, I can make it more graceful.

@AJCStriker

Actually I think it might be you that misunderstands. It doesn't allow double chests, and will inform the user if they try to form a craft with one. Go ahead and do some tests with the current beta

EDIT: To be clear, you can cause a chest collision by either modify a craft after formation by, for example, deleting a block next to a chest and replacing the block with a chest or by flying the craft with a chest on its borders next to another chest outside the craft. In both cases, the map updater checks for a collision and throws a custom exception to prevent crashes or inventory loss in the chest.

@mwkaicz

Right, so that's actually what the plugin does right now in the latest beta build. I guess great minds think alike? :) It does not allow the craft to be created if there is a double chest on it, and if it finds a double chest somehow slipped through on the map update it will throw a custom exception on just the map update task so the server doesn't go down and the chest contents are not lost.

EDIT: to clarify, the above was not directed to mwkaicz's most recent post, but to an earlier one. Your most recent post sounds like an excellent plan to add another layer of protection.

Ok, I have done a bunch of testing.

Test parameters: 5 test crafts containing 50 chests each.

• Each chest has a different number of items in it
• The chests are arranged in a "checkered" pattern horizontally to maximize chances of collisions, because I'm pretty if there is a problem, it will show up with the horizontal translations
• 1 test craft containing 20 chests was arranged in a checkered pattern vertically just to be sure

After crafts were run simultaneously at maximum speed for an extended period using cruise control. One craft was moved by hand in random directions with random rotations over the duration of the test.

Results:
Total chest translations - 1,620,000
Total block translations - 5,190,000

No observed errors in chests. When checked, all chests contained their original contents.

1 Movecraft bug was noticed: when a sign was placed on a chest and the craft was moved, a "severe map updater error" was seen on the console and the sign disappeared. The chest contents were unaffected. This is a known bug that randomly crops up if a "fragile" block is placed on another "fragile" block.

2 Minecraft errors were noticed: on one ship, many of the chests dissapeared due to lag between the server and client. They dissapeared visually, but were still there on the server. This is a known Minecraft error when alot of updates are sent rapidly between client and server, most commonly creating "phantom signs". It was fixed by logging out and back in. The chest contents were unaffected.

The other Minecraft bug that was noticed was that during extreme lag a player fell off a chest, at a distance of no greater than 2 blocks. They died from the fall.

@VariousArtist

So what about detecting chests around the craft in translation and rotation task? When it will be near new location the craft and will threaten collison of doublechests, then disallow move and make a craft-obstructed event.

What are you thinking about this solution? In my opinion, it's best way how prevent this crash, before it comes and it should work. I'll try it and let you know.

EDIT: Ok, I got it :) no problems during translation, craft stops one block before chest when is same type of chest on both sides. Now is impossible join chest from craft with anotherone which isn't on the craft. I must to do same in rotation task then I will make request to Loraxe repository (today event, in my time zone cca after 6 hours from last edit of this comment)

@serpental

That should be a problem, what's the plugin name? Look one or two pages back on this site. Kyren_Leah has same problem as you.

@AJCStriker

Sorry, I've been away. There is a fairly simple solution, eliminate the chests during craft reconstruction and replace them when it is done. This is already done with dispensers. However, I am curious if you have been able to produce this issue yet. The system does forbid the use of double chests already, both during ship detection and it checks again during map updates. I will also do some more rigorous tests and share the results.

I agree this is an area for concern and we should be careful. But I think its a little early to throw out the idea.

@serpental please send one of your craft files, or tell something about world. Is generated with some plugin or not?

@AJCStriker

I understand your situation very well, I know what can do an unexpected bug in some plugin (I got one total freez of server yesterday from another plugin). I was afraid of doublechests too, especially I expected problems during the sinking process and I undestand what's happening during translation, but I don't know how to help Lorax to solve this.

Honestly I think that was a mistake to add support for doublechests now. There are so many unexpected events that may occur. And I'm not sure f.e. how is solved when are doublechest and double trapped chest abreast. In my opion, there should be prevention against doublechests in tasks or don't support them jet all. (I prefere prevention, but I understand if you not) But there are many of important bug fixs and we must move (like a craft :)).

If you want, we can try to do it upside down. Lorax could merge his version of bug fixs (without support of chests) to my update. And merging of chests make as separated request next time. As we could not find any mistake in hovercrafts over two weeks of testing in our server. But final decision, how it will be, is yours.