Grief Prevention

Grief Prevention

1M Downloads

ExtendIntoGroundDistance seem to act on the topmost rather than bottommost Y coordinate on claiming

LinusLeonard opened this issue · 8 comments

commented

Observed Behavior

ExtendIntoGroundDistance is not respected at all, but rather seem to be updating the wrong Y coordinate.

Example 1 of freshly created claim where the lowest ground block was level 77 and higest was 78, with ExtendIntoGroundDistance set to 5. Note that the claim is between 72 and -57. This means nobody can dig under it unless very far down.

Lesser Boundary Corner: world;100;-57;232
Greater Boundary Corner: world;104;72;236

Example 2, increased ExtendIntoGroundDistance to 20, I get this result for a claim with the lowest portion on block 78 and the highest on 79:

Lesser Boundary Corner: world;105;-57;237
Greater Boundary Corner: world;110;58;242

Example 3, ExtendIntoGroundDistance still set to 20, and both claim corners on the same Y coordinate, in this case 79:

Lesser Boundary Corner: world;112;-47;246
Greater Boundary Corner: world;116;59;250

Expected Behavior

Expected behavior is that the plugin should behave as described in the documentation at https://docs.griefprevention.com/configuration/#claim-limits

_Claims.ExtendIntoGroundDistance: 5

How far into the ground a new claim should extend from the golden shovel location. If the golden shovel is used at two different heights, the lower value will be used in conjunction with this variable. To guarantee all claims run all the way to bedrock, use a very large number. The default doesn’t sink to bedrock because that would effectively claim valuable ore that the player placing them claim hasn’t discovered yet. Please note! As a player digs or builds underneath his claim, his claim automatically sinks lower with him. Thus, mine shafts and basements are safe from grief even if you set this variable at a small value._

In other words, example 1 above should result in

Lesser Boundary Corner: world;100;72;232
Greater Boundary Corner: world;104;78;236

Example 2 should result in

Lesser Boundary Corner: world;105;58;237
Greater Boundary Corner: world;110;78;242

Example 3 should result in

Lesser Boundary Corner: world;112;59;246
Greater Boundary Corner: world;116;79;250

Reproduction steps

Create claim with golden shovel as usual with ExtendIntoGroundDistance set to some value, and then note the resulting ClaimData file contents.

Behavior first observed on a Paper 1.20.2 server with several plugins, but behavior also exhibits on a fresh Paper 1.20.2 server installation with GriefPrevention as the only plugin and using the default configuration that is generated when the plugin is run. Same with Paper 1.20.4-381 which is the current latest version.

Stack trace or error log

No response

Server version

This server is running Paper version git-Paper-318 (MC: 1.20.2) (Implementing API version 1.20.2-R0.1-SNAPSHOT) (Git: 9271ee7)
You are running the latest version
Previous version: git-Paper-307 (MC: 1.20.2)

GriefPrevention version

16.18.1

Configuration

# Default values are perfect for most servers.  If you want to customize and have a question, look for the answer here first: http://dev.bukkit.org/bukkit-plugins/grief-prevention/pages/setup-and-configuration/

