Civcraft / Citadel

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/Citadel
BSD 3-Clause "New" or "Revised" License
6 stars 23 forks source link

Fix for dupe bug #136

Closed suirad closed 8 years ago

suirad commented 8 years ago

Addresses Civcraft/Citadel#99

In my research on the subject, i have narrowed down that the dupe when reinforcing stone is not a bug related to stone itself. It is a case that would happen to any reinforceable material that is also a place-able block. The only reason that is was limited to just stone blocks is because diamond and iron ingots are used for the other two reinforcement materials. But in my tests, any placeable block would have the same bug as stone currently does. I tested this by swapping stone with iron ore, and it had the same effect.

The two changes i have added, is to explicitly compare the block placed and reinforcement material to see if they are the same, and increment the values needed/removed by 1 to take into consideration the block placed.

CivcraftBot commented 8 years ago

Can one of the admins verify this patch? Type 'ok to test' to test.

rourke750 commented 8 years ago

Have you tested this because I don't believe this will work. The bug is related to spigot, (almost got fucking fixed but then dmca) and I believe it ignores what ever you set the item stack to and just decrements by one. For example if you had a stack at 10 and set it to 5 on blockplace afterwards it would be 9. I don't remember clearly if that was the case but I believe it was.

suirad commented 8 years ago

I have tested it by trying to place reinforced stone but only having one in inventory. It cancels with not enough. If i have 2 stone and place reinforced stone, it leaves me with zero. Can you further explain your example, the parameters aren't quite clear. I still have my test server up and can try it out real quick.

rourke750 commented 8 years ago

You would need to log in but this best explains it https://hub.spigotmc.org/jira/browse/SPIGOT-466?filter=-2

rourke750 commented 8 years ago

Sounds like you already fixed it though!

suirad commented 8 years ago

Hmm interesting. So testing out what was these pics. the initial problem is fixed but now if not the first stack is placed, it takes two away from the first stack it finds and one from the stack used to place. I am looking into it.

rourke750 commented 8 years ago

It's a very annoying bug which would be very messy to fix. I've mostly left it alone as this doesn't cause the end of the world.

suirad commented 8 years ago

I think i can narrow down a fix. Give me a few minutes.

suirad commented 8 years ago

Alright @rourke750 check it out. I have citadel working exactly as expected with block and non blocks as reinforcement materials. Pretty much, if there was more than one stack, and the held stack isn't the stack held when the block is placed, it would consume one extra block(as it should anyways). It is addressed here.

I was sure to go back and recheck that nothing changed for non block reinforcements.

ttk2 commented 8 years ago

ok to test

ttk2 commented 8 years ago

looks cool, drop the build on Civtest and give it a run.

rourke750 commented 8 years ago

Going to leave this for a little, some other stuff I need to watch for current citadel. After that I'll look into this.