ThunderGemios10 / Survival-Games

Survival Games plugin for Spigot - UPDATED for 1.13+
https://www.spigotmc.org/resources/survival-games.17740/
GNU General Public License v3.0
25 stars 22 forks source link

Bug in tile inventory reset at game reset (does not work at all) #8

Open sshipway opened 8 years ago

sshipway commented 8 years ago

When the world is reset, only block changes are reset, not Inventories or other metadata. This means

Checking the code seems to show that a chest's content is randomly updated on first open by ChestReplaceEvent to match the selected level in chest.yml; this level is picked by the colour of the wool in slot 0 or else 1. However the items previously there are all deleted first. The nightly 'chest restock' is actually just clearing the array of chests already visited, so that they are reloaded on next open. This would seem to be a big thing to fix as the GameManager.openedChest hashset would need to become a hashmap holding the original inventory, and the clear of this map done at arena reset or at chest restock would need to also reload the original content back into the chests; thus a new function for resetChests rather than just clearing the array.

This still does not solve the problem of how droppers/dispensers/armourstands/frames/etc should be correctly reset. Droppers and dispensers in particular are difficult as they are usually triggered by redstone and so catching the player interact event will not work.

sshipway commented 8 years ago

Here's how my fix works.

This does not fix frames, droppers, or dispensers. Kept entities are not moved back to their original locations if they moved.

Thus, in your HG world, you can place a block of coloured wool in every chest slot0 to indicate what tier of loot it should contain. You can also place special items into specific chests and set clear-chest=false so that they are kept on randomisation.

Obviously this needs a bit of testing. There may be a race condition in the reset code, and the original author did not put in much in the way of error trapping and debug messages, so I'm going to let the kids loose on it and see what happens. They want "opey loot" and since the default chest level is 1 they wont get this until I prime all the chests with a wool block.

sshipway commented 8 years ago

Fix for chests is in #10 No fix yet for dispenser, droppers, furnaces, frames...

ThunderGemios10 commented 8 years ago

@sshipway It seems chests still won't restock.. Players have been receiving an error. [15:37:18] [Server thread/INFO]: [SurvivalGames] [Debug] ChestReplaceEvent: 485,30,540 [15:37:18] [Server thread/INFO]: [SurvivalGames] [Debug] Clearing initial content [15:37:18] [Server thread/INFO]: [SurvivalGames] Chest at 485,30,540 is full: cannot add items

sshipway commented 8 years ago

That message is produced when the system tries 10 times to find an empty slot to put the new item into, but cannot, and so gives up. Strangely, the clearing content should ensure lots of free spaces...

The original code simply tried forever; all I added was the 10-attempt limit in case a chest started with a lot of items in it.

The players reporting the error, did they say which version of MC they are playing -- Bukkit, Spigot, 1.8.x, 1.9.x ? It may be that they are using the bleeding-edge spigot 1.9 (I tested against Spigot 1.8) and this has changes in the way the Inventory object works. Also, is this happening on initial stock, or on midnight restock, or on recurring restock event? As I mentioned previously, Java is not really my language, and I had a lot of confusion about object copies and object references; possibly there are some places where objects have not been properly deep-copied and this problem shows up on the 2nd or 3rd refill event

tl;dr - moar info!

ThunderGemios10 commented 8 years ago

@sshipway They are using Spigot x1.9, And since i want this plugin to work in 1.9, This needs to be addressed.

sshipway commented 8 years ago

I'm not able to duplicate this with my 1.8 setup; did you do anything special when creating the release JAR file? I'll pull down a copy of the release and try it against a 1.9 Spigot this week and see if that duplicates it for me; if so I will have a fix out soon...

ThunderGemios10 commented 8 years ago

@sshipway No.. These jars were actually compiled at my jenkins CI Server: http://ci.thundergemios10.com

They are compiled normally in Maven.

sshipway commented 8 years ago

The release is definitely different in size from the one I built here. Which version of Java are you running? I've noticed that can cause issues

ThunderGemios10 commented 8 years ago

JDK 8? I'm building on Java 8

sshipway commented 8 years ago

Found it. When getting inventory contents, 1.8 returns itemstacks only for occupied inventory positions; 1.9 returns one itemstack for each slot, even if it is empty. So the code to detect a full inventory always thinks chest is full even when it is not. Fairly easy fix in ChestReplaceEvent, have someting to test in place now.

At the same time also found a minor bug when deleting an arena (cant reset chests when arena has been deleted!) and in the "/sg reload" help text. I think I've fixed it all but Im too tired to test now (10pm here in NZ), will try to find time Tue or Wed evening.

ThunderGemios10 commented 8 years ago

@sshipway Ok.

sshipway commented 8 years ago

(Problem was in ChestReplaceEvent using Inventory.getSize() > 25 where I should have used Inventory.firstEmpty()<0 )

ThunderGemios10 commented 8 years ago

@sshipway Oh!

sshipway commented 8 years ago

Fixed in pull req #12 This also adds a new command /sg refill to make refill testing easier! All tested now -- sorry for the wait, but mon/tue are scouting nights so no free time for coding. Please test yourself as well, in case there is something that makes it work on my system but not on yours...

sshipway commented 8 years ago

It occurs to me that it should be possible to use the same chest hook to handle access to droppers, dispensers and furnaces, though we would also need to trap the event for dropper/dispenser trigger and for furnace changes. Then also a special tier would be needed for these as they could not use chest loot tiers -- maybe create a chest.furnace0 etc and chest.dropper0, or simply not have randomisation on these entities at all, just inventory resetting.

sshipway commented 8 years ago

Furnace change Events: FurnaceBurnEvent, FurnaceSmeltEvent, FurnaceExtractEvent Dispenser event: BlockDispenseEvent All of them can catch the same inventory open event as the chests. This should make it pretty simple to have inventories reset on all these inventory blocks. Might try coding something later this week.

ThunderGemios10 commented 8 years ago

Nice.. These are very useful..

XinityDevs commented 8 years ago

@sshipway Did you end up coding?

sshipway commented 8 years ago

Sorry, just no time recently; I've been buried under all my other projects. I would think the best way to do droppers/dispensers/furnaces is to not have randomisation on them, but hook into the listed events, plus the existing inventory open event, to store the inventories and restore with the normal function. The current inventory open hook ignores non-chest items so it would need to be expanded; other hooks could be made the same way. I still have this in my 'to look at' list but if someone else picks it up and runs with it I won't be upset ;)

sshipway commented 8 years ago

Probably, support for disp/drop/furn should be moved to s separate ticket as this tickets main issue has alreayd been addressed