GriefPrevention:
  SeaLevelOverrides:
    world: -1
    world_nether: -1
    world_the_end: -1
  Claims:
    Mode:
      world: Survival
      world_nether: Survival
      world_the_end: Survival
    PreventGlobalMonsterEggs: true
    PreventTheft: true
    ProtectCreatures: true
    PreventButtonsSwitches: true
    LockWoodenDoors: true
    LockTrapDoors: true
    LockFenceGates: true
    EnderPearlsRequireAccessTrust: true
    RaidTriggersRequireBuildTrust: true
    ProtectHorses: true
    ProtectDonkeys: true
    ProtectLlamas: true
    InitialBlocks: 0
    Claim Blocks Accrued Per Hour:
      Default: 0
    Max Accrued Claim Blocks:
      Default: 0
    Accrued Idle Threshold: 0
    AccruedIdlePercent: 0
    AbandonReturnRatio: 1.0
    AutomaticNewPlayerClaimsRadius: -1
    AutomaticNewPlayerClaimsRadiusMinimum: 0
    ExtendIntoGroundDistance: 20
    MinimumWidth: 3
    MinimumArea: 9
    MaximumDepth: -64
    InvestigationTool: STICK
    ModificationTool: GOLDEN_SHOVEL
    Expiration:
      ChestClaimDays: 0
      UnusedClaimDays: 30
      AllClaims:
        DaysInactive: 30
        ExceptWhenOwnerHasTotalClaimBlocks: -1
        ExceptWhenOwnerHasBonusClaimBlocks: -1
      AutomaticNatureRestoration:
        SurvivalWorlds: false
    AllowTrappedInAdminClaims: true
    MaximumNumberOfClaimsPerPlayer: 0
    CreationRequiresWorldGuardBuildPermission: true
    VillagerTradingRequiresPermission: true
    CommandsRequiringAccessTrust: /sethome
    DeliverManuals: false
    ManualDeliveryDelaySeconds: 30
    RavagersBreakBlocks: false
    FireSpreadsInClaims: false
    FireDamagesInClaims: false
    LecternReadingRequiresAccessTrust: true
  Spam:
    Enabled: false
    LoginCooldownSeconds: 60
    LoginLogoutNotificationsPerMinute: 5
    ChatSlashCommands: /me;/global;/local
    WhisperSlashCommands: /tell;/pm;/r;/whisper;/msg
    WarningMessage: Please reduce your noise level.  Spammers will be banned.
    BanOffenders: false
    BanMessage: Banned for spam.
    AllowedIpAddresses: 1.2.3.4; 5.6.7.8
    DeathMessageCooldownSeconds: 120
    Logout Message Delay In Seconds: 0
  PvP:
    RulesEnabledInWorld:
      world: false
      world_nether: false
      world_the_end: false
    ProtectFreshSpawns: true
    PunishLogout: false
    CombatTimeoutSeconds: 15
    AllowCombatItemDrop: false
    BlockedSlashCommands: /home;/vanish;/spawn;/tpa
    ProtectPlayersInLandClaims:
      PlayerOwnedClaims: true
      AdministrativeClaims: true
      AdministrativeSubdivisions: true
    AllowLavaDumpingNearOtherPlayers:
      PvPWorlds: true
      NonPvPWorlds: false
    AllowFlintAndSteelNearOtherPlayers:
      PvPWorlds: true
      NonPvPWorlds: false
    ProtectPetsOutsideLandClaims: true
  Economy:
    ClaimBlocksMaxBonus: 0
    ClaimBlocksPurchaseCost: 100.0
    ClaimBlocksSellValue: 100.0
  ProtectItemsDroppedOnDeath:
    PvPWorlds: false
    NonPvPWorlds: false
  BlockLandClaimExplosions: true
  BlockSurfaceCreeperExplosions: true
  BlockSurfaceOtherExplosions: false
  LimitSkyTrees: true
  LimitTreeGrowth: false
  PistonMovement: CLAIMS_ONLY
  PistonExplosionSound: true
  FireSpreads: false
  FireDestroys: false
  AdminsGetWhispers: false
  AdminsGetSignNotifications: false
  VisualizationAntiCheatCompatMode: false
  SmartBan: true
  Mute New Players Using Banned Words: true
  MaxPlayersPerIpAddress: 6
  SilenceBans: true
  Siege:
    Worlds: []
    BreakableBlocks:
    - DIRT
    DoorsOpenDelayInSeconds: 300
    CooldownEndInMinutes: 60
  EndermenMoveBlocks: false
  SilverfishBreakBlocks: false
  CreaturesTrampleCrops: false
  RabbitsEatCrops: true
  HardModeZombiesBreakDoors: true
  Database:
    URL: ''
    UserName: ''
    Password: ''
  UseBanCommand: false
  BanCommandPattern: ban %name% %reason%
  Advanced:
    fixNegativeClaimblockAmounts: true
    ClaimExpirationCheckRate: 60
    OfflinePlayer_cache_days: 90
  Abridged Logs:
    Days To Keep: 7
    Included Entry Types:
      Social Activity: true
      Suspicious Activity: true
      Administrative Activity: false
      Debug: false
      Muted Chat Messages: false
  ConfigVersion: 1

Plugin list

No response

Running without GriefPrevention

  • I attempted running the server without GriefPrevention installed.
  • The problem does not occur when GriefPrevention is removed from the server.

