SquidDev-CC / CCTweaks

Random additions to ComputerCraft (somewhat deprecated, use CC-Tweaked if you're on Minecraft 1.12).
MIT License
12 stars 2 forks source link

Fixing Turtle and server protection mechanisms #103

Closed inklit closed 8 years ago

inklit commented 8 years ago

I was wondering if it would be possible for you to overwrite turtle.dig and turtle.move in CCTweaks to modify the behaviour to support proper protection in GriefPrevention. Not really GriefPrevention specifically, but all Sponge plugins bloodmc and I were under the impression that dan200 was running MinecraftServer.getServer.isBlockProtected(world, x, y, z, this) to check if the block is supposed to be protected or not but apparently he is not running that but some other weird code that he shouldn't be doing. isBlockProtected in Sponge will return true if the pre-interact event it fires gets cancelled by a plugin. So, I'm asking you if it would be possible to modify the Turtle's code for proper protection compatibility. On a side note, if you fired this function in Plethora for your lasers, I would also be able to start using that on my server which would be fantastic.

Alternatively, I thought of another way that protection could be done although it is very fragile and hacky. If you were to create a function that always returns the turtle's coordinates, I could overwrite the turtle.dig and turtle.move functions in the ROM and then make an HTTP request to a web server hosted by a Sponge plugin that fires the event and returns the result of the event to the HTTP stream. If the event is cancelled according to the HTTP request, I could simply cancel the event in the ROM. This doesn't seem like the best way to do it because while I don't know how to override ROM edits, I'm sure anybody who is experienced will be able to do so with ease. (GPS would work, but that can be spoofed and not all Turtle's have GPS capability)

inklit commented 8 years ago

Thank you! I am very excited to test this fix and see if it works.

inklit commented 8 years ago

I'm not entirely sure if this worked or not. These effects are quite hard to explain and there's a vast majority of them.

It would be far easier to demonstrate the issues on my server than try to explain them but I will try my best.

If a turtle is inside of a protected region, it is able to move and break blocks without any problems (which is not supposed to happen).

If a turtle is outside of a protected region, it is unable to pass into a protected region or place/break blocks in the protected region. This is intended, but the problem with this is that it _actually duplicates the block_s that it attempts to break, and permanently loses the blocks it attempts to place.

Also, the Turtle turns off when it attempts to move into a claim, which tells me that it was probably GriefPrevention resetting the block state instead of CCTweaks preventing it from attempting to move in the first place.

SquidDev commented 8 years ago

@inklit Hrrmr. Just double checking you enabled the turtle.enableProtection option (it is disabled by default)? I haven't done anything fancy here (I'm utilising CC's existing hooks) so I'm not sure this is CCTweaks's fault.

inklit commented 8 years ago

Ah I did not. Let me restart the server and try this again. My bad.

inklit commented 8 years ago

It seems to have the same symtopms with enableProtection on and off.

inklit commented 8 years ago

Do you think you could print to the console every time that isBlockProtected is fired and the results of the function?

SquidDev commented 8 years ago

At the top:

import org.squiddev.cctweaks.core.utils.DebugLogger
ComputerCraftAPI.registerPermissionProvider(new ITurtlePermissionProvider() {
    @Override
    public boolean isBlockEnterable(World world, BlockPos pos) {
        // Block moving outside the world border
        DebugLogger.debug("Position is " + pos + " → + " world.getWorldBorder().contains(pos));
        return !Config.Turtle.enableProtection || world.getWorldBorder().contains(pos);
    }

    @Override
    public boolean isBlockEditable(World world, BlockPos pos) {
        DebugLogger.debug("Position is " + pos + " / " + world);
        if (!Config.Turtle.enableProtection || !(world instanceof WorldServer)) return true;

        WorldServer worldServer = (WorldServer) world;

        // Setup a fake player
        TurtlePlayer player = new TurtlePlayer(worldServer);
        player.setPosition(pos.getX(), pos.getY(), pos.getZ());
        // Check the block can be broken and is within the world border
        DebugLogger.debug("Got protected:  " + worldServer.getMinecraftServer().isBlockProtected(world, pos, player));
        return !worldServer.getMinecraftServer().isBlockProtected(world, pos, player) && world.getWorldBorder().contains(pos);
    }
});
SquidDev commented 8 years ago

At a guess some of this is due to GriefProtection, some of this is due to CCTweaks or CC being derpy. This is going to be hard to debug.

inklit commented 8 years ago

I think all of the stuff before was GriefPrevention attempting to be as resilient as it could with CC trying to force block changes. We will see what isBlockProtected is returning soon after I compile.

SquidDev commented 8 years ago

I'm pretty sure CC adds this already: see dan200.computercraft.ComputerCraft.isBlockEnterable and .isBlockEditable (lines 460 and 482).

If this is the case then this commit can be safely reverted as it doesn't do anything.


Is there a way to disable GriefProtection's retrospective changing of blocks? This might make the issue easier to track down.

inklit commented 8 years ago

GriefPrevention simply cancels the block event as far as I know, but I don't think it would be possible to disable that without removing it from the source and recompiling.

inklit commented 8 years ago

I'm not seeing any debug related information in the server console, I enabled debug mode in CCTweaks.conf, is it going somewhere else?

SquidDev commented 8 years ago

My theory is that it is never reaching my code as ComputerCraft.isBlockEnterable/Editable is called first which checks MinecraftServer.isBlockProtected before hand.

inklit commented 8 years ago

I made my own version with System.out.println() and it seemed to work but I didn't make it output in a very understandable manner. It is definitely reaching your code.

inklit commented 8 years ago

Do you happen to know what code is fired to actually change the blocks? Maybe we can look at it from another angle and try to change that code instead to act like an actual player.

isBlockProtected was returning true and getWorldBorder was returning false both as expected

EDIT: I changed my code to reflect the changes you had except I replaced the debug logger with System.out.println() and the results seem to be different. I believe this sheds some pretty obvious light on the problem.

isBlockEditable is never fired when a Turtle moves. isBlockEnterable never fires isBlockprotected. isBlockProtected is actually not returning true.

[10:40:35] [Server thread/INFO] [STDOUT]: [org.squiddev.cctweaks.core.turtle.DefaultTurtleProviders$4:isBlockEditable:135]: Position is BlockPos{x=5895, y=71, z=2901} / net.minecraft.world.WorldServer@3cb26ff5
[10:40:35] [Server thread/INFO] [STDOUT]: [org.squiddev.cctweaks.core.turtle.DefaultTurtleProviders$4:isBlockEditable:144]: Got protected:  false
>
SquidDev commented 8 years ago

Well the key code is dan200.computercraft.shared.turtle.core.TurtleMoveCommand.execute/.canEnter. Just thought I'd check: is turtlesObeyBlockProtection true in the ComputerCraft config file?

inklit commented 8 years ago

Yes, it's turned on.

My theory is that Sponge's code is never being fired for isBlockProtected. (If it is being fired, it isn't passing the event to GriefPrevention for GriefPrevention to cancel it properly) I am wondering if Sponge ever actually overwrites isBlockProtected for getMinecraftServer() in the first place.

This is the code that Sponge should be firing. https://github.com/SpongePowered/SpongeCommon/commit/4755835a78b38261332cefe16125e2b0ffc71717

inklit commented 8 years ago

The issue seemed to be the later of the mentioned above. GriefPrevention wasn't receiving the onBlockPre event properly thus not cancelling it. The issue seems to be fixed by https://github.com/MinecraftPortCentral/GriefPrevention/commit/5166e7725fc2665976431ea73b9d893cd198c209

The commits that were made seem to be unnecessary as you suspected thus can be reverted. They proved incredibly useful while debugging this problem and tracking down the source of the problem, though.

There's a slight oddity in the fact the Turtle inherits the build permissions of whoever placed it, but this should not be a problem on my server since you can't interact with Turtles that are inside of claims. I believe players will find it useful since now they can protect their farms without the Turtle glitching about inside of the claim.

Thanks for helping me debug this.