Running with only GriefPrevention

  • I attempted running only GriefPrevention on the server.
  • The issue still occurs when GriefPrevention is the only plugin running.

Running on a fresh, clean server installation

  • I attempted testing for the issue on a new server.
  • The issue still occurs on a new server.

Using unmodified client

  • I attempted testing for the issue with the vanilla client.
  • The issue still occurs when using the vanilla client.

We appreciate you taking the time to fill out a bug report!

  • I searched for similar issues before submitting this bug report.
commented

issue is also present in 16.18.2-beta2

commented

Thanks for the replies @Jikoo

For the general audience I do realize that this is expected and safe behavior even if it isn't really reflected in the documentation. "Unfortunately", my server is full of grown ups who won't be upset about this if I give them instructions how this system works :)

Question, the documentation mentions this "As a player digs or builds underneath his claim, his claim automatically sinks lower with him". Is this still the case irrespective of the AutoExtendClaimTask? Or are the actions of "player digs" and "builds underneath" connected to the AutoExtendClaimTask functionality somehow?

In other words, let's say my players are of the non-upset-getting variety, and I recompile the plugin with all calls to AutoExtendClaimTask commented out (I count two in total, one for using the shovel, and one for the command). Will the claim still extend downwards when you as a claim owner either dig down into it or place a block in it?

commented

Should still work, yes - that uses the datastore's extension method directly because it doesn't need to search the rest of the claim. I would still caution you against that, because frankly I've met a lot more adults with reading comprehension issues than I'd care to think about (myself included sometimes, oh boy), but it's your server. Personally I would consider instead setting the max claim depth.

commented

I'm not proficient in the Java ecosystem, but after doing some oldschool print-debugging it looks like it's the AutoExtendClaimTask mechanism that is setting off this behavior. Everything seems fine with the claim data until PlayerEventHandler.java line 2627 where there's a call to AutoExtendClaimTask (according to the source code zip in the 16.18.1 GitHub release). Haven't got the first clue how to debug this any further though.

commented

Claims have to auto extend on creation. Imagine how upset your average player would be if they attempted to recreate a claim over their build only to find out that only the top half was protected. Yeah, the system is designed to allow people to mine under the claim if possible, but that's secondary to the goal of being secure and easy to use.

If you're suggesting that the claims should not extend because there are no player blocks, then this is a duplicate of #2046, a missing natural block definition.

commented

I should also note because of your title that GP internally doesn't really care which Y is lower, because its claims all extend up to build limit. It's something I personally would like to change - would be nice to see the coordinates of the claim display top and bottom - but only the lowest Y has any effect.

commented

Hehe, I hear ya.

In terms of fixing this issue (and/or #2046 if it is indeed the same thing and can be solved in the same way), how much to get it to the top of the priority list and solved in the not too distant future? I believe this particular bunch of adults with above-par reading comprehension has paid Robo in the past for some GP-related work (probably RealEstate), but don't qoute me on that, I wasn't the server operator back then.

commented

In my eyes, this is mostly two major problems:

  • Definitions should be editable by administrators
    • Need advanced config systems, probably an entirely separate file (or files) from the regular config
    • Should support tags as well as material names (which should be flexible enough to benefit non-material listings like entities)
    • Should auto-update advanced configs to support non-power users
      • Should probably not auto-update advanced configs if they have been edited, or if done (i.e. adding a couple new materials) should note exactly what changed
  • GP's debug logging should be improved so that all administrators have the tools to figure out definition issues
    • Currently an on/off toggle with no subcategorization, leading to useless spam as coverage increases
    • Does not have much coverage, i.e. the auto-extend task has no logging but has caused issues before (like that time when I broke it and then everyone realized it existed because it started working again and got mad about it even though it's been in place for years)
    • Is a bit dated (mainly string concatenation when it should be using suppliers) and may cause slight performance issues with more widespread usage depending on how much information is included

If what you're asking is "who can I pay and how much" I don't know, but I'm not your guy - I generally refuse to do any hobby work with a price tag attached because it ruins the fun and relaxation. This is definitely a higher priority thing for me, but I've left it alone because it does require design decisions that I'd want Robo's feedback through the process of. Because I don't engage in live chat regarding the hobby work, only forums/emails, that is generally very slow going, so I've been poking at other items on the laundry list of problems instead.